Refactor dependency proxy authentication to pass the actual token to policy code

Context

In !136655 (merged) we needed to add authorization code that checks if a token has the required scopes to access a group's dependency proxy. Because we only have a handle to the user and not the token in the policy code, we had to implement the authorization check not in a code inside app/policies, but instead in a service called by the controller, where we have a handle to the token.

Adding dependency proxy policy code that checks if a personal access token has the required scopes is hard because we pass the token user, and not the token itself, to the can?(...) check. Given a token user, it is impossible to get the token because there's a 1 to many association between users and personal access tokens.

This is not the case with a deploy token. With deploy tokens, we pass the token itself. With a handle to the token, we can write policy code that checks if the token has the required scopes, like:

app/policies/group_policy.rb#L409

  def valid_dependency_proxy_deploy_token
    @user.is_a?(DeployToken) && @user&.valid_for_dependency_proxy? && @user&.has_access_to_group?(@subject)
  end

app/models/deploy_token.rb#L57

  def valid_for_dependency_proxy?
    group_type? &&
      active? &&
      (Gitlab::Auth::REGISTRY_SCOPES & scopes).size == Gitlab::Auth::REGISTRY_SCOPES.size
  end

We can refactor how things are passed around, so that the actual personal access token is what is passed to dependency proxy policy code, not the personal access token user.

Summary of Proposed Changes

To make it easier to review and test the code changes, I created a draft MR: !139484 (closed)

---

• Pass the token string to Auth::DependencyProxyAuthenticationService (already in !136655 (merged))
• Modify Auth::DependencyProxyAuthenticationService to include the PersonalAccessToken's token in the encoded JsonWebToken response

-        token['user_id'] = current_user.id if current_user
-        token['deploy_token'] = deploy_token.token if deploy_token
+        if current_user
+          token['user_id'] = current_user.id
+          token['personal_access_token'] = raw_token if raw_token
+        elsif deploy_token
+          token['deploy_token'] = deploy_token.token
+        end
+

• Modify DependencyProxy::AuthTokenService to extract the token string, and return the matching PersonalAccessToken. Rename #user_or_deploy_token_from_jwt to #token_from_jwt.

-      if token_payload['user_id']
-        User.find(token_payload['user_id'])
+      if token_payload['personal_access_token']
+        PersonalAccessTokensFinder.new(state: 'active').find_by_token(token_payload['personal_access_token'])

• Modify Groups::DependencyProxy::ApplicationController
  • use DependencyProxy::AuthTokenService#token_from_jwt
  • Remove this line: delegate :actor, to: :@authentication_result, allow_nil: true

-          user_or_deploy_token = ::DependencyProxy::AuthTokenService.user_or_deploy_token_from_jwt(token)
+          @token = ::DependencyProxy::AuthTokenService.token_from_jwt(token)
 
-          case user_or_deploy_token
-          when User
-            @authentication_result = Gitlab::Auth::Result.new(user_or_deploy_token, nil, :user, [])
-            sign_in(user_or_deploy_token) unless user_or_deploy_token.project_bot?
+          case @token
+          when PersonalAccessToken
+            @authentication_result = Gitlab::Auth::Result.new(@token.user, nil, :user, [])
+            sign_in(@token.user) unless @token.user.project_bot?
           when DeployToken
-            @authentication_result = Gitlab::Auth::Result.new(user_or_deploy_token, nil, :deploy_token, [])
+            @authentication_result = Gitlab::Auth::Result.new(@token, nil, :deploy_token, [])
           end

+      def authenticated_user
+        @token
+      end

With the above changes in place, we'll have the actual token (PersonalAccessToken or DeployToken) passed as auth_user in the can?(auth_user, :read_dependency_proxy, group) line in DependencyProxy::GroupAccess#authorize_read_dependency_proxy!. And that will allow us to modify the dependency proxy policy code to add scope checks, just like how we check deploy tokens.

A few more changes, so that PersonalAccessTokens will be handled properly in policy code:

• add include PolicyActor to the PersonalAccessToken model
• add this method to PersonalAccessToken (this is duplicated from the User model. Maybe extract into a concern?):

def can_admin_all_resources?
  can?(:admin_all_resources)
end

• refine how we check group access level. access_level(for_any_session: true) >= GroupMember::GUEST does not work because of this guard clause.

   def access_level(for_any_session: false)
     return GroupMember::NO_ACCESS if @user.nil?
-    return GroupMember::NO_ACCESS unless user_is_user?
 
-    @access_level ||= lookup_access_level!(for_any_session: for_any_session)
+    if user_is_user?
+      @access_level ||= lookup_access_level!(for_any_session: for_any_session)
+    elsif user.is_a?(PersonalAccessToken)
+      @access_level ||= lookup_access_level!(for_any_session: for_any_session, user: @user.user)
+    else
+      return GroupMember::NO_ACCESS
+    end
   end
 
-  def lookup_access_level!(for_any_session: false)
-    @subject.max_member_access_for_user(@user)
+  def lookup_access_level!(for_any_session: false, user: @user)
+    @subject.max_member_access_for_user(user)
   end

Improvements

Risks

Involved components

Optional: Intended side effects

Optional: Missing test coverage