-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add email to JWT claims #17530
Comments
Thanks for raising 🙏 We are actively looking into our OIDC support, and will take a look further into this @ChevronTango ! Hopefully we can move the current token implementation to a more visible beta status soon, and then we can look at modifying and adding additional claims as you suggested! |
Thanks @loujaybee. This is very exciting and I'm keen to help get it out ASAP. It unlocks a lot of potential for Gitpod, so if there's anything we can do to help do let us know. |
Thanks for the quick turnaround, on the pull request @csweichel ! Question: Do we not also need to proxy through the |
|
I think there might be hope still with this deployment 😄! Although I'm not familiar with the OIDC go package we are using, I poked around the documentation and it seems that type UserInfoSetter interface {
UserInfo
SetSubject(sub string)
UserInfoProfileSetter
SetEmail(email string, verified bool)
SetPhone(phone string, verified bool)
SetAddress(address UserInfoAddress)
AppendClaims(key string, values interface{})
} |
@ChevronTango should be good. Have a nice weekend :)! I think this will deploy on Monday, but running from sources it works. |
You are a star @filiptronicek, much appreciated... now we just need to get everything else working! |
|
I hate to be the downer but... {
"iss": "https://api.gitpod.io/idp",
"aud": [
"sigstore"
],
"azp": "sigstore",
"c_hash": "rwIyisNnRNIDTlLm1pbRGQ",
"exp": 1684860542,
"iat": 1684856942,
"auth_time": 1684856942,
"sub": "https://github.com/ChevronTango/fulcio",
"name": "ChevronTango",
"email": "[email protected]",
"email_verified": true
} that's not my email |
this raises a very good point, as we are going to be asking people to trust Gitpod and the email that it says the user is. Where is the email taken from? if a user has added additional integrations/providers, such as private GitLab or Bitbucket instances, do we trust those to provide the email correctly? I feel like only the main GitLab/GitHub IDP's can really be trusted. Does each account in Gitpod have a single verified email that cannot be changed? Where has that email come from? is it always from one of the main IDPs or could it come from a third party? Can you have different emails from different IDP's all linked to the same Gitpod account? |
Thanks for the report @ChevronTango, we'll take a look into the email issue. Regarding the questions about IDP source for the emails, we'll factor that into our upcoming documentation: https://github.com/gitpod-io/website/pull/3656. I'll try to remember to drop back here when we have that put together to see if it covers everything you mentioned above. 🙏 |
So I recon I know what happened with that email. I added an additional IDP to connect to a test GitLab instance I was playing with and that had a dummy email I added. I was testing a repo in my dummy GitLab instance using a dummy account but still wanted to use Gitpod to test it. My worry is that this exposes a dangerous attack vector with the new email IDP. If I can create a new integration in Gitpod with any email in that I like and associate it with my account, I can potentially impersonate any user/email that I like. The only way we can realistically use this is if the provider you choose to use for the primary email matches the provider for the currently open workspace, and also if that is one of the built in integrations and not a user added one. The current approach of just cycling through all the providers in an account isn't robust enough, especially if we want people to trust us to have validated this properly. |
@ChevronTango we were chatting about the attack vector you're mentioning here yesterday. We should never trust self-hosted GitLab instances with giving us e-mail addresses we expose. I think it's ok-enough to trust https://github.com and https://gitlab.com, but only those (maybe in addition to Bitbucket but I'm not familiar with it). |
Agreed. Best approach is to only trust the main IDPs, and it would be even better if we picked the IDP that matched the currently open workspace that was requesting the token. Any manually added integrations should never be trusted to provide accurate information in gitpod tokens. I think Bitbucket is fine to trust. I suppose an alternative is for gitpod to validate the emails itself, but that would add a lot of complexity with email integrations that I think is unnecessary for the ultimate end goal. |
As an addendum, the services we trust should be configurable in config rather than hard coded. I imagine some Gitpod Dedicated users may well trust different private repositories like Gitlab for their organisation as a whole. Those aren't user configured though, but we should have the option for an administrator to say which core IDPs they are happy to trust. |
@filiptronicek @loujaybee is there an issue to cover the remediation work highlighted here or should I raise one. I note that sigstore/fulcio#1177 is currently held up on this as I can't reasonably add gitpod as an IDP to a service like fulcio when the emails it could be sending back are potentially anything the user wants. Fulcio is too important a service and they need to be able to trust Gitpod is being honest about the emails it is claiming. |
Is your feature request related to a problem?
I would like to use the tokens generated from
gp idp token
with gitsign and fulcio but they are missing certain claims for that to work.Describe the behaviour you'd like
Please include a additional
email
andemail_verified
claims in the JWT token.https://github.com/sigstore/fulcio/blob/main/docs/oidc.md#email-1
Describe alternatives you've considered
Additional context
Support for fulcio allows me to progress my investigation as part of #666
The text was updated successfully, but these errors were encountered: