[ARVADOS] created: 2fc6dde609883cf65b6d0eadd16a7bb8a738f026
git at public.curoverse.com
git at public.curoverse.com
Wed Apr 23 10:59:05 EDT 2014
at 2fc6dde609883cf65b6d0eadd16a7bb8a738f026 (commit)
commit 2fc6dde609883cf65b6d0eadd16a7bb8a738f026
Author: Brett Smith <brett at curoverse.com>
Date: Wed Apr 23 10:59:08 2014 -0400
api: Migrate VM auth scopes to new system.
VirtualMachinesController was the only one doing anything special with
API token scopes before we provided the more general-purpose system.
This commit removes its specialized code, and provides a database
migration to convert those specialized scopes to the general-purpose
schema.
diff --git a/services/api/app/controllers/arvados/v1/virtual_machines_controller.rb b/services/api/app/controllers/arvados/v1/virtual_machines_controller.rb
index 10b4bd8..e176348 100644
--- a/services/api/app/controllers/arvados/v1/virtual_machines_controller.rb
+++ b/services/api/app/controllers/arvados/v1/virtual_machines_controller.rb
@@ -1,12 +1,8 @@
class Arvados::V1::VirtualMachinesController < ApplicationController
skip_before_filter :find_object_by_uuid, :only => :get_all_logins
skip_before_filter :render_404_if_no_object, :only => :get_all_logins
- skip_before_filter(:require_auth_scope_all,
- :only => [:logins, :get_all_logins])
before_filter(:admin_required,
:only => [:logins, :get_all_logins])
- before_filter(:require_auth_scope_for_get_all_logins,
- :only => [:logins, :get_all_logins])
def logins
get_all_logins
@@ -44,16 +40,4 @@ class Arvados::V1::VirtualMachinesController < ApplicationController
end
render json: { kind: "arvados#HashList", items: @response }
end
-
- protected
-
- def require_auth_scope_for_get_all_logins
- if @object
- # Client wants all logins for a single VM.
- require_auth_scope(['all', arvados_v1_virtual_machine_url(@object.uuid)])
- else
- # ...for a non-existent VM, or all VMs.
- require_auth_scope(['all'])
- end
- end
end
diff --git a/services/api/db/migrate/20140423133559_new_scope_format.rb b/services/api/db/migrate/20140423133559_new_scope_format.rb
new file mode 100644
index 0000000..5b69e95
--- /dev/null
+++ b/services/api/db/migrate/20140423133559_new_scope_format.rb
@@ -0,0 +1,48 @@
+# At the time we introduced scopes everywhere, VirtualMachinesController
+# recognized scopes that gave the URL for a VM to grant access to that VM's
+# login list. This migration converts those VM-specific scopes to the new
+# general format, and back.
+
+class NewScopeFormat < ActiveRecord::Migration
+ include CurrentApiClient
+
+ VM_PATH_REGEX =
+ %r{(/arvados/v1/virtual_machines/[0-9a-z]{5}-[0-9a-z]{5}-[0-9a-z]{15})}
+ OLD_SCOPE_REGEX = %r{^https?://[^/]+#{VM_PATH_REGEX.source}$}
+ NEW_SCOPE_REGEX = %r{^GET #{VM_PATH_REGEX.source}/logins$}
+
+ def fix_scopes_matching(regex)
+ act_as_system_user
+ ApiClientAuthorization.find_each do |auth|
+ auth.scopes = auth.scopes.map do |scope|
+ if match = regex.match(scope)
+ yield match
+ else
+ scope
+ end
+ end
+ auth.save!
+ end
+ end
+
+ def up
+ fix_scopes_matching(OLD_SCOPE_REGEX) do |match|
+ "GET #{match[1]}/logins"
+ end
+ end
+
+ def down
+ case Rails.env
+ when 'test'
+ hostname = 'www.example.com'
+ else
+ require 'socket'
+ hostname = Socket.gethostname
+ end
+ fix_scopes_matching(NEW_SCOPE_REGEX) do |match|
+ Rails.application.routes.url_for(controller: 'virtual_machines',
+ uuid: match[1].split('/').last,
+ host: hostname, protocol: 'https')
+ end
+ end
+end
diff --git a/services/api/db/schema.rb b/services/api/db/schema.rb
index e2301e5..969b7c6 100644
--- a/services/api/db/schema.rb
+++ b/services/api/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended to check this file into your version control system.
-ActiveRecord::Schema.define(:version => 20140407184311) do
+ActiveRecord::Schema.define(:version => 20140423133559) do
create_table "api_client_authorizations", :force => true do |t|
t.string "api_token", :null => false
@@ -70,7 +70,7 @@ ActiveRecord::Schema.define(:version => 20140407184311) do
create_table "collections", :force => true do |t|
t.string "locator"
t.string "owner_uuid"
- t.datetime "created_at"
+ t.datetime "created_at", :null => false
t.string "modified_by_client_uuid"
t.string "modified_by_user_uuid"
t.datetime "modified_at"
@@ -80,7 +80,7 @@ ActiveRecord::Schema.define(:version => 20140407184311) do
t.string "redundancy_confirmed_by_client_uuid"
t.datetime "redundancy_confirmed_at"
t.integer "redundancy_confirmed_as"
- t.datetime "updated_at"
+ t.datetime "updated_at", :null => false
t.string "uuid"
t.text "manifest_text"
end
@@ -104,8 +104,8 @@ ActiveRecord::Schema.define(:version => 20140407184311) do
t.string "repository_name"
t.string "sha1"
t.string "message"
- t.datetime "created_at"
- t.datetime "updated_at"
+ t.datetime "created_at", :null => false
+ t.datetime "updated_at", :null => false
end
add_index "commits", ["repository_name", "sha1"], :name => "index_commits_on_repository_name_and_sha1", :unique => true
@@ -133,8 +133,8 @@ ActiveRecord::Schema.define(:version => 20140407184311) do
t.string "modified_by_user_uuid"
t.datetime "modified_at"
t.text "properties"
- t.datetime "created_at"
- t.datetime "updated_at"
+ t.datetime "created_at", :null => false
+ t.datetime "updated_at", :null => false
end
add_index "humans", ["uuid"], :name => "index_humans_on_uuid", :unique => true
@@ -182,8 +182,8 @@ ActiveRecord::Schema.define(:version => 20140407184311) do
t.boolean "running"
t.boolean "success"
t.string "output"
- t.datetime "created_at"
- t.datetime "updated_at"
+ t.datetime "created_at", :null => false
+ t.datetime "updated_at", :null => false
t.string "priority"
t.string "is_locked_by_uuid"
t.string "log"
@@ -235,7 +235,7 @@ ActiveRecord::Schema.define(:version => 20140407184311) do
create_table "links", :force => true do |t|
t.string "uuid"
t.string "owner_uuid"
- t.datetime "created_at"
+ t.datetime "created_at", :null => false
t.string "modified_by_client_uuid"
t.string "modified_by_user_uuid"
t.datetime "modified_at"
@@ -244,9 +244,7 @@ ActiveRecord::Schema.define(:version => 20140407184311) do
t.string "name"
t.string "head_uuid"
t.text "properties"
- t.datetime "updated_at"
- t.string "head_kind"
- t.string "tail_kind"
+ t.datetime "updated_at", :null => false
end
add_index "links", ["created_at"], :name => "index_links_on_created_at"
@@ -304,7 +302,7 @@ ActiveRecord::Schema.define(:version => 20140407184311) do
create_table "pipeline_instances", :force => true do |t|
t.string "uuid"
t.string "owner_uuid"
- t.datetime "created_at"
+ t.datetime "created_at", :null => false
t.string "modified_by_client_uuid"
t.string "modified_by_user_uuid"
t.datetime "modified_at"
@@ -313,7 +311,7 @@ ActiveRecord::Schema.define(:version => 20140407184311) do
t.text "components"
t.boolean "success"
t.boolean "active", :default => false
- t.datetime "updated_at"
+ t.datetime "updated_at", :null => false
t.text "properties"
end
diff --git a/services/api/test/fixtures/api_client_authorizations.yml b/services/api/test/fixtures/api_client_authorizations.yml
index 77e5048..f772c4f 100644
--- a/services/api/test/fixtures/api_client_authorizations.yml
+++ b/services/api/test/fixtures/api_client_authorizations.yml
@@ -42,7 +42,7 @@ admin_vm:
api_token: adminvirtualmachineabcdefghijklmnopqrstuvwxyz12345
expires_at: 2038-01-01 00:00:00
# scope refers to the testvm fixture.
- scopes: ["https://www.example.com/arvados/v1/virtual_machines/zzzzz-2x53u-382brsig8rp3064"]
+ scopes: ["GET /arvados/v1/virtual_machines/zzzzz-2x53u-382brsig8rp3064/logins"]
admin_noscope:
api_client: untrusted
commit 0451d362f079f008abd4fcc12d6b42c42f743e2a
Author: Brett Smith <brett at curoverse.com>
Date: Tue Apr 22 17:45:46 2014 -0400
api: Introduce path-based API token scopes.
Refs #2662 for background discussion.
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 4b13fca..0331127 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -321,7 +321,8 @@ class ApplicationController < ActionController::Base
end
def require_auth_scope_all
- require_login and require_auth_scope(['all'])
+ require_login and
+ require_auth_scope(["#{request.method} #{request.path}"])
end
def require_auth_scope(ok_scopes)
diff --git a/services/api/lib/current_api_client.rb b/services/api/lib/current_api_client.rb
index 401be16..2d9fe18 100644
--- a/services/api/lib/current_api_client.rb
+++ b/services/api/lib/current_api_client.rb
@@ -31,12 +31,15 @@ module CurrentApiClient
# Does the current API client authorization include any of ok_scopes?
def current_api_client_auth_has_scope(ok_scopes)
- auth_scopes = current_api_client_authorization.andand.scopes || []
- unless auth_scopes.index('all') or (auth_scopes & ok_scopes).any?
- logger.warn "Insufficient auth scope: need #{ok_scopes}, #{current_api_client_authorization.inspect} has #{auth_scopes}"
- return false
- end
- true
+ (current_api_client_authorization.andand.scopes || []).select { |scope|
+ if scope == 'all'
+ true
+ elsif scope.end_with? '/'
+ ok_scopes.select { |s| s.start_with? scope }.any?
+ else
+ ok_scopes.include? scope
+ end
+ }.any?
end
def system_user_uuid
diff --git a/services/api/test/fixtures/api_client_authorizations.yml b/services/api/test/fixtures/api_client_authorizations.yml
index 5a715e3..77e5048 100644
--- a/services/api/test/fixtures/api_client_authorizations.yml
+++ b/services/api/test/fixtures/api_client_authorizations.yml
@@ -51,6 +51,28 @@ admin_noscope:
expires_at: 2038-01-01 00:00:00
scopes: []
+active_userlist:
+ api_client: untrusted
+ user: active
+ api_token: activeuserlistabcdefghijklmnopqrstuvwxyz1234568900
+ expires_at: 2038-01-01 00:00:00
+ scopes: ["GET /arvados/v1/users"]
+
+active_specimens:
+ api_client: untrusted
+ user: active
+ api_token: activespecimensabcdefghijklmnopqrstuvwxyz123456890
+ expires_at: 2038-01-01 00:00:00
+ scopes: ["GET /arvados/v1/specimens/"]
+
+active_apitokens:
+ api_client: trusted_workbench
+ user: active
+ api_token: activeapitokensabcdefghijklmnopqrstuvwxyz123456789
+ expires_at: 2038-01-01 00:00:00
+ scopes: ["GET /arvados/v1/api_client_authorizations",
+ "POST /arvados/v1/api_client_authorizations"]
+
spectator:
api_client: untrusted
user: spectator
diff --git a/services/api/test/integration/api_client_authorizations_scopes_test.rb b/services/api/test/integration/api_client_authorizations_scopes_test.rb
index 7269d38..0961b81 100644
--- a/services/api/test/integration/api_client_authorizations_scopes_test.rb
+++ b/services/api/test/integration/api_client_authorizations_scopes_test.rb
@@ -32,6 +32,46 @@ class Arvados::V1::ApiTokensScopeTest < ActionController::IntegrationTest
request_with_auth(:post_via_redirect, *args)
end
+ test "user list token can only list users" do
+ auth_with :active_userlist
+ get_with_auth v1_url('users')
+ assert_response :success
+ get_with_auth v1_url('users', 'current')
+ assert_response 403
+ get_with_auth v1_url('virtual_machines')
+ assert_response 403
+ end
+
+ test "specimens token can see exactly owned specimens" do
+ auth_with :active_specimens
+ get_with_auth v1_url('specimens')
+ assert_response 403
+ get_with_auth v1_url('specimens', specimens(:owned_by_active_user).uuid)
+ assert_response :success
+ get_with_auth v1_url('specimens', specimens(:owned_by_spectator).uuid)
+ assert_includes(403..404, @response.status)
+ end
+
+ test "multiple scopes" do
+ def get_token_count
+ get_with_auth v1_url('api_client_authorizations')
+ assert_response :success
+ token_count = JSON.parse(@response.body)['items_available']
+ assert_not_nil(token_count, "could not find token count")
+ token_count
+ end
+ auth_with :active_apitokens
+ get_with_auth v1_url('api_client_authorizations',
+ api_client_authorizations(:active_apitokens).uuid)
+ assert_response 403
+ token_count = get_token_count
+ post_with_auth(v1_url('api_client_authorizations'),
+ api_client_authorization: {user_id: users(:active).id})
+ assert_response :success
+ assert_equal(token_count + 1, get_token_count,
+ "token count suggests new token was not created")
+ end
+
test "token without scope has no access" do
# Logs are good for this test, because logs have relatively
# few access controls enforced at the model level.
commit 5ac5b09dc05defd5a95f41e027b5b7162e3497b9
Author: Brett Smith <brett at curoverse.com>
Date: Tue Apr 22 16:58:37 2014 -0400
api: Token scope tests are now integration tests.
Because our token scopes are URL-based, it helps to be able to test
at a level where we can introspect a real request URL. Rails'
controller tests don't do that.
diff --git a/services/api/test/fixtures/api_client_authorizations.yml b/services/api/test/fixtures/api_client_authorizations.yml
index fd856f8..5a715e3 100644
--- a/services/api/test/fixtures/api_client_authorizations.yml
+++ b/services/api/test/fixtures/api_client_authorizations.yml
@@ -42,7 +42,7 @@ admin_vm:
api_token: adminvirtualmachineabcdefghijklmnopqrstuvwxyz12345
expires_at: 2038-01-01 00:00:00
# scope refers to the testvm fixture.
- scopes: ["http://test.host/arvados/v1/virtual_machines/zzzzz-2x53u-382brsig8rp3064"]
+ scopes: ["https://www.example.com/arvados/v1/virtual_machines/zzzzz-2x53u-382brsig8rp3064"]
admin_noscope:
api_client: untrusted
diff --git a/services/api/test/functional/arvados/v1/api_tokens_scope_test.rb b/services/api/test/functional/arvados/v1/api_tokens_scope_test.rb
deleted file mode 100644
index 14cacf1..0000000
--- a/services/api/test/functional/arvados/v1/api_tokens_scope_test.rb
+++ /dev/null
@@ -1,31 +0,0 @@
-# The v1 API uses token scopes to control access to the REST API at the path
-# level. This is enforced in the base ApplicationController, making it a
-# functional test that we can run against many different controllers.
-
-require 'test_helper'
-
-class Arvados::V1::ApiTokensScopeTest < ActionController::TestCase
- test "token without scope has no access" do
- # The logs controller is good for this test, because logs have relatively
- # few access controls enforced at the model level.
- @controller = Arvados::V1::LogsController.new
- authorize_with :admin_noscope
- get :index
- assert_response 403
- get :show, {id: logs(:log1).uuid}
- assert_response 403
- post :create, log: {}
- assert_response 403
- end
-
- test "VM login scopes work" do
- # A system administration script makes an API token with limited scope
- # for virtual machines to let it see logins.
- @controller = Arvados::V1::VirtualMachinesController.new
- authorize_with :admin_vm
- get :logins, uuid: virtual_machines(:testvm).uuid
- assert_response :success
- get :logins, uuid: virtual_machines(:testvm2).uuid
- assert(@response.status >= 400, "getting testvm2 logins should have failed")
- end
-end
diff --git a/services/api/test/integration/api_client_authorizations_scopes_test.rb b/services/api/test/integration/api_client_authorizations_scopes_test.rb
new file mode 100644
index 0000000..7269d38
--- /dev/null
+++ b/services/api/test/integration/api_client_authorizations_scopes_test.rb
@@ -0,0 +1,59 @@
+# The v1 API uses token scopes to control access to the REST API at the path
+# level. This is enforced in the base ApplicationController, making it a
+# functional test that we can run against many different controllers.
+
+require 'test_helper'
+
+class Arvados::V1::ApiTokensScopeTest < ActionController::IntegrationTest
+ fixtures :all
+
+ def setup
+ @token = {}
+ end
+
+ def auth_with(name)
+ @token = {api_token: api_client_authorizations(name).api_token}
+ end
+
+ def v1_url(*parts)
+ (['arvados', 'v1'] + parts).join('/')
+ end
+
+ def request_with_auth(method, path, params={})
+ https!
+ send(method, path, @token.merge(params))
+ end
+
+ def get_with_auth(*args)
+ request_with_auth(:get_via_redirect, *args)
+ end
+
+ def post_with_auth(*args)
+ request_with_auth(:post_via_redirect, *args)
+ end
+
+ test "token without scope has no access" do
+ # Logs are good for this test, because logs have relatively
+ # few access controls enforced at the model level.
+ auth_with :admin_noscope
+ get_with_auth v1_url('logs')
+ assert_response 403
+ get_with_auth v1_url('logs', logs(:log1).uuid)
+ assert_response 403
+ post_with_auth(v1_url('logs'), log: {})
+ assert_response 403
+ end
+
+ test "VM login scopes work" do
+ # A system administration script makes an API token with limited scope
+ # for virtual machines to let it see logins.
+ def vm_logins_url(name)
+ v1_url('virtual_machines', virtual_machines(name).uuid, 'logins')
+ end
+ auth_with :admin_vm
+ get_with_auth vm_logins_url(:testvm)
+ assert_response :success
+ get_with_auth vm_logins_url(:testvm2)
+ assert(@response.status >= 400, "getting testvm2 logins should have failed")
+ end
+end
commit 7b7f514314e03e65d8a2e966958aab769fb1597b
Author: Brett Smith <brett at curoverse.com>
Date: Tue Apr 22 14:52:54 2014 -0400
api: Add test for API token with empty scope.
diff --git a/services/api/test/fixtures/api_client_authorizations.yml b/services/api/test/fixtures/api_client_authorizations.yml
index 0a19c3a..fd856f8 100644
--- a/services/api/test/fixtures/api_client_authorizations.yml
+++ b/services/api/test/fixtures/api_client_authorizations.yml
@@ -44,6 +44,13 @@ admin_vm:
# scope refers to the testvm fixture.
scopes: ["http://test.host/arvados/v1/virtual_machines/zzzzz-2x53u-382brsig8rp3064"]
+admin_noscope:
+ api_client: untrusted
+ user: admin
+ api_token: adminnoscopeabcdefghijklmnopqrstuvwxyz123456789012
+ expires_at: 2038-01-01 00:00:00
+ scopes: []
+
spectator:
api_client: untrusted
user: spectator
diff --git a/services/api/test/functional/arvados/v1/api_tokens_scope_test.rb b/services/api/test/functional/arvados/v1/api_tokens_scope_test.rb
index 3e17dca..14cacf1 100644
--- a/services/api/test/functional/arvados/v1/api_tokens_scope_test.rb
+++ b/services/api/test/functional/arvados/v1/api_tokens_scope_test.rb
@@ -5,6 +5,19 @@
require 'test_helper'
class Arvados::V1::ApiTokensScopeTest < ActionController::TestCase
+ test "token without scope has no access" do
+ # The logs controller is good for this test, because logs have relatively
+ # few access controls enforced at the model level.
+ @controller = Arvados::V1::LogsController.new
+ authorize_with :admin_noscope
+ get :index
+ assert_response 403
+ get :show, {id: logs(:log1).uuid}
+ assert_response 403
+ post :create, log: {}
+ assert_response 403
+ end
+
test "VM login scopes work" do
# A system administration script makes an API token with limited scope
# for virtual machines to let it see logins.
commit 3bdcdc52bc52449783954d2faa91f98a62fa073e
Author: Brett Smith <brett at curoverse.com>
Date: Tue Apr 22 14:44:34 2014 -0400
api: Test VM login scopes.
The virtual machine controller is the only one doing anything
interesting with API token scopes right now. I'm writing this test
for that functionality to make sure it's stays effective through
refactoring.
diff --git a/services/api/test/fixtures/api_client_authorizations.yml b/services/api/test/fixtures/api_client_authorizations.yml
index 5cada90..0a19c3a 100644
--- a/services/api/test/fixtures/api_client_authorizations.yml
+++ b/services/api/test/fixtures/api_client_authorizations.yml
@@ -36,6 +36,14 @@ active_trustedclient:
api_token: 27bnddk6x2nmq00a1e3gq43n9tsl5v87a3faqar2ijj8tud5en
expires_at: 2038-01-01 00:00:00
+admin_vm:
+ api_client: untrusted
+ user: admin
+ api_token: adminvirtualmachineabcdefghijklmnopqrstuvwxyz12345
+ expires_at: 2038-01-01 00:00:00
+ # scope refers to the testvm fixture.
+ scopes: ["http://test.host/arvados/v1/virtual_machines/zzzzz-2x53u-382brsig8rp3064"]
+
spectator:
api_client: untrusted
user: spectator
diff --git a/services/api/test/functional/arvados/v1/api_tokens_scope_test.rb b/services/api/test/functional/arvados/v1/api_tokens_scope_test.rb
new file mode 100644
index 0000000..3e17dca
--- /dev/null
+++ b/services/api/test/functional/arvados/v1/api_tokens_scope_test.rb
@@ -0,0 +1,18 @@
+# The v1 API uses token scopes to control access to the REST API at the path
+# level. This is enforced in the base ApplicationController, making it a
+# functional test that we can run against many different controllers.
+
+require 'test_helper'
+
+class Arvados::V1::ApiTokensScopeTest < ActionController::TestCase
+ test "VM login scopes work" do
+ # A system administration script makes an API token with limited scope
+ # for virtual machines to let it see logins.
+ @controller = Arvados::V1::VirtualMachinesController.new
+ authorize_with :admin_vm
+ get :logins, uuid: virtual_machines(:testvm).uuid
+ assert_response :success
+ get :logins, uuid: virtual_machines(:testvm2).uuid
+ assert(@response.status >= 400, "getting testvm2 logins should have failed")
+ end
+end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list