feat: GitLab OIDC Provider #519
Conversation
|
|
| func (g *ProviderGitLab) Claims(ctx context.Context, exchange *oauth2.Token) (*Claims, error) { | ||
| tokenSource := oauth2.StaticTokenSource(exchange) | ||
| client := oauth2.NewClient(ctx, tokenSource) | ||
| req, err := http.NewRequest("GET", "https://gitlab.com/oauth/userinfo", nil) |
There was a problem hiding this comment.
should probably allow for the base gitlab url to be configurable
There was a problem hiding this comment.
Would it be possible to derive this url form the IssuerURL?
There was a problem hiding this comment.
@perryao I think it would make sense to allow custom gitlab urls here!
There was a problem hiding this comment.
Based on https://gitlab.com/.well-known/openid-configuration returning "issuer": "https://gitlab.com", I'm going to assume that I can just concatenate /oauth/userinfo to whatever the user puts into their kratos config for issuerUrl. Otherwise, I'll default to https://gitlab.com
perryao
force-pushed
the
feat/gitlab-selfservice
branch
from
df679d6
to
74ed3bd
Compare
|
@aeneasr reviewing CONTRIBUTING.md, it states
The history in strategy/oidc suggests I should prefix the commits with either Do you have a preference? |
|
Sorry for my super-late reply! Don't worry about the commit message, I'll squash merge it and pick an appropriate message :) |
There was a problem hiding this comment.
Thank you for the great work and sorry for the late review, the last few days were really stressful and I didn't have time to review all the PRs :)
This looks great, I've added a small addition. I think the last remaining point would be to document how to set up GitLab over here: https://www.ory.sh/kratos/docs/guides/sign-in-with-github-google-facebook-linkedin
Thank you! :)
perryao
force-pushed
the
feat/gitlab-selfservice
branch
from
8db9ebe
to
305305f
Compare
Co-authored-by: hackerman <3372410+aeneasr@users.noreply.github.com>
|
Could you fix the build issues please? :) |
aeneasr
added
the
stale
|
@aeneasr I'm sorry this took me so long to get back to. I merged in the latest from master to resolve conflicts and fixed the build issues. |
|
No worries - it's great that you are contributing :) I left another comment answering one of your questions in the PR |
|
Hey there, can you please enable "allow edits by maintainers"? I can then resolve conflicts & merge! |
|
@aeneasr I believe you should have that access. Let me know if that's not the case |
…vice # Conflicts: # docs/docs/self-service/flows/user-login-user-registration/openid-connect-social-sign-in-oauth2.mdx # go.sum
|
Yup, you're right - looks like the GitHub UI simply was unable to merge this :) |
Related issue
Would close #518
Proposed changes
Checklist
vulnerability. If this pull request addresses a security. vulnerability, I
confirm that I got green light (please contact
security@ory.sh) from the maintainers to push
the changes.
works.
Further comments