The Wayback Machine - https://web.archive.org/web/20200916022131/https://github.com/ory/kratos/pull/519
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: GitLab OIDC Provider #519

Open
wants to merge 16 commits into
base: master
from
Open

Conversation

@perryao
Copy link

perryao commented Jun 17, 2020

Related issue

Would close #518

Proposed changes

  • Adds provider_gitlab.go and updates the configuration schema to allow for gitlab as an OIDC provider

Checklist

  • I have read the contributing guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    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.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

Further comments

@CLAassistant
Copy link

CLAassistant commented Jun 17, 2020

CLA assistant check
All committers have signed the CLA.

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)

This comment has been minimized.

@perryao

perryao Jun 17, 2020

Author

should probably allow for the base gitlab url to be configurable

This comment has been minimized.

@zepatrik

zepatrik Aug 3, 2020

Member

Would it be possible to derive this url form the IssuerURL?

This comment has been minimized.

@aeneasr

aeneasr Aug 3, 2020

Member

@perryao I think it would make sense to allow custom gitlab urls here!

This comment has been minimized.

@perryao

perryao Aug 10, 2020

Author

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 perryao changed the title Feat/gitlab selfservice feat: GitLab OIDC Provider Jun 17, 2020
@perryao perryao force-pushed the perryao:feat/gitlab-selfservice branch from df679d6 to 74ed3bd Jun 20, 2020
@perryao
Copy link
Author

perryao commented Jun 20, 2020

@aeneasr reviewing CONTRIBUTING.md, it states

Ensure that each commit has a subsystem prefix (ex: controller:).

The history in strategy/oidc suggests I should prefix the commits with either ss: or maybe feat(oidc-gitlab).

Do you have a preference?

@aeneasr
Copy link
Member

aeneasr commented Jun 26, 2020

Sorry for my super-late reply! Don't worry about the commit message, I'll squash merge it and pick an appropriate message :)

Copy link
Member

aeneasr left a 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! :)

selfservice/strategy/oidc/provider_gitlab.go Outdated Show resolved Hide resolved
@perryao perryao force-pushed the perryao:feat/gitlab-selfservice branch from 8db9ebe to 305305f Jun 26, 2020
@perryao perryao marked this pull request as ready for review Jun 27, 2020
@aeneasr
Copy link
Member

aeneasr commented Jun 30, 2020

Could you fix the build issues please? :)

@aeneasr aeneasr added the stale label Jul 27, 2020
@perryao
Copy link
Author

perryao commented Aug 2, 2020

@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.

@perryao perryao requested a review from aeneasr Aug 2, 2020
@aeneasr
Copy link
Member

aeneasr commented Aug 3, 2020

No worries - it's great that you are contributing :) I left another comment answering one of your questions in the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.