Coder.com•2y ago

OIDC with Gitlab

Even with https://github.com/coder/coder/pull/5507 merged have no success with gitlab as OIDC provider. If I put a debug print of the claims data here: https://github.com/coder/coder/blob/main/coderd/userauth.go#L253 I something like this:

{"request_id": "b4bdb687-ebc3-4417-859a-30a801a14900", "Username": {"aud": "d5d9aadca67c0f49b2b7184c1904f2b602ed658f7902183f42fc36b94f945847", "auth_time": 1673612004, "email": "[email protected]", "email_verified": true, "exp": 1673882335, "groups_direct": ["aaa", "bbb", "ccc"], "iat": 1673882215, "iss": "https://gitlab.xxx.de", "sub": "7", "sub_legacy": "46e97dbbd9e7fd062ff2b6413072f0f634652eef58735409ea59638889c4e804"}}

{"request_id": "b4bdb687-ebc3-4417-859a-30a801a14900", "Username": {"aud": "d5d9aadca67c0f49b2b7184c1904f2b602ed658f7902183f42fc36b94f945847", "auth_time": 1673612004, "email": "[email protected]", "email_verified": true, "exp": 1673882335, "groups_direct": ["aaa", "bbb", "ccc"], "iat": 1673882215, "iss": "https://gitlab.xxx.de", "sub": "7", "sub_legacy": "46e97dbbd9e7fd062ff2b6413072f0f634652eef58735409ea59638889c4e804"}}

The nickname is completely missing. Has anyone an idea what I do wrong?

GitHub

feat: allow configurable username claim field in OIDC by janLo · Pu...

Gitlab does not set the preferred_username field. Therefore, coder generates something from the user's email address, which is not very helpful. This allows the administrator to change the fiel...

GitHub

coder/userauth.go at main · coder/coder

A tool that provisions remote development environments via Terraform - coder/userauth.go at main · coder/coder

32 Replies

JanLOP•2y ago

By default all custom claims are only returned from the UserInfo endpoint and not included in the ID token. You can optionally pass a response: keyword with one or both of the symbols :id_token or :user_info to specify where the claim should be returned.

JanLOP•2y ago

https://github.com/doorkeeper-gem/doorkeeper-openid_connect#claims

GitHub

GitHub - doorkeeper-gem/doorkeeper-openid_connect: OpenID Connect e...

OpenID Connect extension for Doorkeeper. Contribute to doorkeeper-gem/doorkeeper-openid_connect development by creating an account on GitHub.

JanLOP•2y ago

could this be teh reason? Can I somehow "fix" this? (on the coder side)

kyle•2y ago

Hmm, interesting. I'm not exactly sure how we'd go about fixing it. To be certain I'm following, you need the "nickname" claim to appear for a username, but it seems to only pass that if special parameters are passed to the request?

JanLOP•2y ago

No, you need to request this from the user info endpoint after fetching the id token. https://pkg.go.dev/github.com/coreos/go-oidc/v3/oidc#Provider.UserInfo I can try to implement it myself but might need some help with the unittests (client mocking) as I'm not a regular go developer. I'll in parallel open an issue at gitlab to provide basic information within the id token maybe. Not sure it was a deliberate decision on their side to only make this available within the user info ep

kyle•2y ago

I see! Is id_token returned?

JanLOP•2y ago

Yes The id token is apparently the only MUST part in the standard

kyle•2y ago

Ahh, gotcha. So if there are no claims, then we should be executing the Provider.UserInfo call?

JanLOP•2y ago

exactly

JanLOP•2y ago

This does the trick.

oidc-gitlab.patch

JanLOP•2y ago

The only thing is - now the unittests need to be fixed and I have no capacity in at the moment to understand the http client mocking

kyle•2y ago

Neato. I'll put up a fix right away and we can do a release for you!

JanLOP•2y ago

Shall I create a PR anyway?

kyle•2y ago

Thanks for investigating 🙂 Nah, I'll just steal your patch. I'll mention your GH username @JanLo

JanLOP•2y ago

thank you!

kyle•2y ago

I'm going to do this in a bit of a different way. My code is going to assume that the id_token has zero claims attached - is that a fair assumption?

JanLOP•2y ago

I cannot really tell if that is always true, the OIDC standard says, it can be in either or. For gitlab this is true - but this is only one provider

kyle•2y ago

But in this specific case is it true?

JanLOP•2y ago

This specification defines a set of standard Claims. They can be requested to be returned either in the UserInfo Response, per Section 5.3.2, or in the ID Token, per Section 2.

ah, you mean, fetch user info if no claims are attached?

kyle•2y ago

Yup! Current code:

    if len(claims) == 0 {
        userInfo, err := api.OIDCConfig.Provider.UserInfo(ctx, oauth2.StaticTokenSource(state.Token))
        if err != nil {
            httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
                Message: "Failed to obtain user information claims.",
                Detail:  "The OIDC provider returned no claims as part of the `id_token`. The attempt to fetch claims via the UserInfo endpoint failed: " + err.Error(),
            })
            return
        }
        err = userInfo.Claims(&claims)
        if err != nil {
            httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
                Message: "Failed to unmarshal user info claims.",
                Detail:  err.Error(),
            })
            return
        }
    }

    if len(claims) == 0 {
        userInfo, err := api.OIDCConfig.Provider.UserInfo(ctx, oauth2.StaticTokenSource(state.Token))
        if err != nil {
            httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
                Message: "Failed to obtain user information claims.",
                Detail:  "The OIDC provider returned no claims as part of the `id_token`. The attempt to fetch claims via the UserInfo endpoint failed: " + err.Error(),
            })
            return
        }
        err = userInfo.Claims(&claims)
        if err != nil {
            httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
                Message: "Failed to unmarshal user info claims.",
                Detail:  err.Error(),
            })
            return
        }
    }

Not sure if it returns no claims though. If it returns some claims, then we have a wonky situation ;p

JanLOP•2y ago

that is not true, gitlab strange enough sends teh direct group memberships and the user email in the id_token claim

kyle•2y ago

Hmm, very unfortunate

JanLOP•2y ago

yes. I'm not sure if they created this situation deliberately or if it happened just because someone did not fully read the documentation of the library they used 😉 I'll definitely open an issue on the GL side For this case, I think it's acceptable to do this extra work on the coder side becuase this code is only hit if someone provided a valid oidc callback anyway - so I don't see much potential for dos

JanLOP•2y ago

https://gitlab.com/gitlab-org/gitlab/-/issues/388176

GitLab

Provide OIDC nickname/preferred_username claims in id_token (#38817...

The OIDC feature provides a lot of information as documented in https://docs.gitlab.com/ee/integration/openid_connect_provider.html#shared-information. I tried to use this with an external application...

kyle•2y ago

@JanL PR almost up!

kyle•2y ago

https://github.com/coder/coder/pull/5735

GitHub

fix: use UserInfo endpoint with OIDC by kylecarbs · Pull Request ...

This resolves a user issue surfaced in Discord: https://discord.com/channels/747933592273027093/1064566338875576361/1064566338875576361 Both methods of obtaining claims need to be used according to...

JanLOP•2y ago

Oh wow, thank you so much!

kyle•2y ago

@JanL can you build from main to try it out? Or do you need us to cut a release?

JanLOP•2y ago

I'll test it first thing tomorrow morning, I've turned my device off already. Is it alright to cherry-pick just this commit on top of the last release? Then I don't need a separate release for just this change.

kyle•2y ago

Yup, you should be able to cherry it! Let me know if there are any issues!

JanLOP•2y ago

👍 Sorry for the late feedback, I got distracted this morning. The patch works perfect. Thank you!

kyle•2y ago

Awesome!

Gaming

Programming

OIDC with Gitlab