From: Peter Amstutz Date: Fri, 22 Nov 2019 15:37:02 +0000 (-0500) Subject: Activate new users created on the login cluster by federation-migrate X-Git-Tag: 2.0.0~114 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/ed5be18ec61d2edecbf878785633aea2b056b20a?ds=inline Activate new users created on the login cluster by federation-migrate Users are activated if they were active on their original cluster. Update check script part of federation-migrate test to check is_active. Also add assertions to permission test "users with bidirectional read permission in group can see each other" to explictly check that the other user appears in user's user listing. no issue # Arvados-DCO-1.1-Signed-off-by: Peter Amstutz --- diff --git a/sdk/python/arvados/commands/federation_migrate.py b/sdk/python/arvados/commands/federation_migrate.py index 885d6fda03..e74d6215c7 100755 --- a/sdk/python/arvados/commands/federation_migrate.py +++ b/sdk/python/arvados/commands/federation_migrate.py @@ -197,14 +197,17 @@ def choose_new_user(args, by_email, email, userhome, username, old_user_uuid, cl return None print("(%s) No user listed with same email to migrate %s to %s, will create new user with username '%s'" % (email, old_user_uuid, userhome, username)) if not args.dry_run: + oldhomecluster = old_user_uuid[0:5] + oldhomearv = clusters[oldhomecluster] newhomecluster = userhome[0:5] homearv = clusters[userhome] user = None try: + olduser = oldhomearv.users().get(uuid=old_user_uuid).execute() conflicts = homearv.users().list(filters=[["username", "=", username]]).execute() if conflicts["items"]: homearv.users().update(uuid=conflicts["items"][0]["uuid"], body={"user": {"username": username+"migrate"}}).execute() - user = homearv.users().create(body={"user": {"email": email, "username": username}}).execute() + user = homearv.users().create(body={"user": {"email": email, "username": username, "is_active": olduser["is_active"]}}).execute() except arvados.errors.ApiError as e: print("(%s) Could not create user: %s" % (email, str(e))) return None diff --git a/sdk/python/tests/fed-migrate/check.py b/sdk/python/tests/fed-migrate/check.py index 8f494be2fb..85d2d31f23 100644 --- a/sdk/python/tests/fed-migrate/check.py +++ b/sdk/python/tests/fed-migrate/check.py @@ -8,6 +8,10 @@ apiA = arvados.api(host=j["arvados_api_hosts"][0], token=j["superuser_tokens"][0 apiB = arvados.api(host=j["arvados_api_hosts"][1], token=j["superuser_tokens"][1], insecure=True) apiC = arvados.api(host=j["arvados_api_hosts"][2], token=j["superuser_tokens"][2], insecure=True) +### +### Check users on API server "A" (the LoginCluster) ### +### + users = apiA.users().list().execute() assert len(users["items"]) == 11 @@ -22,6 +26,15 @@ for i in range(1, 10): by_username[u["username"]] = u["uuid"] assert found +# Should be active +for i in (1, 2, 3, 4, 5, 6, 7, 8): + found = False + for u in users["items"]: + if u["username"] == ("case%d" % i) and u["email"] == ("case%d@test" % i) and u["is_active"] is True: + found = True + assert found, "Not found case%i" % i + +# case9 should not be active found = False for u in users["items"]: if (u["username"] == "case9" and u["email"] == "case9@test" and @@ -29,23 +42,40 @@ for u in users["items"]: found = True assert found + +### +### Check users on API server "B" (federation member) ### +### users = apiB.users().list().execute() assert len(users["items"]) == 11 -for i in range(2, 10): +for i in range(2, 9): found = False for u in users["items"]: - if u["username"] == ("case%d" % i) and u["email"] == ("case%d@test" % i) and u["uuid"] == by_username[u["username"]]: + if (u["username"] == ("case%d" % i) and u["email"] == ("case%d@test" % i) and + u["uuid"] == by_username[u["username"]] and u["is_active"] is True): found = True - assert found + assert found, "Not found case%i" % i + +found = False +for u in users["items"]: + if (u["username"] == "case9" and u["email"] == "case9@test" and + u["uuid"] == by_username[u["username"]] and u["is_active"] is False): + found = True +assert found + +### +### Check users on API server "C" (federation member) ### +### users = apiC.users().list().execute() assert len(users["items"]) == 8 for i in (2, 4, 6, 7, 8): found = False for u in users["items"]: - if u["username"] == ("case%d" % i) and u["email"] == ("case%d@test" % i) and u["uuid"] == by_username[u["username"]]: + if (u["username"] == ("case%d" % i) and u["email"] == ("case%d@test" % i) and + u["uuid"] == by_username[u["username"]] and u["is_active"] is True): found = True assert found @@ -54,7 +84,8 @@ for i in (2, 4, 6, 7, 8): for i in (3, 5, 9): found = False for u in users["items"]: - if u["username"] == ("case%d" % i) and u["email"] == ("case%d@test" % i) and u["uuid"] == by_username[u["username"]]: + if (u["username"] == ("case%d" % i) and u["email"] == ("case%d@test" % i) and + u["uuid"] == by_username[u["username"]] and u["is_active"] is True): found = True assert not found diff --git a/services/api/test/unit/permission_test.rb b/services/api/test/unit/permission_test.rb index 275d2a651b..18d2fbbcb5 100644 --- a/services/api/test/unit/permission_test.rb +++ b/services/api/test/unit/permission_test.rb @@ -287,6 +287,12 @@ class PermissionTest < ActiveSupport::TestCase a = create :active_user, first_name: "A" b = create :active_user, first_name: "B" other = create :active_user, first_name: "OTHER" + + assert_empty(User.readable_by(b).where(uuid: a.uuid), + "#{b.first_name} should not be able to see 'a' in the user list") + assert_empty(User.readable_by(a).where(uuid: b.uuid), + "#{a.first_name} should not be able to see 'b' in the user list") + act_as_system_user do g = create :group [a,b].each do |u| @@ -296,6 +302,12 @@ class PermissionTest < ActiveSupport::TestCase name: 'can_read', head_uuid: u.uuid, tail_uuid: g.uuid) end end + + assert_not_empty(User.readable_by(b).where(uuid: a.uuid), + "#{b.first_name} should be able to see 'a' in the user list") + assert_not_empty(User.readable_by(a).where(uuid: b.uuid), + "#{a.first_name} should be able to see 'b' in the user list") + a_specimen = act_as_user a do Specimen.create! end