-
Notifications
You must be signed in to change notification settings - Fork 3
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(proposal): Vela OpenID Connect Provider #976
base: main
Are you sure you want to change the base?
Conversation
### ID_TOKEN_REQUEST_TOKEN | ||
|
||
Token is signed by _VELA_SERVER_PRIVATE_KEY_ using HS256 algorithm. It has the following claims: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think converting all our JWTs to the RS256 method is probably the way to go but figured that transition was a tad out of scope for this proposal.
But in terms of this specific new token (VELA_ID_TOKEN_REQUEST_TOKEN
), should we implement a server-side parser of the RSA keys and have it signed by the same private key as the ID_TOKEN
? Or add another token type to the HMAC set with plans to migrate everything at a later time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
converting all our JWTs to the RS256 method is probably the way to go but figured that transition was a tad out of scope for this proposal.
i agree with all of that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i picture rsa key parsing to be handled on the client but if you want to explain this a bit more maybe im not understanding the benefits fully
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think its fine to have the request tokens and the id tokens signed by the same key but i havent looked into it much to know if theres a "standard" for that. im sure if you ask a security pro theyll probably suggest we dont reuse keys 😅
} | ||
|
||
// Database Func | ||
func (e *engine) RotateKeys(_ context.Context) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you think about auto rotating keys after N days
- The build is the expected build (build_id) | ||
- The token is not expired | ||
- The token has indeed been signed by the Vela server | ||
- The associated build is actively running (leaked request tokens are not vulnerable post build). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this run the risk that an external service using the token might not be done with it yet? maybe we give it a little bit of wiggle room?
im picturing a scenario where Vela hands off the workload to a different service and that service might take some time to provision resources or something.
or... are we expecting id_tokens to always be generated within the build context and then the resulting token can be handed off to the external service?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ID token request token is bound by that rule yes because the request for an ID token should always occur during a running build. If there is a pass off that occurs, the new workload should be using the ID token not the ID request token
some food for thought, theres this open source provider/consumer library we might be able to pull inspiration from for some of the lower level details |
we might want to also look into using csrf tokens that the server holds onto to protect against request forgery this article from Google goes into a bit more detail https://developers.google.com/identity/openid-connect/openid-connect#createxsrftoken the idea is that you exchange another "session token" that the user needs to pass along at each part of the process to ensure someone malicious didnt man in the middle the token probably overkill for this first pass, and im not sure how to do the user identity bit but definitely something to think about if we want to build a "consumer" that accepts the id_token in exchance for an access_token |
Adding some comments from the recent committer meeting:
|
No description provided.