16263: Rename no_federation -> bypass_federation
authorPeter Amstutz <peter.amstutz@curii.com>
Tue, 31 Mar 2020 20:33:27 +0000 (16:33 -0400)
committerPeter Amstutz <peter.amstutz@curii.com>
Tue, 31 Mar 2020 20:33:27 +0000 (16:33 -0400)
Enforce if bypass_federation is true that user is admin.

Update API revision and make federation migrate check for it.

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz@curii.com>

lib/controller/federation/conn.go
sdk/go/arvados/api.go
sdk/python/arvados/commands/federation_migrate.py
sdk/python/tests/fed-migrate/check.py
services/api/app/controllers/application_controller.rb
services/api/app/controllers/arvados/v1/schema_controller.rb
services/api/app/controllers/arvados/v1/users_controller.rb

index 05ea7769f14267b5e74927b6173665bcce24d281..10e1bc5f136d9d9d0ea05df6c37ad7b7f7a0d91f 100644 (file)
@@ -427,7 +427,7 @@ func (conn *Conn) batchUpdateUsers(ctx context.Context,
 }
 
 func (conn *Conn) UserList(ctx context.Context, options arvados.ListOptions) (arvados.UserList, error) {
-       if id := conn.cluster.Login.LoginCluster; id != "" && id != conn.cluster.ClusterID && !options.NoFederation {
+       if id := conn.cluster.Login.LoginCluster; id != "" && id != conn.cluster.ClusterID && !options.BypassFederation {
                resp, err := conn.chooseBackend(id).UserList(ctx, options)
                if err != nil {
                        return resp, err
@@ -447,7 +447,7 @@ func (conn *Conn) UserCreate(ctx context.Context, options arvados.CreateOptions)
 }
 
 func (conn *Conn) UserUpdate(ctx context.Context, options arvados.UpdateOptions) (arvados.User, error) {
-       if options.NoFederation {
+       if options.BypassFederation {
                return conn.local.UserUpdate(ctx, options)
        }
        return conn.chooseBackend(options.UUID).UserUpdate(ctx, options)
index f3fdd254ffe99a5cd1110de920c4ca68aac41e50..e60108335163714717c2fa8c9d0ed7760c0bc303 100644 (file)
@@ -84,7 +84,7 @@ type ListOptions struct {
        Count              string                 `json:"count"`
        IncludeTrash       bool                   `json:"include_trash"`
        IncludeOldVersions bool                   `json:"include_old_versions"`
-       NoFederation       bool                   `json:"no_federation"`
+       BypassFederation   bool                   `json:"bypass_federation"`
 }
 
 type CreateOptions struct {
@@ -95,9 +95,9 @@ type CreateOptions struct {
 }
 
 type UpdateOptions struct {
-       UUID         string                 `json:"uuid"`
-       Attrs        map[string]interface{} `json:"attrs"`
-       NoFederation bool                   `json:"no_federation"`
+       UUID             string                 `json:"uuid"`
+       Attrs            map[string]interface{} `json:"attrs"`
+       BypassFederation bool                   `json:"bypass_federation"`
 }
 
 type UpdateUUIDOptions struct {
index 0eaf1c03e2bcf1148e97f43d438a5de01853d9a9..445775ccedcd1f4ef246297c22a17e470e0f0e94 100755 (executable)
@@ -66,8 +66,8 @@ def connect_clusters(args):
             errors.append("Inconsistent login cluster configuration, expected '%s' on %s but was '%s'" % (loginCluster, config["ClusterID"], config["Login"]["LoginCluster"]))
             continue
 
-        if arv._rootDesc["revision"] < "20190926":
-            errors.append("Arvados API server revision on cluster '%s' is too old, must be updated to at least Arvados 1.5 before running migration." % config["ClusterID"])
+        if arv._rootDesc["revision"] < "20200331":
+            errors.append("Arvados API server revision on cluster '%s' is too old, must be updated to at least Arvados 2.0.2 before running migration." % config["ClusterID"])
             continue
 
         try:
@@ -98,7 +98,7 @@ def fetch_users(clusters, loginCluster):
     users = []
     for c, arv in clusters.items():
         print("Getting user list from %s" % c)
-        ul = arvados.util.list_all(arv.users().list, no_federation=True)
+        ul = arvados.util.list_all(arv.users().list, bypass_federation=True)
         for l in ul:
             if l["uuid"].startswith(c):
                 users.append(l)
@@ -171,14 +171,14 @@ def update_username(args, email, user_uuid, username, migratecluster, migratearv
     print("(%s) Updating username of %s to '%s' on %s" % (email, user_uuid, username, migratecluster))
     if not args.dry_run:
         try:
-            conflicts = migratearv.users().list(filters=[["username", "=", username]], no_federation=True).execute()
+            conflicts = migratearv.users().list(filters=[["username", "=", username]], bypass_federation=True).execute()
             if conflicts["items"]:
                 # There's already a user with the username, move the old user out of the way
                 migratearv.users().update(uuid=conflicts["items"][0]["uuid"],
-                                          no_federation=True,
+                                          bypass_federation=True,
                                           body={"user": {"username": username+"migrate"}}).execute()
             migratearv.users().update(uuid=user_uuid,
-                                      no_federation=True,
+                                      bypass_federation=True,
                                       body={"user": {"username": username}}).execute()
         except arvados.errors.ApiError as e:
             print("(%s) Error updating username of %s to '%s' on %s: %s" % (email, user_uuid, username, migratecluster, e))
@@ -210,10 +210,10 @@ def choose_new_user(args, by_email, email, userhome, username, old_user_uuid, cl
             try:
                 olduser = oldhomearv.users().get(uuid=old_user_uuid).execute()
                 conflicts = homearv.users().list(filters=[["username", "=", username]],
-                                                 no_federation=True).execute()
+                                                 bypass_federation=True).execute()
                 if conflicts["items"]:
                     homearv.users().update(uuid=conflicts["items"][0]["uuid"],
-                                           no_federation=True,
+                                           bypass_federation=True,
                                            body={"user": {"username": username+"migrate"}}).execute()
                 user = homearv.users().create(body={"user": {"email": email, "username": username,
                                                              "is_active": olduser["is_active"]}}).execute()
@@ -250,7 +250,7 @@ def activate_remote_user(args, email, homearv, migratearv, old_user_uuid, new_us
         return None
 
     try:
-        findolduser = migratearv.users().list(filters=[["uuid", "=", old_user_uuid]], no_federation=True).execute()
+        findolduser = migratearv.users().list(filters=[["uuid", "=", old_user_uuid]], bypass_federation=True).execute()
         if len(findolduser["items"]) == 0:
             return False
         if len(findolduser["items"]) == 1:
@@ -280,7 +280,7 @@ def activate_remote_user(args, email, homearv, migratearv, old_user_uuid, new_us
         print("(%s) Activating user %s on %s" % (email, new_user_uuid, migratecluster))
         try:
             if not args.dry_run:
-                migratearv.users().update(uuid=new_user_uuid, no_federation=True,
+                migratearv.users().update(uuid=new_user_uuid, bypass_federation=True,
                                           body={"is_active": True}).execute()
         except arvados.errors.ApiError as e:
             print("(%s) Could not activate user %s on %s: %s" % (email, new_user_uuid, migratecluster, e))
index 85d2d31f2309fe3182fc1d606982d1bea34e41c1..8165b3eba73dd3ef4adf04083b039bc67bbb26c0 100644 (file)
@@ -12,7 +12,7 @@ apiC = arvados.api(host=j["arvados_api_hosts"][2], token=j["superuser_tokens"][2
 ### Check users on API server "A" (the LoginCluster) ###
 ###
 
-users = apiA.users().list().execute()
+users = apiA.users().list(bypass_federation=True).execute()
 
 assert len(users["items"]) == 11
 
@@ -46,7 +46,7 @@ assert found
 ###
 ### Check users on API server "B" (federation member) ###
 ###
-users = apiB.users().list().execute()
+users = apiB.users().list(bypass_federation=True).execute()
 assert len(users["items"]) == 11
 
 for i in range(2, 9):
@@ -68,7 +68,7 @@ assert found
 ###
 ### Check users on API server "C" (federation member) ###
 ###
-users = apiC.users().list().execute()
+users = apiC.users().list(bypass_federation=True).execute()
 assert len(users["items"]) == 8
 
 for i in (2, 4, 6, 7, 8):
index 68fa7d88015d3ff3b081c9a185e6db7aa5ba1b64..a3435d0b68387a6fe7edb26a4b767a583e7c2e29 100644 (file)
@@ -53,6 +53,7 @@ class ApplicationController < ActionController::Base
   before_action :reload_object_before_update, :only => :update
   before_action(:render_404_if_no_object,
                 except: [:index, :create] + ERROR_ACTIONS)
+  before_action :only_admin_can_bypass_federation
 
   attr_writer :resource_attrs
 
@@ -139,6 +140,12 @@ class ApplicationController < ActionController::Base
     render_not_found "Object not found" if !@object
   end
 
+  def only_admin_can_bypass_federation
+    if params[:bypass_federation] && current_user.nil? or !current_user.is_admin
+      send_error("The bypass_federation parameter is only permitted when current user is admin", status: 403)
+    end
+  end
+
   def render_error(e)
     logger.error e.inspect
     if e.respond_to? :backtrace and e.backtrace
@@ -656,7 +663,7 @@ class ApplicationController < ActionController::Base
         location: "query",
         required: false,
       },
-      no_federation: {
+      bypass_federation: {
         type: 'boolean',
         required: false,
         description: 'bypass federation behavior, list items from local instance database only'
index 5c223410151788926445ac440e20a723ce6cf9aa..b9aba2726f555883d304fac490f050b6177275b4 100644 (file)
@@ -33,10 +33,10 @@ class Arvados::V1::SchemaController < ApplicationController
         id: "arvados:v1",
         name: "arvados",
         version: "v1",
-        # format is YYYYMMDD, must be fixed with (needs to be linearly
+        # format is YYYYMMDD, must be fixed width (needs to be lexically
         # sortable), updated manually, may be used by clients to
         # determine availability of API server features.
-        revision: "20200212",
+        revision: "20200331",
         source_version: AppVersion.hash,
         sourceVersion: AppVersion.hash, # source_version should be deprecated in the future
         packageVersion: AppVersion.package_version,
index 475f6d54ad95fe63c64374211d063e6300235188..d9ab5556ffc9ac7826abda00bc18e3d4b700269c 100644 (file)
@@ -251,7 +251,7 @@ class Arvados::V1::UsersController < ApplicationController
 
   def self._update_requires_parameters
     super.merge({
-      no_federation: {
+      bypass_federation: {
         type: 'boolean', required: false,
       },
     })