Dependency Proxy SSO access
🏘 Context
The Dependency Proxy is a pull-through-cache, allowing users to pull container images from DockerHub through GitLab.
Currently, users cannot access the Dependency Proxy if Group SAML is enabled for their group.
This MR updates the permissions to properly find the users SAML/SSO session when using the Dependency Proxy.
🔬 The details
The dependency proxy functionality checks user authorization using the declarative policy framework, just like most of the rest of GitLab (things like Ability.allowed?
and can?
). When dealing with group saml/sso, the EE::GroupPolicy
class checks the sso session here by using ::Gitlab::Auth::GroupSaml::SsoEnforcer
. The problem is that SsoEnforcer
expects a user session, but such a session is not persisted by CLIs like docker or Git. If access is prevented by SsoEnforcer
, all group permissions are disabled and the user's member access level in the group is set to NO_ACCESS
.
Fortunately, as I just mentioned, Git runs into the same problem and we have already solved this problem for the Git CLI with ::Gitlab::Auth::GroupSaml::SessionEnforcer
. SessionEnforcer
will take a user and find whether or not they have a currently active session, in other words, checking that they are logged in via SSO.
In the case of the dependency proxy, we are able to authenticate the user using standard credentials or personal access tokens, so we can look up their sessions using SessionEnforcer
. Thus, by updating the dependency proxy permissions to check against SessionEnforcer
, we can enable SSO access for the dependency proxy.
📣 What does this MR do?
- Updates the group policy to find a user session using
GroupSaml::SessionEnforcer
instead ofGroupSaml::SsoEnforcer
when handling client based api requests (in this case the dependency proxy).
📸 Screenshots or Screencasts (strongly suggested)
Before change - Unable to pull through group with enforced SSO enabled
→ docker pull gdk.test:3443/dp-test/dependency_proxy/containers/alpine:latest
Error response from daemon: error parsing HTTP 404 response body: unexpected end of JSON input: ""
With change - Able to pull through group with enforced SSO enabled
→ docker pull gdk.test:3443/dp-test/dependency_proxy/containers/alpine:latest
latest: Pulling from dp-test/dependency_proxy/containers/alpine
Digest: sha256:1775bebec23e1f3ce486989bfc9ff3c4e951690df84aa9f926497d82f2ffca9d
Status: Image is up to date for gdk.test:3443/dp-test/dependency_proxy/containers/alpine:latest
gdk.test:3443/dp-test/dependency_proxy/containers/alpine:latest
💻 How to setup and validate locally (strongly suggested)
-
Set up your GDK with the Dependency Proxy
-
Set up your GDK to enable group SAML
-
Create private group
-
Log in a new user to the group using the local saml provider (username is
user1
password isuser1pass
) -
Ensure that user is a reporter or higher for the group
-
Create a personal access token for that user
-
Log in to the dependency proxy using the PAT:
# use your localhost host name and ssh port docker login gdk.test:3443 username: <username> password: <personal_access_token>
-
Pull an image through the dependency proxy for your group:
docker pull gdk.test:3443/<full_group_path>/dependency_proxy/containers/alpine:latest
-
The image should pull successfully.
☑ Does this MR meet the acceptance criteria?
Conformity
-
I have included changelog trailers, or none are needed. (Does this MR need a changelog?) -
I have added/updated documentation, or it's not needed. (Is documentation required?) -
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) - [-] I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?)
-
I have self-reviewed this MR per code review guidelines. -
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) -
I have followed the style guides. -
This change is backwards compatible across updates, or this does not apply.
Availability and Testing
-
I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) - [-] I have tested this MR in all supported browsers, or it's not needed.
- [-] I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.
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 #294018 (closed)