feat(vcs): support for new VCS integration#554
Open
palkerecsenyi wants to merge 10 commits intoCERNDocumentServer:masterfrom
Open
feat(vcs): support for new VCS integration#554palkerecsenyi wants to merge 10 commits intoCERNDocumentServer:masterfrom
palkerecsenyi wants to merge 10 commits intoCERNDocumentServer:masterfrom
Conversation
12 tasks
16 tasks
f52e4c1 to
3d42ad5
Compare
93f0c16 to
cf12ab5
Compare
4fbe832 to
89ae57b
Compare
1c8c4ee to
a176cf4
Compare
0a04164 to
f2bad63
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #552
The new OAuth remote for GitLab will use the same system as all other remotes (i.e. being registered via
invenio-oauthclient). This will allow us to plug in custom handlers at various points. Notably, we can override the GitLab-specificaccount_info_serializerfunction and perform our own validation on the rawuserinfoobject, even before it gets turned into the generic object with significantly less information.GitLab returns an
identitiesfield in theuserinfo, containing a list of external authentication providers the user has linked to their account, and the ID each provider uses to represent the user.For example:
{ "id": 1234, "username": "johnsmith", "identities": [ { "provider": "openid_connect", "extern_uid": "johnsmith", "saml_provider_id": null, }, { "provider": "ldapmain", "extern_uid": "cn=johnsmith,ou=users,ou=organic units,dc=cern,dc=ch", "saml_provider_id": null, }, { "provider": "kerberos", "extern_uid": "johnsmith@CERN.CH", "saml_provider_id": null, }, ], ... }In this case, we can see the GitLab user
johnsmithhas a CERN username ofjohnsmith. It is possible for the GitLab username to be different to the CERN username, so we cannot simply rely on the GitLab username.We already store the user's CERN username in the database as part of the
extra_datafield for the CERN/keycloakRemoteAccount. We just have to look this up (based on the OAuth client ID of the CERN remote app) and compare the two values.This way, we ensure the user has logged in with the GitLab account associated with the same CERN account as their CDS account, which avoids some security issues and bugs.
The function
gitlab_account_info_serializeroverrides theaccount_info_serializer(but still makes a call to the original once validation has passed). This override can be enabled ininvenio.cfgwhich I will also update soon.Error handling
Right now, the error handling means a 500 Internal Server Error is returned when there's a mismatch between the user IDs. This could be an acceptable outcome, since such behaviour is not likely to happen without careful and deliberate interference with the authentication process. The following error is logged on the backend:
Since we don't have direct control of the view handler (that's in invenio-oauthclient) it's a little tricky to customise the error that's returned to the end user.
Testing
Running this locally is a little complicated due to the need to connect to various services.
Here's what you need:
hostname.cern.chinternally but you'll need a CA certificate from https://ca.cern.ch/ca/ ("New CERN Host certificate"). Make sure to set this URL in all the relevant places ininvenio.cfg