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

enhance(oidc): provide support for custom issuer #1160

Merged
merged 4 commits into from
Jul 19, 2024

Conversation

ecrupper
Copy link
Contributor

This change will allow platform admins to publish the OpenID configuration (and in turn the JWKS) to custom URLs.

Of course, in order to have accurate information, these custom URLs will need to source their data from the standard Vela routes. But this allows selective exposure of OpenID data to external services such as GCP and AWS.

@ecrupper ecrupper requested a review from a team as a code owner July 15, 2024 19:24
Copy link

codecov bot commented Jul 15, 2024

Codecov Report

Attention: Patch coverage is 10.52632% with 17 lines in your changes missing coverage. Please review.

Project coverage is 52.11%. Comparing base (bde655a) to head (c41ae01).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1160      +/-   ##
==========================================
- Coverage   52.14%   52.11%   -0.04%     
==========================================
  Files         550      550              
  Lines       18842    18857      +15     
==========================================
+ Hits         9825     9827       +2     
- Misses       8455     8468      +13     
  Partials      562      562              
Files Coverage Δ
cmd/vela-server/token.go 0.00% <ø> (ø)
compiler/native/environment.go 86.04% <100.00%> (+0.21%) ⬆️
api/oi_config.go 0.00% <0.00%> (ø)
cmd/vela-server/validate.go 0.00% <0.00%> (ø)
cmd/vela-server/main.go 0.00% <0.00%> (ø)
cmd/vela-server/server.go 0.00% <0.00%> (ø)

@plyr4
Copy link
Contributor

plyr4 commented Jul 15, 2024

sweet, i think we also need it here

edit: no, thats meant to be hit by the worker holding the request token...
so it shouldnt be issuer. what about something else? would an external system ever be in charge of doing that exchange using only the request token + request url..?

@plyr4
Copy link
Contributor

plyr4 commented Jul 15, 2024

also should we add url validation for oidc-issuer?
like we do for server-addr

addr, err := url.Parse(c.String("server-addr"))
if err != nil {
	return err
}

Copy link
Contributor

@KellyMerrick KellyMerrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ecrupper ecrupper merged commit 18de798 into main Jul 19, 2024
11 of 13 checks passed
@ecrupper ecrupper deleted the enhance/oidc-custom-issuer branch July 19, 2024 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants