From: Tom Clegg Date: Tue, 26 Nov 2019 21:16:27 +0000 (-0500) Subject: Merge branch '15877-accept-json-in-json' X-Git-Tag: 2.0.0~99 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/5338c3fe0abbc6599aa290085be13eecfb0044e9?hp=b69a0b0f7b4b9a773fb060a592057f9bd146e480 Merge branch '15877-accept-json-in-json' fixes #15877 Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- diff --git a/apps/workbench/app/controllers/users_controller.rb b/apps/workbench/app/controllers/users_controller.rb index febd6e3a1d..27fc12bf4c 100644 --- a/apps/workbench/app/controllers/users_controller.rb +++ b/apps/workbench/app/controllers/users_controller.rb @@ -124,7 +124,7 @@ class UsersController < ApplicationController def show_pane_list if current_user.andand.is_admin - super | %w(Admin) + %w(Admin) | super else super end diff --git a/apps/workbench/app/views/users/_setup_popup.html.erb b/apps/workbench/app/views/users/_setup_popup.html.erb index d6f25136c4..4c3a95e87e 100644 --- a/apps/workbench/app/views/users/_setup_popup.html.erb +++ b/apps/workbench/app/views/users/_setup_popup.html.erb @@ -11,7 +11,7 @@ SPDX-License-Identifier: AGPL-3.0 %>
-

Setup Shell Account

+

Setup Account


@@ -48,7 +48,7 @@ SPDX-License-Identifier: AGPL-3.0 %> <% end %>
- +
- +
diff --git a/apps/workbench/app/views/users/_show_admin.html.erb b/apps/workbench/app/views/users/_show_admin.html.erb index ddff79be01..1da22d438f 100644 --- a/apps/workbench/app/views/users/_show_admin.html.erb +++ b/apps/workbench/app/views/users/_show_admin.html.erb @@ -4,35 +4,36 @@ SPDX-License-Identifier: AGPL-3.0 %>
+

- As an admin, you can log in as this user. When you’ve - finished, you will need to log out and log in again with your - own account. + This page enables you to manage users.

-
- <%= button_to "Log in as #{@object.full_name}", sudo_user_url(id: @object.uuid), class: 'btn btn-primary' %> -
-

- As an admin, you can setup a shell account for this user. - The login name is automatically generated from the user's e-mail address. + This button sets up a user. After setup, they will be able use + Arvados. This dialog box also allows you to optionally set up a + shell account for this user. The login name is automatically + generated from the user's e-mail address.

-
- <%= link_to "Setup shell account #{'for ' if @object.full_name.present?} #{@object.full_name}", setup_popup_user_url(id: @object.uuid), {class: 'btn btn-primary', :remote => true, 'data-toggle' => "modal", 'data-target' => '#user-setup-modal-window'} %> -
+ <%= link_to "Setup account #{'for ' if @object.full_name.present?} #{@object.full_name}", setup_popup_user_url(id: @object.uuid), {class: 'btn btn-primary', :remote => true, 'data-toggle' => "modal", 'data-target' => '#user-setup-modal-window'} %> -

+

As an admin, you can deactivate and reset this user. This will remove all repository/VM permissions for the user. If you "setup" the user again, the user will have to sign the user - agreement again. + agreement again. You may also want to reassign data ownership. +

+ + <%= button_to "Deactivate #{@object.full_name}", unsetup_user_url(id: @object.uuid), class: 'btn btn-primary', data: {confirm: "Are you sure you want to deactivate #{@object.full_name}?"} %> + +

+ As an admin, you can log in as this user. When you’ve + finished, you will need to log out and log in again with your + own account.

-
- <%= button_to "Deactivate #{@object.full_name}", unsetup_user_url(id: @object.uuid), class: 'btn btn-primary', data: {confirm: "Are you sure you want to deactivate #{@object.full_name}?"} %> -
+ <%= button_to "Log in as #{@object.full_name}", sudo_user_url(id: @object.uuid), class: 'btn btn-primary' %>
diff --git a/apps/workbench/test/integration/users_test.rb b/apps/workbench/test/integration/users_test.rb index bad01a1c62..57be9d370d 100644 --- a/apps/workbench/test/integration/users_test.rb +++ b/apps/workbench/test/integration/users_test.rb @@ -77,6 +77,8 @@ class UsersTest < ActionDispatch::IntegrationTest find('a', text: 'Show'). click + click_link 'Attributes' + assert page.has_text? 'modified_by_user_uuid' page.within(:xpath, '//span[@data-name="is_active"]') do assert_equal "false", text, "Expected new user's is_active to be false" @@ -84,7 +86,7 @@ class UsersTest < ActionDispatch::IntegrationTest click_link 'Advanced' click_link 'Metadata' - assert page.has_text? 'can_login' # make sure page is rendered / ready + assert page.has_text? 'can_read' # make sure page is rendered / ready assert page.has_no_text? 'VirtualMachine:' end @@ -103,9 +105,9 @@ class UsersTest < ActionDispatch::IntegrationTest # Setup user click_link 'Admin' - assert page.has_text? 'As an admin, you can setup' + assert page.has_text? 'This button sets up a user' - click_link 'Setup shell account for Active User' + click_link 'Setup account for Active User' within '.modal-content' do find 'label', text: 'Virtual Machine' @@ -113,6 +115,7 @@ class UsersTest < ActionDispatch::IntegrationTest end visit user_url + click_link 'Attributes' assert page.has_text? 'modified_by_client_uuid' click_link 'Advanced' @@ -123,7 +126,7 @@ class UsersTest < ActionDispatch::IntegrationTest # Click on Setup button again and this time also choose a VM click_link 'Admin' - click_link 'Setup shell account for Active User' + click_link 'Setup account for Active User' within '.modal-content' do select("testvm.shell", :from => 'vm_uuid') @@ -132,12 +135,15 @@ class UsersTest < ActionDispatch::IntegrationTest end visit user_url + click_link 'Attributes' find '#Attributes', text: 'modified_by_client_uuid' click_link 'Advanced' click_link 'Metadata' assert page.has_text? 'VirtualMachine: testvm.shell' assert page.has_text? '["test group one", "test-group-two"]' + vm_links = all("a", text: "VirtualMachine:") + assert_equal(2, vm_links.size) end test "unsetup active user" do @@ -155,7 +161,7 @@ class UsersTest < ActionDispatch::IntegrationTest user_url = page.current_url # Verify that is_active is set - find('a,button', text: 'Attributes').click + click_link 'Attributes' assert page.has_text? 'modified_by_user_uuid' page.within(:xpath, '//span[@data-name="is_active"]') do assert_equal "true", text, "Expected user's is_active to be true" @@ -176,6 +182,8 @@ class UsersTest < ActionDispatch::IntegrationTest # poltergeist returns true for confirm(), so we don't need to accept. end + click_link 'Attributes' + # Should now be back in the Attributes tab for the user assert page.has_text? 'modified_by_user_uuid' page.within(:xpath, '//span[@data-name="is_active"]') do @@ -188,7 +196,7 @@ class UsersTest < ActionDispatch::IntegrationTest # setup user again and verify links present click_link 'Admin' - click_link 'Setup shell account for Active User' + click_link 'Setup account for Active User' within '.modal-content' do select("testvm.shell", :from => 'vm_uuid') @@ -196,6 +204,7 @@ class UsersTest < ActionDispatch::IntegrationTest end visit user_url + click_link 'Attributes' assert page.has_text? 'modified_by_client_uuid' click_link 'Advanced' @@ -211,7 +220,7 @@ class UsersTest < ActionDispatch::IntegrationTest # Setup user click_link 'Admin' - assert page.has_text? 'As an admin, you can setup' + assert page.has_text? 'This button sets up a user' click_link 'Add new group' diff --git a/apps/workbench/test/test_helper.rb b/apps/workbench/test/test_helper.rb index 28a7fa5137..84728b8c68 100644 --- a/apps/workbench/test/test_helper.rb +++ b/apps/workbench/test/test_helper.rb @@ -362,6 +362,7 @@ module Minitest n += 1 raise if n > 2 || e.is_a?(Skip) STDERR.puts "Test failed, retrying (##{n})" + ActiveSupport::TestCase.reset_api_fixtures_now retry end rescue *PASSTHROUGH_EXCEPTIONS diff --git a/doc/_includes/_vocabulary_migrate_py.liquid b/doc/_includes/_vocabulary_migrate_py.liquid new file mode 120000 index 0000000000..9cd36294fc --- /dev/null +++ b/doc/_includes/_vocabulary_migrate_py.liquid @@ -0,0 +1 @@ +../../tools/vocabulary-migrate/vocabulary-migrate.py \ No newline at end of file diff --git a/doc/admin/workbench2-vocabulary.html.textile.liquid b/doc/admin/workbench2-vocabulary.html.textile.liquid index 82c384c282..e259f78625 100644 --- a/doc/admin/workbench2-vocabulary.html.textile.liquid +++ b/doc/admin/workbench2-vocabulary.html.textile.liquid @@ -48,4 +48,20 @@ The @values@ member is optional and is used to define valid key/label pairs when When any key or value has more than one label option, Workbench2's user interface will allow the user to select any of the options. But because only the IDs are saved in the system, when the property is displayed in the user interface, the label shown will be the first of each group defined in the vocabulary file. For example, the user could select the property key @Species@ and @Homo sapiens@ as its value, but the user interface will display it as @Animal: Human@ because those labels are the first in the vocabulary definition. -Internally, Workbench2 uses the IDs to do property based searches, so if the user searches by @Animal: Human@ or @Species: Homo sapiens@, both will return the same results. \ No newline at end of file +Internally, Workbench2 uses the IDs to do property based searches, so if the user searches by @Animal: Human@ or @Species: Homo sapiens@, both will return the same results. + +h2. Properties migration + +After installing the new vocabulary definition, it may be necessary to migrate preexisting properties that were set up using literal strings. This can be a big task depending on the number of properties on the vocabulary and the amount of collections and projects on the cluster. + +To help with this task we provide below a migration example script that accepts the new vocabulary definition file as an input, and uses the @ARVADOS_API_TOKEN@ and @ARVADOS_API_HOST@ environment variables to connect to the cluster, search for every collection and group that has properties with labels defined on the vocabulary file, and migrates them to the corresponding identifiers. + +This script will not run if the vocabulary file has duplicated labels for different keys or for different values inside a key, this is a failsafe mechanism to avoid migration errors. + +Please take into account that this script requires admin credentials. It also offers a @--dry-run@ flag that will report what changes are required without applying them, so it can be reviewed by an administrator. + +Also, take into consideration that this example script does case-sensitive matching on labels. + +{% codeblock as python %} +{% include 'vocabulary_migrate_py' %} +{% endcodeblock %} \ No newline at end of file diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb index e84a3d2187..651eacf626 100644 --- a/services/api/app/models/api_client_authorization.rb +++ b/services/api/app/models/api_client_authorization.rb @@ -108,10 +108,25 @@ class ApiClientAuthorization < ArvadosModel clnt end + def self.check_system_root_token token + if token == Rails.configuration.SystemRootToken + return ApiClientAuthorization.new(user: User.find_by_uuid(system_user_uuid), + api_token: token, + api_client: ApiClient.new(is_trusted: true, url_prefix: "")) + else + return nil + end + end + def self.validate(token:, remote: nil) - return nil if !token + return nil if token.nil? or token.empty? remote ||= Rails.configuration.ClusterID + auth = self.check_system_root_token(token) + if !auth.nil? + return auth + end + case token[0..2] when 'v2/' _, token_uuid, secret, optional = token.split('/') @@ -221,20 +236,16 @@ class ApiClientAuthorization < ArvadosModel # Sync user record. if remote_user_prefix == Rails.configuration.Login.LoginCluster - # Remote cluster controls our user database, copy both - # 'is_active' and 'is_admin' - user.is_active = remote_user['is_active'] + # Remote cluster controls our user database, set is_active if + # remote is active. If remote is not active, user will be + # unsetup (see below). + user.is_active = true if remote_user['is_active'] user.is_admin = remote_user['is_admin'] else if Rails.configuration.Users.NewUsersAreActive || Rails.configuration.RemoteClusters[remote_user_prefix].andand["ActivateUsers"] - # Default policy is to activate users, so match activate - # with the remote record. - user.is_active = remote_user['is_active'] - elsif !remote_user['is_active'] - # Deactivate user if the remote is inactive, otherwise don't - # change 'is_active'. - user.is_active = false + # Default policy is to activate users + user.is_active = true if remote_user['is_active'] end end @@ -243,6 +254,10 @@ class ApiClientAuthorization < ArvadosModel end act_as_system_user do + if user.is_active && !remote_user['is_active'] + user.unsetup + end + user.save! # We will accept this token (and avoid reloading the user diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb index a49aa6f56a..d9bb18c3e5 100644 --- a/services/api/app/models/user.rb +++ b/services/api/app/models/user.rb @@ -21,6 +21,7 @@ class User < ArvadosModel }, uniqueness: true, allow_nil: true) + validate :must_unsetup_to_deactivate before_update :prevent_privilege_escalation before_update :prevent_inactive_admin before_update :verify_repositories_empty, :if => Proc.new { |user| @@ -188,17 +189,19 @@ class User < ArvadosModel # create links def setup(openid_prefix:, repo_name: nil, vm_uuid: nil) - oid_login_perm = create_oid_login_perm openid_prefix repo_perm = create_user_repo_link repo_name vm_login_perm = create_vm_login_permission_link(vm_uuid, username) if vm_uuid group_perm = create_user_group_link - return [oid_login_perm, repo_perm, vm_login_perm, group_perm, self].compact + return [repo_perm, vm_login_perm, group_perm, self].compact end # delete user signatures, login, repo, and vm perms, and mark as inactive def unsetup # delete oid_login_perms for this user + # + # note: these permission links are obsolete, they have no effect + # on anything and they are not created for new users. Link.where(tail_uuid: self.email, link_class: 'permission', name: 'can_login').destroy_all @@ -234,6 +237,37 @@ class User < ArvadosModel self.save! end + def must_unsetup_to_deactivate + if self.is_active_changed? && + self.is_active_was == true && + !self.is_active + + group = Group.where(name: 'All users').select do |g| + g[:uuid].match(/-f+$/) + end.first + + # When a user is set up, they are added to the "All users" + # group. A user that is part of the "All users" group is + # allowed to self-activate. + # + # It doesn't make sense to deactivate a user (set is_active = + # false) without first removing them from the "All users" group, + # because they would be able to immediately reactivate + # themselves. + # + # The 'unsetup' method removes the user from the "All users" + # group (and also sets is_active = false) so send a message + # explaining the correct way to deactivate a user. + # + if Link.where(tail_uuid: self.uuid, + head_uuid: group[:uuid], + link_class: 'permission', + name: 'can_read').any? + errors.add :is_active, "cannot be set to false directly, use the 'Deactivate' button on Workbench, or the 'unsetup' API call" + end + end + end + def set_initial_username(requested: false) if !requested.is_a?(String) || requested.empty? email_parts = email.partition("@") @@ -577,30 +611,6 @@ class User < ArvadosModel merged end - def create_oid_login_perm(openid_prefix) - # Check oid_login_perm - oid_login_perms = Link.where(tail_uuid: self.email, - head_uuid: self.uuid, - link_class: 'permission', - name: 'can_login') - - if !oid_login_perms.any? - # create openid login permission - oid_login_perm = Link.create!(link_class: 'permission', - name: 'can_login', - tail_uuid: self.email, - head_uuid: self.uuid, - properties: { - "identity_url_prefix" => openid_prefix, - }) - logger.info { "openid login permission: " + oid_login_perm[:uuid] } - else - oid_login_perm = oid_login_perms.first - end - - return oid_login_perm - end - def create_user_repo_link(repo_name) # repo_name is optional if not repo_name diff --git a/services/api/config/arvados_config.rb b/services/api/config/arvados_config.rb index f82f6e5f37..b5fcd43414 100644 --- a/services/api/config/arvados_config.rb +++ b/services/api/config/arvados_config.rb @@ -111,7 +111,7 @@ arvcfg.declare_config "Login.ProviderAppID", String, :sso_app_id arvcfg.declare_config "Login.LoginCluster", String arvcfg.declare_config "Login.RemoteTokenRefresh", ActiveSupport::Duration arvcfg.declare_config "TLS.Insecure", Boolean, :sso_insecure -arvcfg.declare_config "Services.SSO.ExternalURL", NonemptyString, :sso_provider_url +arvcfg.declare_config "Services.SSO.ExternalURL", String, :sso_provider_url arvcfg.declare_config "AuditLogs.MaxAge", ActiveSupport::Duration, :max_audit_log_age arvcfg.declare_config "AuditLogs.MaxDeleteBatch", Integer, :max_audit_log_delete_batch arvcfg.declare_config "AuditLogs.UnloggedAttributes", Hash, :unlogged_attributes, ->(cfg, k, v) { arrayToHash cfg, "AuditLogs.UnloggedAttributes", v } diff --git a/services/api/test/functional/arvados/v1/repositories_controller_test.rb b/services/api/test/functional/arvados/v1/repositories_controller_test.rb index 537fe52527..cfcd917d65 100644 --- a/services/api/test/functional/arvados/v1/repositories_controller_test.rb +++ b/services/api/test/functional/arvados/v1/repositories_controller_test.rb @@ -52,7 +52,7 @@ class Arvados::V1::RepositoriesControllerTest < ActionController::TestCase end act_as_system_user do u = users(:active) - u.is_active = false + u.unsetup u.save! end authorize_with :admin 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 74f017a678..e3763c389e 100644 --- a/services/api/test/functional/arvados/v1/users_controller_test.rb +++ b/services/api/test/functional/arvados/v1/users_controller_test.rb @@ -113,11 +113,8 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase assert_not_nil created['email'], 'expected non-nil email' assert_nil created['identity_url'], 'expected no identity_url' - # arvados#user, repo link and link add user to 'All users' group - verify_links_added 4 - - verify_link response_items, 'arvados#user', true, 'permission', 'can_login', - created['uuid'], created['email'], 'arvados#user', false, 'User' + # repo link and link add user to 'All users' group + verify_links_added 3 verify_link response_items, 'arvados#repository', true, 'permission', 'can_manage', "foo/#{repo_name}", created['uuid'], 'arvados#repository', true, 'Repository' @@ -255,8 +252,8 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase assert_not_nil response_object['uuid'], 'expected uuid for the new user' assert_equal response_object['email'], 'foo@example.com', 'expected given email' - # four extra links; system_group, login, group and repo perms - verify_links_added 4 + # three extra links; system_group, group and repo perms + verify_links_added 3 end test "setup user with fake vm and expect error" do @@ -292,8 +289,8 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase assert_not_nil response_object['uuid'], 'expected uuid for the new user' assert_equal response_object['email'], 'foo@example.com', 'expected given email' - # five extra links; system_group, login, group, vm, repo - verify_links_added 5 + # four extra links; system_group, group, vm, repo + verify_links_added 4 end test "setup user with valid email, no vm and no repo as input" do @@ -310,11 +307,8 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase assert_not_nil response_object['uuid'], 'expected uuid for new user' assert_equal response_object['email'], 'foo@example.com', 'expected given email' - # three extra links; system_group, login, and group - verify_links_added 3 - - verify_link response_items, 'arvados#user', true, 'permission', 'can_login', - response_object['uuid'], response_object['email'], 'arvados#user', false, 'User' + # two extra links; system_group, and group + verify_links_added 2 verify_link response_items, 'arvados#group', true, 'permission', 'can_read', 'All users', response_object['uuid'], 'arvados#group', true, 'Group' @@ -347,8 +341,8 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase assert_equal 'test_first_name', response_object['first_name'], 'expecting first name' - # five extra links; system_group, login, group, repo and vm - verify_links_added 5 + # four extra links; system_group, group, repo and vm + verify_links_added 4 end test "setup user with an existing user email and check different object is created" do @@ -370,8 +364,8 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase assert_not_equal response_object['uuid'], inactive_user['uuid'], 'expected different uuid after create operation' assert_equal inactive_user['email'], response_object['email'], 'expected given email' - # system_group, openid, group, and repo. No vm link. - verify_links_added 4 + # system_group, group, and repo. No vm link. + verify_links_added 3 end test "setup user with openid prefix" do @@ -398,11 +392,8 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase assert_nil created['identity_url'], 'expected no identity_url' # verify links - # four new links: system_group, arvados#user, repo, and 'All users' group. - verify_links_added 4 - - verify_link response_items, 'arvados#user', true, 'permission', 'can_login', - created['uuid'], created['email'], 'arvados#user', false, 'User' + # three new links: system_group, repo, and 'All users' group. + verify_links_added 3 verify_link response_items, 'arvados#repository', true, 'permission', 'can_manage', 'foo/usertestrepo', created['uuid'], 'arvados#repository', true, 'Repository' @@ -457,12 +448,10 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase assert_not_nil created['email'], 'expected non-nil email' assert_nil created['identity_url'], 'expected no identity_url' - # five new links: system_group, arvados#user, repo, vm and 'All - # users' group link - verify_links_added 5 + # four new links: system_group, repo, vm and 'All users' group link + verify_links_added 4 - verify_link response_items, 'arvados#user', true, 'permission', 'can_login', - created['uuid'], created['email'], 'arvados#user', false, 'User' + # system_group isn't part of the response. See User#add_system_group_permission_link verify_link response_items, 'arvados#repository', true, 'permission', 'can_manage', 'foo/usertestrepo', created['uuid'], 'arvados#repository', true, 'Repository' diff --git a/services/api/test/helpers/users_test_helper.rb b/services/api/test/helpers/users_test_helper.rb index 585619e747..6ca9977a5e 100644 --- a/services/api/test/helpers/users_test_helper.rb +++ b/services/api/test/helpers/users_test_helper.rb @@ -48,11 +48,9 @@ module UsersTestHelper oid_login_perms = Link.where(tail_uuid: email, link_class: 'permission', name: 'can_login').where("head_uuid like ?", User.uuid_like_pattern) - if expect_oid_login_perms - assert oid_login_perms.any?, "expected oid_login_perms" - else - assert !oid_login_perms.any?, "expected all oid_login_perms deleted" - end + + # these don't get added any more! they shouldn't appear ever. + assert !oid_login_perms.any?, "expected all oid_login_perms deleted" repo_perms = Link.where(tail_uuid: uuid, link_class: 'permission', diff --git a/services/api/test/integration/remote_user_test.rb b/services/api/test/integration/remote_user_test.rb index b3cfe27190..04a45420fd 100644 --- a/services/api/test/integration/remote_user_test.rb +++ b/services/api/test/integration/remote_user_test.rb @@ -143,6 +143,30 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest assert_equal 'blarney@example.com', json_response['email'] end + test 'remote user is deactivated' do + Rails.configuration.RemoteClusters['zbbbb'].ActivateUsers = true + get '/arvados/v1/users/current', + params: {format: 'json'}, + headers: auth(remote: 'zbbbb') + assert_response :success + assert_equal true, json_response['is_active'] + + # revoke original token + @stub_content[:is_active] = false + + # simulate cache expiry + ApiClientAuthorization.where( + uuid: salted_active_token(remote: 'zbbbb').split('/')[1]). + update_all(expires_at: db_current_time - 1.minute) + + # re-authorize after cache expires + get '/arvados/v1/users/current', + params: {format: 'json'}, + headers: auth(remote: 'zbbbb') + assert_equal false, json_response['is_active'] + + end + test 'authenticate with remote token, remote username conflicts with local' do @stub_content[:username] = 'active' get '/arvados/v1/users/current', @@ -255,6 +279,24 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest refute_includes(group_uuids, groups(:testusergroup_admins).uuid) end + test 'do not auto-activate user from untrusted cluster' do + Rails.configuration.RemoteClusters['zbbbb'].AutoSetupNewUsers = false + Rails.configuration.RemoteClusters['zbbbb'].ActivateUsers = false + get '/arvados/v1/users/current', + params: {format: 'json'}, + headers: auth(remote: 'zbbbb') + assert_response :success + assert_equal 'zbbbb-tpzed-000000000000000', json_response['uuid'] + assert_equal false, json_response['is_admin'] + assert_equal false, json_response['is_active'] + assert_equal 'foo@example.com', json_response['email'] + assert_equal 'barney', json_response['username'] + post '/arvados/v1/users/zbbbb-tpzed-000000000000000/activate', + params: {format: 'json'}, + headers: auth(remote: 'zbbbb') + assert_response 422 + end + test 'auto-activate user from trusted cluster' do Rails.configuration.RemoteClusters['zbbbb'].ActivateUsers = true get '/arvados/v1/users/current', diff --git a/services/api/test/integration/user_sessions_test.rb b/services/api/test/integration/user_sessions_test.rb index fdb8628c5d..fcc0ce4e52 100644 --- a/services/api/test/integration/user_sessions_test.rb +++ b/services/api/test/integration/user_sessions_test.rb @@ -111,6 +111,7 @@ class UserSessionsApiTest < ActionDispatch::IntegrationTest ].each do |testcase| test "user auto-activate #{testcase.inspect}" do # Configure auto_setup behavior according to testcase[:cfg] + Rails.configuration.Users.NewUsersAreActive = false Rails.configuration.Users.AutoSetupNewUsers = testcase[:cfg][:auto] Rails.configuration.Users.AutoSetupNewUsersWithVmUUID = (testcase[:cfg][:vm] ? virtual_machines(:testvm).uuid : "") diff --git a/services/api/test/integration/users_test.rb b/services/api/test/integration/users_test.rb index 11ebb3f4fd..6a1d5c8011 100644 --- a/services/api/test/integration/users_test.rb +++ b/services/api/test/integration/users_test.rb @@ -36,9 +36,7 @@ class UsersTest < ActionDispatch::IntegrationTest assert_not_nil created['email'], 'expected non-nil email' assert_nil created['identity_url'], 'expected no identity_url' - # arvados#user, repo link and link add user to 'All users' group - verify_link response_items, 'arvados#user', true, 'permission', 'can_login', - created['uuid'], created['email'], 'arvados#user', false, 'arvados#user' + # repo link and link add user to 'All users' group verify_link response_items, 'arvados#repository', true, 'permission', 'can_manage', 'foo/usertestrepo', created['uuid'], 'arvados#repository', true, 'Repository' @@ -117,9 +115,7 @@ class UsersTest < ActionDispatch::IntegrationTest assert_not_nil created['email'], 'expected non-nil email' assert_equal created['email'], 'foo@example.com', 'expected input email' - # three new links: system_group, arvados#user, and 'All users' group. - verify_link response_items, 'arvados#user', true, 'permission', 'can_login', - created['uuid'], created['email'], 'arvados#user', false, 'arvados#user' + # two new links: system_group, and 'All users' group. verify_link response_items, 'arvados#group', true, 'permission', 'can_read', 'All users', created['uuid'], 'arvados#group', true, 'Group' @@ -196,9 +192,7 @@ class UsersTest < ActionDispatch::IntegrationTest assert_not_nil created['uuid'], 'expected uuid for the new user' assert_equal created['email'], 'foo@example.com', 'expected given email' - # five extra links: system_group, login, group, repo and vm - verify_link response_items, 'arvados#user', true, 'permission', 'can_login', - created['uuid'], created['email'], 'arvados#user', false, 'arvados#user' + # four extra links: system_group, login, group, repo and vm verify_link response_items, 'arvados#group', true, 'permission', 'can_read', 'All users', created['uuid'], 'arvados#group', true, 'Group' @@ -339,4 +333,119 @@ class UsersTest < ActionDispatch::IntegrationTest end + test "cannot set is_activate to false directly" do + post('/arvados/v1/users', + params: { + user: { + email: "bob@example.com", + username: "bobby" + }, + }, + headers: auth(:admin)) + assert_response(:success) + user = json_response + assert_equal false, user['is_active'] + + post("/arvados/v1/users/#{user['uuid']}/activate", + params: {}, + headers: auth(:admin)) + assert_response(:success) + user = json_response + assert_equal true, user['is_active'] + + put("/arvados/v1/users/#{user['uuid']}", + params: { + user: {is_active: false} + }, + headers: auth(:admin)) + assert_response 422 + end + + test "cannot self activate when AutoSetupNewUsers is false" do + Rails.configuration.Users.NewUsersAreActive = false + Rails.configuration.Users.AutoSetupNewUsers = false + + user = nil + token = nil + act_as_system_user do + user = User.create!(email: "bob@example.com", username: "bobby") + ap = ApiClientAuthorization.create!(user: user, api_client: ApiClient.all.first) + token = ap.api_token + end + + get("/arvados/v1/users/#{user['uuid']}", + params: {}, + headers: {"HTTP_AUTHORIZATION" => "Bearer #{token}"}) + assert_response(:success) + user = json_response + assert_equal false, user['is_active'] + + post("/arvados/v1/users/#{user['uuid']}/activate", + params: {}, + headers: {"HTTP_AUTHORIZATION" => "Bearer #{token}"}) + assert_response 422 + assert_match(/Cannot activate without being invited/, json_response['errors'][0]) + end + + + test "cannot self activate after unsetup" do + Rails.configuration.Users.NewUsersAreActive = false + Rails.configuration.Users.AutoSetupNewUsers = false + + user = nil + token = nil + act_as_system_user do + user = User.create!(email: "bob@example.com", username: "bobby") + ap = ApiClientAuthorization.create!(user: user, api_client_id: 0) + token = ap.api_token + end + + post("/arvados/v1/users/setup", + params: {uuid: user['uuid']}, + headers: auth(:admin)) + assert_response :success + + post("/arvados/v1/users/#{user['uuid']}/activate", + params: {}, + headers: {"HTTP_AUTHORIZATION" => "Bearer #{token}"}) + assert_response 403 + assert_match(/Cannot activate without user agreements/, json_response['errors'][0]) + + post("/arvados/v1/user_agreements/sign", + params: {uuid: 'zzzzz-4zz18-t68oksiu9m80s4y'}, + headers: {"HTTP_AUTHORIZATION" => "Bearer #{token}"}) + assert_response :success + + post("/arvados/v1/users/#{user['uuid']}/activate", + params: {}, + headers: {"HTTP_AUTHORIZATION" => "Bearer #{token}"}) + assert_response :success + + get("/arvados/v1/users/#{user['uuid']}", + params: {}, + headers: {"HTTP_AUTHORIZATION" => "Bearer #{token}"}) + assert_response(:success) + user = json_response + assert_equal true, user['is_active'] + + post("/arvados/v1/users/#{user['uuid']}/unsetup", + params: {}, + headers: auth(:admin)) + assert_response :success + + get("/arvados/v1/users/#{user['uuid']}", + params: {}, + headers: {"HTTP_AUTHORIZATION" => "Bearer #{token}"}) + assert_response(:success) + user = json_response + assert_equal false, user['is_active'] + + post("/arvados/v1/users/#{user['uuid']}/activate", + params: {}, + headers: {"HTTP_AUTHORIZATION" => "Bearer #{token}"}) + assert_response 422 + assert_match(/Cannot activate without being invited/, json_response['errors'][0]) + end + + end diff --git a/services/api/test/unit/api_client_authorization_test.rb b/services/api/test/unit/api_client_authorization_test.rb index c390a02c04..fb90418b84 100644 --- a/services/api/test/unit/api_client_authorization_test.rb +++ b/services/api/test/unit/api_client_authorization_test.rb @@ -26,4 +26,37 @@ class ApiClientAuthorizationTest < ActiveSupport::TestCase assert_empty ApiClientAuthorization.where(uuid: api_client_authorizations(:expired).uuid) end + test "accepts SystemRootToken" do + assert_nil ApiClientAuthorization.validate(token: "xxxSystemRootTokenxxx") + + # will create a new ApiClientAuthorization record + Rails.configuration.SystemRootToken = "xxxSystemRootTokenxxx" + + auth = ApiClientAuthorization.validate(token: "xxxSystemRootTokenxxx") + assert_equal "xxxSystemRootTokenxxx", auth.api_token + assert_equal User.find_by_uuid(system_user_uuid).id, auth.user_id + assert auth.api_client.is_trusted + + # now change the token and try to use the old one first + Rails.configuration.SystemRootToken = "newxxxSystemRootTokenxxx" + + # old token will fail + assert_nil ApiClientAuthorization.validate(token: "xxxSystemRootTokenxxx") + # new token will work + auth = ApiClientAuthorization.validate(token: "newxxxSystemRootTokenxxx") + assert_equal "newxxxSystemRootTokenxxx", auth.api_token + assert_equal User.find_by_uuid(system_user_uuid).id, auth.user_id + + # now change the token again and use the new one first + Rails.configuration.SystemRootToken = "new2xxxSystemRootTokenxxx" + + # new token will work + auth = ApiClientAuthorization.validate(token: "new2xxxSystemRootTokenxxx") + assert_equal "new2xxxSystemRootTokenxxx", auth.api_token + assert_equal User.find_by_uuid(system_user_uuid).id, auth.user_id + # old token will fail + assert_nil ApiClientAuthorization.validate(token: "newxxxSystemRootTokenxxx") + end + + end diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb index adc37cc595..3480e55318 100644 --- a/services/api/test/unit/user_test.rb +++ b/services/api/test/unit/user_test.rb @@ -458,14 +458,6 @@ class UserTest < ActiveSupport::TestCase resp_user = find_obj_in_resp response, 'User' verify_user resp_user, email - oid_login_perm = find_obj_in_resp response, 'Link', 'arvados#user' - - verify_link oid_login_perm, 'permission', 'can_login', resp_user[:email], - resp_user[:uuid] - - assert_equal openid_prefix, oid_login_perm[:properties]['identity_url_prefix'], - 'expected identity_url_prefix not found for oid_login_perm' - group_perm = find_obj_in_resp response, 'Link', 'arvados#group' verify_link group_perm, 'permission', 'can_read', resp_user[:uuid], nil @@ -503,14 +495,6 @@ class UserTest < ActiveSupport::TestCase resp_user = find_obj_in_resp response, 'User' verify_user resp_user, email - oid_login_perm = find_obj_in_resp response, 'Link', 'arvados#user' - - verify_link oid_login_perm, 'permission', 'can_login', resp_user[:email], - resp_user[:uuid] - - assert_equal openid_prefix, oid_login_perm[:properties]['identity_url_prefix'], - 'expected identity_url_prefix not found for oid_login_perm' - group_perm = find_obj_in_resp response, 'Link', 'arvados#group' verify_link group_perm, 'permission', 'can_read', resp_user[:uuid], nil @@ -535,12 +519,6 @@ class UserTest < ActiveSupport::TestCase resp_user = find_obj_in_resp response, 'User' verify_user resp_user, email - oid_login_perm = find_obj_in_resp response, 'Link', 'arvados#user' - verify_link oid_login_perm, 'permission', 'can_login', resp_user[:email], - resp_user[:uuid] - assert_equal openid_prefix, oid_login_perm[:properties]['identity_url_prefix'], - 'expected identity_url_prefix not found for oid_login_perm' - group_perm = find_obj_in_resp response, 'Link', 'arvados#group' verify_link group_perm, 'permission', 'can_read', resp_user[:uuid], nil @@ -646,9 +624,7 @@ class UserTest < ActiveSupport::TestCase verify_link_exists(Rails.configuration.Users.AutoSetupNewUsers || active, groups(:all_users).uuid, user.uuid, "permission", "can_read") - # Check for OID login link. - verify_link_exists(Rails.configuration.Users.AutoSetupNewUsers || active, - user.uuid, user.email, "permission", "can_login") + # Check for repository. if named_repo = (prior_repo or Repository.where(name: expect_repo_name).first) diff --git a/tools/arvbox/lib/arvbox/docker/service/workbench2/run-service b/tools/arvbox/lib/arvbox/docker/service/workbench2/run-service index 85c03399f7..e14704d71d 100755 --- a/tools/arvbox/lib/arvbox/docker/service/workbench2/run-service +++ b/tools/arvbox/lib/arvbox/docker/service/workbench2/run-service @@ -21,8 +21,8 @@ fi cat < /usr/src/workbench2/public/config.json { "API_HOST": "${localip}:${services[controller-ssl]}", - "VOCABULARY_URL": "vocabulary-example.json", - "FILE_VIEWERS_CONFIG_URL": "file-viewers-example.json" + "VOCABULARY_URL": "/vocabulary-example.json", + "FILE_VIEWERS_CONFIG_URL": "/file-viewers-example.json" } EOF diff --git a/tools/vocabulary-migrate/vocabulary-migrate.py b/tools/vocabulary-migrate/vocabulary-migrate.py new file mode 100644 index 0000000000..920344b320 --- /dev/null +++ b/tools/vocabulary-migrate/vocabulary-migrate.py @@ -0,0 +1,121 @@ +#!/usr/bin/env python +# +# Copyright (C) The Arvados Authors. All rights reserved. +# +# SPDX-License-Identifier: CC-BY-SA-3.0 + +import argparse +import copy +import json +import logging +import os +import sys + +import arvados +import arvados.util + +logger = logging.getLogger('arvados.vocabulary_migrate') +logger.setLevel(logging.INFO) + +class VocabularyError(Exception): + pass + +opts = argparse.ArgumentParser(add_help=False) +opts.add_argument('--vocabulary-file', type=str, metavar='PATH', required=True, + help=""" +Use vocabulary definition file at PATH for migration decisions. +""") +opts.add_argument('--dry-run', action='store_true', default=False, + help=""" +Don't actually migrate properties, but only check if any collection/project +should be migrated. +""") +opts.add_argument('--debug', action='store_true', default=False, + help=""" +Sets logging level to DEBUG. +""") +arg_parser = argparse.ArgumentParser( + description='Migrate collections & projects properties to the new vocabulary format.', + parents=[opts]) + +def parse_arguments(arguments): + args = arg_parser.parse_args(arguments) + if args.debug: + logger.setLevel(logging.DEBUG) + if not os.path.isfile(args.vocabulary_file): + arg_parser.error("{} doesn't exist or isn't a file.".format(args.vocabulary_file)) + return args + +def _label_to_id_mappings(data, obj_name): + result = {} + for obj_id, obj_data in data.items(): + for lbl in obj_data['labels']: + obj_lbl = lbl['label'] + if obj_lbl not in result: + result[obj_lbl] = obj_id + else: + raise VocabularyError('{} label "{}" for {} ID "{}" already seen at {} ID "{}".'.format(obj_name, obj_lbl, obj_name, obj_id, obj_name, result[obj_lbl])) + return result + +def key_labels_to_ids(vocab): + return _label_to_id_mappings(vocab['tags'], 'key') + +def value_labels_to_ids(vocab, key_id): + if key_id in vocab['tags'] and 'values' in vocab['tags'][key_id]: + return _label_to_id_mappings(vocab['tags'][key_id]['values'], 'value') + return {} + +def migrate_properties(properties, key_map, vocab): + result = {} + for k, v in properties.items(): + key = key_map.get(k, k) + value = value_labels_to_ids(vocab, key).get(v, v) + result[key] = value + return result + +def main(arguments=None): + args = parse_arguments(arguments) + vocab = None + with open(args.vocabulary_file, 'r') as f: + vocab = json.load(f) + arv = arvados.api('v1') + if 'tags' not in vocab or vocab['tags'] == {}: + logger.warning('Empty vocabulary file, exiting.') + return 1 + if not arv.users().current().execute()['is_admin']: + logger.error('Admin privileges required.') + return 1 + key_label_to_id_map = key_labels_to_ids(vocab) + migrated_counter = 0 + + for key_label in key_label_to_id_map: + logger.debug('Querying objects with property key "{}"'.format(key_label)) + for resource in [arv.collections(), arv.groups()]: + objs = arvados.util.list_all( + resource.list, + order=['created_at'], + select=['uuid', 'properties'], + filters=[['properties', 'exists', key_label]] + ) + for o in objs: + props = copy.copy(o['properties']) + migrated_props = migrate_properties(props, key_label_to_id_map, vocab) + if not args.dry_run: + logger.debug('Migrating {}: {} -> {}'.format(o['uuid'], props, migrated_props)) + arv.collections().update(uuid=o['uuid'], body={ + 'properties': migrated_props + }).execute() + else: + logger.info('Should migrate {}: {} -> {}'.format(o['uuid'], props, migrated_props)) + migrated_counter += 1 + if not args.dry_run and migrated_counter % 100 == 0: + logger.info('Migrating {} objects...'.format(migrated_counter)) + + if args.dry_run and migrated_counter == 0: + logger.info('Nothing to do.') + elif not args.dry_run: + logger.info('Done, total objects migrated: {}.'.format(migrated_counter)) + return 0 + +if __name__ == "__main__": + sys.exit(main()) \ No newline at end of file