[ARVADOS] created: 1.1.4-184-g49fb55a
Git user
git at public.curoverse.com
Wed May 2 16:12:59 EDT 2018
at 49fb55aa1a9fb35d8e7e620717cb42f77d448a43 (commit)
commit 49fb55aa1a9fb35d8e7e620717cb42f77d448a43
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date: Wed May 2 16:12:15 2018 -0400
12626: Add arvados/v1/users/merge API.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>
diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index 587f4db..3b89067 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -6,9 +6,9 @@ class Arvados::V1::UsersController < ApplicationController
accept_attribute_as_json :prefs, Hash
skip_before_filter :find_object_by_uuid, only:
- [:activate, :current, :system, :setup]
+ [:activate, :current, :system, :setup, :merge]
skip_before_filter :render_404_if_no_object, only:
- [:activate, :current, :system, :setup]
+ [:activate, :current, :system, :setup, :merge]
before_filter :admin_required, only: [:setup, :unsetup, :update_uuid]
def current
@@ -125,6 +125,40 @@ class Arvados::V1::UsersController < ApplicationController
show
end
+ def merge
+ if !Thread.current[:api_client].andand.is_trusted
+ return send_error("supplied API token is not from a trusted client", status: 403)
+ end
+
+ dst_auth = ApiClientAuthorization.validate(token: params[:new_user_token])
+ if !dst_auth
+ return send_error("invalid new_user_token", status: 401)
+ end
+ if !dst_auth.api_client.andand.is_trusted
+ return send_error("supplied new_user_token is not from a trusted client", status: 403)
+ end
+ dst_user = dst_auth.user
+
+ if current_user.uuid == dst_user.uuid
+ return send_error("cannot merge user to self", status: 422)
+ end
+
+ if !dst_user.can?(write: params[:new_owner_uuid])
+ return send_error("new_owner_uuid is not writable", status: 403)
+ end
+
+ redirect = params[:redirect_to_new_user]
+ if !redirect
+ return send_error("merge with redirect_to_new_user=false is not yet supported", status: 422)
+ end
+
+ @object = current_user
+ act_as_system_user do
+ @object.merge(new_owner_uuid: params[:new_owner_uuid], redirect_to_user_uuid: redirect && dst_user.uuid)
+ end
+ show
+ end
+
protected
def self._setup_requires_parameters
diff --git a/services/api/app/controllers/user_sessions_controller.rb b/services/api/app/controllers/user_sessions_controller.rb
index 5de85bc..87967a4 100644
--- a/services/api/app/controllers/user_sessions_controller.rb
+++ b/services/api/app/controllers/user_sessions_controller.rb
@@ -26,9 +26,9 @@ class UserSessionsController < ApplicationController
# Only local users can create sessions, hence uuid_like_pattern
# here.
- user = User.where('identity_url = ? and uuid like ?',
- omniauth['info']['identity_url'],
- User.uuid_like_pattern).first
+ user = User.unscoped.where('identity_url = ? and uuid like ?',
+ omniauth['info']['identity_url'],
+ User.uuid_like_pattern).first
if not user
# Check for permission to log in to an existing User record with
# a different identity_url
@@ -45,6 +45,14 @@ class UserSessionsController < ApplicationController
end
end
end
+
+ while (uuid = user.andand.redirect_to_user_uuid)
+ user = User.where(uuid: uuid).first
+ if !user
+ raise Exception.new("identity_url #{omniauth['info']['identity_url']} redirects to nonexistent uuid #{uuid}")
+ end
+ end
+
if not user
# New user registration
user = User.new(:email => omniauth['info']['email'],
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index b158faa..b267a63 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -92,7 +92,7 @@ class ApiClientAuthorization < ArvadosModel
uuid_prefix+".arvadosapi.com")
end
- def self.validate(token:, remote:)
+ def self.validate(token:, remote: nil)
return nil if !token
remote ||= Rails.configuration.uuid_prefix
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 9209411..4296d61 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -48,6 +48,8 @@ class User < ArvadosModel
has_many :authorized_keys, :foreign_key => :authorized_user_uuid, :primary_key => :uuid
has_many :repositories, foreign_key: :owner_uuid, primary_key: :uuid
+ default_scope { where('redirect_to_user_uuid is null') }
+
api_accessible :user, extend: :common do |t|
t.add :email
t.add :username
@@ -269,25 +271,58 @@ class User < ArvadosModel
old_uuid = self.uuid
self.uuid = new_uuid
save!(validate: false)
- ActiveRecord::Base.descendants.reject(&:abstract_class?).each do |klass|
- klass.columns.each do |col|
- if col.name.end_with?('_uuid')
- column = col.name.to_sym
- klass.where(column => old_uuid).update_all(column => new_uuid)
- end
- end
+ change_all_uuid_refs(old_uuid: old_uuid, new_uuid: new_uuid)
+ end
+ end
+
+ # Merge this user's owned items into dst_user.
+ def merge(new_owner_uuid:, redirect_to_user_uuid:)
+ raise PermissionDeniedError if !current_user.andand.is_admin
+ raise "not implemented" if !redirect_to_user_uuid
+ transaction(requires_new: true) do
+ reload
+ new_user = User.where(uuid: redirect_to_user_uuid).first
+ raise "user does not exist" if !new_user
+ if User.where('uuid in (?) and redirect_to_user_uuid is not null',
+ [new_owner_uuid, redirect_to_user_uuid]).any?
+ raise "cannot merge to/from an already merged user"
end
+ ApiClientAuthorization.
+ where(user_id: id).
+ update_all(user_id: new_user.id)
+ [
+ [AuthorizedKey, :owner_uuid],
+ [AuthorizedKey, :authorized_user_uuid],
+ [Repository, :owner_uuid],
+ [Link, :tail_uuid],
+ [Link, :head_uuid],
+ ].each do |klass, column|
+ klass.where(column => uuid).update_all(column => new_user.uuid)
+ end
+ change_all_uuid_refs(old_uuid: uuid, new_uuid: new_owner_uuid)
+ update_attributes!(redirect_to_user_uuid: new_user.uuid)
end
end
protected
+ def change_all_uuid_refs(old_uuid:, new_uuid:)
+ ActiveRecord::Base.descendants.reject(&:abstract_class?).each do |klass|
+ klass.columns.each do |col|
+ if col.name.end_with?('_uuid')
+ column = col.name.to_sym
+ klass.where(column => old_uuid).update_all(column => new_uuid)
+ end
+ end
+ end
+ end
+
def ensure_ownership_path_leads_to_user
true
end
def permission_to_update
- if username_changed?
+ if username_changed? || redirect_to_user_uuid_changed?
current_user.andand.is_admin
else
# users must be able to update themselves (even if they are
@@ -298,7 +333,8 @@ class User < ArvadosModel
def permission_to_create
current_user.andand.is_admin or
- (self == current_user and
+ (self == current_user &&
+ self.redirect_to_user_uuid.nil? &&
self.is_active == Rails.configuration.new_users_are_active)
end
diff --git a/services/api/config/routes.rb b/services/api/config/routes.rb
index ad2406a..b0c0984 100644
--- a/services/api/config/routes.rb
+++ b/services/api/config/routes.rb
@@ -81,6 +81,7 @@ Server::Application.routes.draw do
post 'setup', on: :collection
post 'unsetup', on: :member
post 'update_uuid', on: :member
+ post 'merge', on: :collection
end
resources :virtual_machines do
get 'logins', on: :member
diff --git a/services/api/db/migrate/20180501182859_add_redirect_to_user_uuid_to_users.rb b/services/api/db/migrate/20180501182859_add_redirect_to_user_uuid_to_users.rb
new file mode 100644
index 0000000..b2460ae
--- /dev/null
+++ b/services/api/db/migrate/20180501182859_add_redirect_to_user_uuid_to_users.rb
@@ -0,0 +1,15 @@
+class AddRedirectToUserUuidToUsers < ActiveRecord::Migration
+ def up
+ add_column :users, :redirect_to_user_uuid, :string
+ User.reset_column_information
+ remove_index :users, name: 'users_search_index'
+ add_index :users, User.searchable_columns('ilike') - ['prefs'], name: 'users_search_index'
+ end
+
+ def down
+ remove_index :users, name: 'users_search_index'
+ remove_column :users, :redirect_to_user_uuid
+ User.reset_column_information
+ add_index :users, User.searchable_columns('ilike') - ['prefs'], name: 'users_search_index'
+ end
+end
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index 2751114..caf7683 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -768,7 +768,8 @@ CREATE TABLE users (
updated_at timestamp without time zone NOT NULL,
default_owner_uuid character varying(255),
is_active boolean DEFAULT false,
- username character varying(255)
+ username character varying(255),
+ redirect_to_user_uuid character varying
);
@@ -2714,7 +2715,7 @@ CREATE UNIQUE INDEX unique_schema_migrations ON schema_migrations USING btree (v
-- Name: users_search_index; Type: INDEX; Schema: public; Owner: -
--
-CREATE INDEX users_search_index ON users USING btree (uuid, owner_uuid, modified_by_client_uuid, modified_by_user_uuid, email, first_name, last_name, identity_url, default_owner_uuid, username);
+CREATE INDEX users_search_index ON users USING btree (uuid, owner_uuid, modified_by_client_uuid, modified_by_user_uuid, email, first_name, last_name, identity_url, default_owner_uuid, username, redirect_to_user_uuid);
--
@@ -3068,3 +3069,5 @@ INSERT INTO schema_migrations (version) VALUES ('20180228220311');
INSERT INTO schema_migrations (version) VALUES ('20180313180114');
+INSERT INTO schema_migrations (version) VALUES ('20180501182859');
+
diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb
index 39c905a..ebab993 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -815,6 +815,88 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
end
end
+ test "refuse to merge with redirect_to_user_uuid=false (not yet supported)" do
+ authorize_with :project_viewer_trustedclient
+ post :merge, {
+ new_user_token: api_client_authorizations(:active_trustedclient).api_token,
+ new_owner_uuid: users(:active).uuid,
+ redirect_to_new_user: false,
+ }
+ assert_response(422)
+ end
+
+ test "refuse to merge user into self" do
+ authorize_with(:active_trustedclient)
+ post(:merge, {
+ new_user_token: api_client_authorizations(:active_trustedclient).api_token,
+ new_owner_uuid: users(:active).uuid,
+ redirect_to_new_user: true,
+ })
+ assert_response(422)
+ end
+
+ [[:active, :project_viewer_trustedclient],
+ [:active_trustedclient, :project_viewer]].each do |src, dst|
+ test "refuse to merge with untrusted token (#{src} -> #{dst})" do
+ authorize_with(src)
+ post(:merge, {
+ new_user_token: api_client_authorizations(dst).api_token,
+ new_owner_uuid: api_client_authorizations(dst).user.uuid,
+ redirect_to_new_user: true,
+ })
+ assert_response(403)
+ end
+ end
+
+ [[:expired_trustedclient, :project_viewer_trustedclient],
+ [:project_viewer_trustedclient, :expired_trustedclient]].each do |src, dst|
+ test "refuse to merge with expired token (#{src} -> #{dst})" do
+ authorize_with(src)
+ post(:merge, {
+ new_user_token: api_client_authorizations(dst).api_token,
+ new_owner_uuid: api_client_authorizations(dst).user.uuid,
+ redirect_to_new_user: true,
+ })
+ assert_response(401)
+ end
+ end
+
+ test "refuse to merge if new_owner_uuid is not writable" do
+ authorize_with(:project_viewer_trustedclient)
+ post(:merge, {
+ new_user_token: api_client_authorizations(:active_trustedclient).api_token,
+ new_owner_uuid: groups(:anonymously_accessible_project).uuid,
+ redirect_to_new_user: true,
+ })
+ assert_response(403)
+ end
+
+ test "refuse to update redirect_to_user_uuid directly" do
+ authorize_with(:active_trustedclient)
+ patch(:update, {
+ id: users(:active).uuid,
+ user: {
+ redirect_to_user_uuid: users(:active).uuid,
+ },
+ })
+ assert_response(403)
+ end
+
+ test "merge 'project_viewer' account into 'active' account" do
+ authorize_with(:project_viewer_trustedclient)
+ post(:merge, {
+ new_user_token: api_client_authorizations(:active_trustedclient).api_token,
+ new_owner_uuid: users(:active).uuid,
+ redirect_to_new_user: true,
+ })
+ assert_response(:success)
+ assert_equal(users(:project_viewer).redirect_to_user_uuid, users(:active).uuid)
+
+ auth = ApiClientAuthorization.validate(token: api_client_authorizations(:project_viewer).api_token)
+ assert_not_nil(auth)
+ assert_not_nil(auth.user)
+ assert_equal(users(:active).uuid, auth.user.uuid)
+ end
NON_ADMIN_USER_DATA = ["uuid", "kind", "is_active", "email", "first_name",
"last_name", "username"].sort
diff --git a/services/api/test/integration/users_test.rb b/services/api/test/integration/users_test.rb
index 8ddab3f..28e43b8 100644
--- a/services/api/test/integration/users_test.rb
+++ b/services/api/test/integration/users_test.rb
@@ -216,4 +216,39 @@ class UsersTest < ActionDispatch::IntegrationTest
end
nil
end
+
+ test 'merge active into project_viewer account' do
+ post('/arvados/v1/groups', {
+ group: {
+ group_class: 'project',
+ name: "active user's stuff",
+ },
+ }, auth(:project_viewer))
+ assert_response(:success)
+ project_uuid = json_response['uuid']
+
+ post('/arvados/v1/users/merge', {
+ new_user_token: api_client_authorizations(:project_viewer_trustedclient).api_token,
+ new_owner_uuid: project_uuid,
+ redirect_to_new_user: true,
+ }, auth(:active_trustedclient))
+ assert_response(:success)
+
+ get('/arvados/v1/users/current', {}, auth(:active))
+ assert_response(:success)
+ assert_equal(users(:project_viewer).uuid, json_response['uuid'])
+
+ get('/arvados/v1/authorized_keys/' + authorized_keys(:active).uuid, {}, auth(:active))
+ assert_response(:success)
+ assert_equal(users(:project_viewer).uuid, json_response['owner_uuid'])
+ assert_equal(users(:project_viewer).uuid, json_response['authorized_user_uuid'])
+
+ get('/arvados/v1/repositories/' + repositories(:foo).uuid, {}, auth(:active))
+ assert_response(:success)
+ assert_equal(users(:project_viewer).uuid, json_response['owner_uuid'])
+
+ get('/arvados/v1/groups/' + groups(:aproject).uuid, {}, auth(:active))
+ assert_response(:success)
+ assert_equal(project_uuid, json_response['owner_uuid'])
+ end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list