Skip to content
Snippets Groups Projects

Deploy token access for the dependency proxy

Merged Steve Abrams requested to merge 280586-dependency-proxy-deploy-tokens into master
All threads resolved!

:deciduous_tree: What are we doing?

The dependency proxy allows users to pull container images from DockerHub through GitLab, caching the images in the process so the cached image can be more easily pulled in the future. In other words, the dependency proxy is a pull through cache.

The dependency proxy acts at the group-level, meaning, you turn it on for your group or subgroup, and then pull images using a URL with the group path, for example docker pull gitlab.com/gitlab-org/dependency_proxy/containers/alpine:latest. Since we are storing files on behalf of the group, this feature requires authentication to be able to be used. Currently, users can use their username/password, or a personal access token (PAT) to authenticate:

→ docker login gitlab.com
Username: sabrams
Password: <personal_access_token>
Login Succeeded

This MR adds the ability for group deploy tokens to be used to login.

:computer: Technical Context

Adding the ability to authenticate with deploy tokens is unfortunately not a trivial change. The authentication flow for the dependency proxy is different from that of the main site and the API. When logging in, the docker client first makes a request for a JWT via jwt_controller. This controller then uses DependencyProxyAuthenticationService to generate and return a token. For PATs, the token contains the user_id. This allows us to simply "sign in" the user when we receive subsequent requests for images. The problem with deploy tokens is we don't have a user to sign in.

The dependency proxy routes for downloading an image are handled by the Groups::DependencyProxyForContainersController.

This controller inherits from Groups::ApplicationController, which inherits from ApplicationController. These inherited controllers are built around the idea that we are accessing the UI. They have many callbacks for things like loading the current_user profile, settings, etc, all of which we don't need. Secondly, the Groups::ApplicationController finds and authorizes the group using the RoutableActions module. This module expects current_user to be defined, but a Deploy Token is not a valid current_user. We could brute force some of this to make deploy tokens work, but that results in about a half dozen callbacks being skipped, and @current_user being set to a Deploy Token, which is getting too messy and hacky.

So if the ApplicationControllers weren't built for Deploy Tokens, what can we do? :thinking:

The API allows deploy tokens, but that authentication flow is completely separate from the standard rails controllers. The Git HTTP endpoints, however, do use regular controllers and also allow deploy tokens! :tada: It turns out there is a separate authentication/authorization workflow for those controllers, and that is the pattern I have decided to implement here.

So, with that all out of the way, let's talk about what is changing and why.

:mag_right: What does this MR do?

  1. We move the DependencyProxy::Auth concern into Groups::DependencyProxy::ApplicationController (see this thread. Given the way this concern is starting to look, it makes more sense to have it as a parent controller for the dependency proxy controllers.

  2. Next, we change the Groups::DependencyProxyForContainersController to inherit from ApplicationController (through the new Groups::DependencyProxy::ApplicationController). It turns out all of those callbacks we had to skip automatically get skipped when there is no current_user value. We can no longer inherit from the Groups::ApplicationController, because, once again, the RoutableActions concern uses current_user to check the group permission. I considered updating that concern to handle deploy tokens, but that did not seem like a good solution. Looking into what the Groups::ApplicationController was actually supplying us with, the only useful item was setting the @group instance variable. So we now do that directly in the dependency proxy controller and handle the authorization in the existing DependencyProxy::Auth concern.

  3. If we look at how Repositories::GitHttpController works, it uses GitLab::Auth::Result to define an "actor", which is whatever credentialed entity has been authenticated (PAT, user, Deploy token, etc). The jwt_controller that I mentioned at the very beginning of this description also uses this same pattern to allow for various authentications against the container registry.

    We use some aliases for the "actor" so we have values for user and authenticated_user, which ApplicationController will then use in certain places to perform any necessary callbacks. By using this pattern, we are able to bypass Devise in the same way that GitHttpController does.

  4. We update the DependencyProxy::Auth concern to use GitLab::Auth::Result, signing in the user if we have a user or PAT. The DependencyProxy::GroupAccess concern checks for :read_dependency_proxy permission. You can see we use the auth_user value from the ApplicationController here so we are guaranteed to get whatever "actor" is logged in.

  5. We update the group_policy permissions. We check if the deploy token is valid, active, and has access to the given group. We also update the permissions for regular users and deploy tokens to being a minimum of reporter. This fixes this bug (internal only link) that I discovered while working through these changes.

A few more notes

  • You'll notice that some of the responses in the tests have changed. I think the new responses are more correct than the old ones were. For example, previously, if you attempted to login or pull an image using the wrong password, a 404 Not Found would be returned. But it makes much more sense to return a 401 Unauthorized so the user knows that their credentials did not work, so they did not even get to the point of finding a group/image.

  • Lastly, we add a feature flag. This MR has a lot of changes around authentication in a somewhat high traffic set of controllers. We don't want to risk any problems, so we set a feature flag around the various authentication logic. I've opted not to attempt to set a feature flag around the changing inherited class for two reasons: first, it would take a lot of additional code that starts to get hard to manage. Second, after following through the code, we really do only benefit from the before_action :group callback, which just finds and sets the group variable.

:camera_with_flash: Screenshots (strongly suggested)

Although we have test cases for the various combinations, I tested logging in and pulling images with all of the following settings both with the feature enabled and disabled.

Feature flag enabled

Group deploy token

Valid Group deploy token:


→ docker login gdk.test:3001
Username: grouptok
Password:
Login Succeeded
→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
latest: Pulling from dp-test/dependency_proxy/containers/alpine
Digest: sha256:1775bebec23e1f3ce486989bfc9ff3c4e951690df84aa9f926497d82f2ffca9d
Status: Downloaded newer image for gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest

Valid Group deploy token for a different group (public group in this test):


→ docker pull gdk.test:3001/dp-2/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""

Group deploy token with incorrect scopes:


→ docker login gdk.test:3001
Username: grouptok
Password:
Error response from daemon: Get http://gdk.test:3001/v2/: error parsing HTTP 403 response body: no error details found in HTTP response body: "{\"message\":\"access forbidden\",\"status\":\"error\",\"http_status\":403}"

Expired Group deploy token:


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
Error response from daemon: Head http://gdk.test:3001/v2/dp-test/dependency_proxy/containers/alpine/manifests/latest: unauthorized: HTTP Basic: Access denied

Revoked Group deploy token:


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
Error response from daemon: Head http://gdk.test:3001/v2/dp-test/dependency_proxy/containers/alpine/manifests/latest: unauthorized: HTTP Basic: Access denied
Project deploy token

→ docker login gdk.test:3001
Username: projecttok
Password:
Error response from daemon: Get http://gdk.test:3001/v2/: error parsing HTTP 403 response body: no error details found in HTTP response body: "{\"message\":\"access forbidden\",\"status\":\"error\",\"http_status\":403}"
Username and password

Public group where user is not a member:


→ docker login gdk.test:3001
Username: sabrams@gitlab.com
Password:
Login Succeeded
→ docker pull gdk.test:3001/dp-2/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""

Private group where user is not a member:


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""

Group where user is a guest:


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""

Group where user is a reporter:


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
latest: Pulling from dp-test/dependency_proxy/containers/alpine
5843afab3874: Pull complete
Digest: sha256:1775bebec23e1f3ce486989bfc9ff3c4e951690df84aa9f926497d82f2ffca9d
Status: Downloaded newer image for gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
Personal access token

Public group where user is not a member:


→ docker login gdk.test:3001
Username: sabrams@gitlab.com
Password:
Login Succeeded
→ docker pull gdk.test:3001/dp-2/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""

Private group where user is not a member:


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""

Group where user is a guest:


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""

Group where user is a reporter:


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
latest: Pulling from dp-test/dependency_proxy/containers/alpine
5843afab3874: Pull complete
Digest: sha256:1775bebec23e1f3ce486989bfc9ff3c4e951690df84aa9f926497d82f2ffca9d
Status: Downloaded newer image for gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest

Feature flag disabled

NOTE: some of these behaviors are not the same as on master due to permissions changes mentioned in point 4 of what does this MR do with regards to this bug. I have noted which specific scenarios have changed with **Changed**.

Group deploy token

→ docker login gdk.test:3001
Username: grouptok
Password:
Error response from daemon: login attempt to http://gdk.test:3001/v2/ failed with status: 404 Not Found
Project deploy token

→ docker login gdk.test:3001
Username: projecttok
Password:
Error response from daemon: Get http://gdk.test:3001/v2/: error parsing HTTP 403 response body: no error details found in HTTP response body: "{\"message\":\"access forbidden\",\"status\":\"error\",\"http_status\":403}"
Username and password

Public group where user is not a member: **Changed**


→ docker login gdk.test:3001
Username: sabrams@gitlab.com
Password:
Login Succeeded
→ docker pull gdk.test:3001/dp-2/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""

Private group where user is not a member:


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""

Group where user is a guest: **Changed**


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""

Group where user is a reporter:


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
latest: Pulling from dp-test/dependency_proxy/containers/alpine
5843afab3874: Pull complete
Digest: sha256:1775bebec23e1f3ce486989bfc9ff3c4e951690df84aa9f926497d82f2ffca9d
Status: Downloaded newer image for gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
Personal access token

Public group where user is not a member: **Changed**


→ docker login gdk.test:3001
Username: sabrams@gitlab.com
Password:
Login Succeeded
→ docker pull gdk.test:3001/dp-2/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""

Private group where user is not a member:


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""

Group where user is a guest: **Changed**


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""

Group where user is a reporter:


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
latest: Pulling from dp-test/dependency_proxy/containers/alpine
5843afab3874: Pull complete
Digest: sha256:1775bebec23e1f3ce486989bfc9ff3c4e951690df84aa9f926497d82f2ffca9d
Status: Downloaded newer image for gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest

Current behavior on master branch

I've included this area so we can compare the behaviors in the disabled feature flag section to prove we have updated the permissions for users/PATs to reporter and above.

Group deploy token

→ docker login gdk.test:3001
Username: grouptok
Password:
Error response from daemon: Get http://gdk.test:3001/v2/: error parsing HTTP 403 response body: no error details found in HTTP response body: "{\"message\":\"access forbidden\",\"status\":\"error\",\"http_status\":403}"
Project deploy token

→ docker login gdk.test:3001
Username: projecttok
Password:
Error response from daemon: Get http://gdk.test:3001/v2/: error parsing HTTP 403 response body: no error details found in HTTP response body: "{\"message\":\"access forbidden\",\"status\":\"error\",\"http_status\":403}"
Username and password

Public group where user is not a member:


→ docker login gdk.test:3001
Username: sabrams@gitlab.com
Password:
Login Succeeded
→ docker pull gdk.test:3001/dp-2/dependency_proxy/containers/alpine:latest
latest: Pulling from dp-2/dependency_proxy/containers/alpine
5843afab3874: Pull complete
Digest: sha256:1775bebec23e1f3ce486989bfc9ff3c4e951690df84aa9f926497d82f2ffca9d
Status: Downloaded newer image for gdk.test:3001/dp-2/dependency_proxy/containers/alpine:latest
gdk.test:3001/dp-2/dependency_proxy/containers/alpine:latest

Private group where user is not a member:


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""

Group where user is a guest:


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
latest: Pulling from dp-test/dependency_proxy/containers/alpine
5843afab3874: Pull complete
Digest: sha256:1775bebec23e1f3ce486989bfc9ff3c4e951690df84aa9f926497d82f2ffca9d
Status: Downloaded newer image for gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
Personal access token

Public group where user is not a member:


→ docker login gdk.test:3001
Username: sabrams@gitlab.com
Password:
Login Succeeded
→ docker pull gdk.test:3001/dp-2/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""

Private group where user is not a member:


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""

Group where user is a guest:


→ docker pull gdk.test:3001/dp-test/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""

:pencil: How to test this feature

  1. See this doc for instructions on how to use the dependency proxy with the GDK.
  2. Enable the feature in a rails console: Feature.enable(:dependency_proxy_deploy_tokens)
  3. Create a group (public or private).
  4. In the group go to Settings -> Repository and create a new deploy token with read_registry and write_registry scope.
  5. Login to the dependency proxy: docker login <your_localhost_with_port> enter your deploy token username and password
  6. Pull an image from the group: docker pull <your_localhost_with_port>/<group_path>/dependency_proxy/containers/alpine:latest

:triangular_ruler: Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Related to #280586 (closed)

Edited by Steve Abrams

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Steve Abrams
  • Steve Abrams
  • Steve Abrams
  • added workflowin review label and removed workflowin dev label

  • Steve Abrams added 1 commit

    added 1 commit

    • 0af9c178 - Deploy token access for the dependency proxy

    Compare with previous version

    • Author Developer
      Resolved by David Fernandez

      Hello @subashis would you please do the initial backend review? I would like @10io to do the maintainer review when it is ready since he has context/domain expertise in this area. Please don't hesitate to ask any questions if any of the description or changes are confusing.

  • Steve Abrams requested review from @subashis and @vdesousa

    requested review from @subashis and @vdesousa

  • Author Developer

    Broken pipeline looks to be unrelated, I will rebase later today/tomorrow.

  • Steve Abrams added 123 commits

    added 123 commits

    Compare with previous version

  • Steve Abrams added 1 commit

    added 1 commit

    • 844f4151 - Deploy token access for the dependency proxy

    Compare with previous version

  • Steve Abrams added 1 commit

    added 1 commit

    • bd4e96ef - Deploy token access for the dependency proxy

    Compare with previous version

  • Subashis Chakraborty approved this merge request

    approved this merge request

  • Subashis Chakraborty requested review from @10io

    requested review from @10io

    • Resolved by David Fernandez

      :wave: @subashis, thanks for approving this merge request.

      Please consider starting a new pipeline if:

      • This is the first time the merge request is approved, or
      • The merge request is ready to be merged, and there has not been a merge request pipeline in the last 2 hours.

      For more info, refer to the guideline.

  • mentioned in issue #301049 (closed)

  • David Fernandez
  • David Fernandez
  • David Fernandez removed review request for @10io

    removed review request for @10io

  • David Fernandez requested review from @ngaskill

    requested review from @ngaskill

  • Nick Gaskill approved this merge request

    approved this merge request

    • Resolved by Nick Gaskill

      :wave: @ngaskill, thanks for approving this merge request.

      Please consider starting a new pipeline if:

      • This is the first time the merge request is approved, or
      • The merge request is ready to be merged, and there has not been a merge request pipeline in the last 2 hours.

      For more info, refer to the guideline.

  • Steve Abrams added 1 commit

    added 1 commit

    Compare with previous version

  • Steve Abrams added 591 commits

    added 591 commits

    Compare with previous version

  • Steve Abrams added 1 commit

    added 1 commit

    Compare with previous version

  • Steve Abrams added 1 commit

    added 1 commit

    • 5096abe7 - Move DependencyProxy::Auth concern to controller

    Compare with previous version

  • Steve Abrams added 1 commit

    added 1 commit

    • b174f61c - Move DependencyProxy::Auth concern to controller

    Compare with previous version

  • Steve Abrams added 1 commit

    added 1 commit

    • 296f367e - Move DependencyProxy::Auth concern to controller

    Compare with previous version

  • Steve Abrams changed the description

    changed the description

  • Steve Abrams added 1 commit

    added 1 commit

    • 51c82236 - Move DependencyProxy::Auth concern to controller

    Compare with previous version

  • mentioned in issue #337264 (closed)

  • Steve Abrams requested review from @10io

    requested review from @10io

  • Vitor Meireles De Sousa approved this merge request

    approved this merge request

  • David Fernandez
  • David Fernandez removed review request for @10io

    removed review request for @10io

  • Steve Abrams added 1 commit

    added 1 commit

    Compare with previous version

  • Steve Abrams requested review from @10io

    requested review from @10io

  • David Fernandez removed review request for @10io

    removed review request for @10io

  • Steve Abrams added 1 commit

    added 1 commit

    • 26d09fae - Deploy token access for the dependency proxy

    Compare with previous version

  • Steve Abrams added 456 commits

    added 456 commits

    Compare with previous version

  • Steve Abrams requested review from @10io

    requested review from @10io

  • Steve Abrams mentioned in merge request !67373 (merged)

    mentioned in merge request !67373 (merged)

  • mentioned in issue #294018 (closed)

  • David Fernandez approved this merge request

    approved this merge request

  • David Fernandez resolved all threads

    resolved all threads

  • David Fernandez enabled an automatic merge when the pipeline for 11f5d38a succeeds

    enabled an automatic merge when the pipeline for 11f5d38a succeeds

  • David Fernandez mentioned in commit 0956ef50

    mentioned in commit 0956ef50

  • added workflowstaging label and removed workflowin review label

  • added workflowcanary label and removed workflowstaging label

  • added workflowproduction label and removed workflowcanary label

  • mentioned in issue #337875 (closed)

  • Congratulations :tada: @sabrams, your Issue/Merge Request has been awarded! (Learn more about the Security Awards Program)

  • mentioned in issue #344624 (closed)

  • Rémy Coutable mentioned in issue #332411

    mentioned in issue #332411

  • Please register or sign in to reply
    Loading