[ARVADOS] updated: ffd4f2337be24dd318f761c70cd9e41280e76d8f

Git user git at public.curoverse.com
Sat Mar 12 17:32:24 EST 2016


Summary of changes:
 .../controllers/arvados/v1/api_client_authorizations_controller.rb  | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

  discards  be1c59b0135072dccbbbdb845a32783109e5d2e4 (commit)
       via  ffd4f2337be24dd318f761c70cd9e41280e76d8f (commit)

This update added new revisions after undoing existing revisions.  That is
to say, the old revision is not a strict subset of the new revision.  This
situation occurs when you --force push a change and generate a repository
containing something like this:

 * -- * -- B -- O -- O -- O (be1c59b0135072dccbbbdb845a32783109e5d2e4)
            \
             N -- N -- N (ffd4f2337be24dd318f761c70cd9e41280e76d8f)

When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit ffd4f2337be24dd318f761c70cd9e41280e76d8f
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat Mar 12 17:32:17 2016 -0500

    8079: Tidy up and document current_api_client_is_trusted.

diff --git a/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb b/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb
index cb232f0..0040ee0 100644
--- a/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb
+++ b/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb
@@ -74,30 +74,46 @@ class Arvados::V1::ApiClientAuthorizationsController < ApplicationController
   end
 
   def find_object_by_uuid
-    conditions = {
-      uuid: (params[:uuid] || params[:id]),
-      user_id: current_user.id,
-    }
-    unless Thread.current[:api_client].andand.is_trusted
-      conditions[:api_token] = current_api_client_authorization.andand.api_token
+    uuid_param = params[:uuid] || params[:id]
+    if (uuid_param != current_api_client_authorization.andand.uuid and
+        not Thread.current[:api_client].andand.is_trusted)
+      return forbidden
     end
-    @object = model_class.where(conditions).first
+    @limit = 1
+    @offset = 0
+    @orders = []
+    @where = {}
+    @filters = [['uuid', '=', uuid_param]]
+    find_objects_for_index
+    @object = @objects.first
   end
 
   def current_api_client_is_trusted
-    unless Thread.current[:api_client].andand.is_trusted
-      if %w[show update destroy].include? params['action']
-        if @object.andand['api_token'] == current_api_client_authorization.andand.api_token
-          return true
-        end
-      elsif params["action"] == "index" and @objects.andand.size == 1
-        filters = @filters.map{|f|f.first}.uniq
-        if [['uuid'], ['api_token']].include? filters
-          return true if @objects.first['api_token'] == current_api_client_authorization.andand.api_token
-        end
-      end
-      send_error('Forbidden: this API client cannot manipulate other clients\' access tokens.',
-                 status: 403)
+    if Thread.current[:api_client].andand.is_trusted
+      return true
+    end
+    # A non-trusted client can do a search for its own token if it
+    # explicitly restricts the search to its own UUID or api_token.
+    # Any other kind of query must return 403, even if it matches only
+    # the current token, because that's currently how Workbench knows
+    # (after searching on scopes) the difference between "the token
+    # I'm using now *is* the only sharing token for this collection"
+    # (403) and "my token is trusted, and there is one sharing token
+    # for this collection" (200).
+    #
+    # The @filters test here also prevents a non-trusted token from
+    # filtering on its own scopes, and discovering whether any _other_
+    # equally scoped tokens exist (403=yes, 200=no).
+    if (@objects.andand.count == 1 and
+        @objects.first.uuid == current_api_client_authorization.andand.uuid and
+        (@filters.map(&:first) & %w(uuid api_token)).any?)
+      return true
     end
+    forbidden
+  end
+
+  def forbidden
+    send_error('Forbidden: this API client cannot manipulate other clients\' access tokens.',
+               status: 403)
   end
 end
diff --git a/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb b/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb
index 1fb94ab..192e6b9 100644
--- a/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb
@@ -72,9 +72,9 @@ class Arvados::V1::ApiClientAuthorizationsControllerTest < ActionController::Tes
    [:admin, :admin, 200, 200, 1],
    [:active, :active, 200, 200, 1],
    # cannot look up other tokens (even for same user) if not trustedclient
-   [:admin, :active, 404, 403],
-   [:admin, :admin_vm, 404, 403],
-   [:active, :admin, 404, 403],
+   [:admin, :active, 403, 403],
+   [:admin, :admin_vm, 403, 403],
+   [:active, :admin, 403, 403],
    # cannot look up other tokens for other users, regardless of trustedclient
    [:admin_trustedclient, :active, 404, 200, 0],
    [:active_trustedclient, :admin, 404, 200, 0],

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list