Allow customisation of OIDC scopes #3234
Conversation
bigkevmcd
force-pushed
the
customise-scopes
branch
2 times, most recently
from
e300c74 to
2deffd8
Compare
| @@ -113,6 +113,7 @@ func NewCommand() *cobra.Command { | |||
| cmd.Flags().DurationVar(&options.OIDC.TokenDuration, "oidc-token-duration", time.Hour, "The duration of the ID token. It should be set in the format: number + time unit (s,m,h) e.g., 20m") | |||
| cmd.Flags().StringVar(&options.OIDC.ClaimsConfig.Username, "oidc-username-claim", auth.ClaimUsername, "JWT claim to use as the user name. By default email, which is expected to be a unique identifier of the end user. Admins can choose other claims, such as sub or name, depending on their provider") | |||
| cmd.Flags().StringVar(&options.OIDC.ClaimsConfig.Groups, "oidc-groups-claim", auth.ClaimGroups, "JWT claim to use as the user's group. If the claim is present it must be an array of strings") | |||
| cmd.Flags().StringSliceVar(&options.OIDC.Scopes, "custom-oidc-scopes", auth.DefaultScopes, "Customise the requested scopes for then OIDC authentication flow - openid will always be requested") | |||
There was a problem hiding this comment.
This flag doesn't seem to be connected right now, maybe we remove it for now or?
There was a problem hiding this comment.
I think it is connected, through a long line of bits.
It is passed here
weave-gitops/cmd/gitops-server/cmd/cmd.go
Line 175 in 2deffd8
There was a problem hiding this comment.
Oh you're right. I was testing with mixed secret/cli config and then missed this
weave-gitops/pkg/server/auth/init.go
Lines 40 to 41 in 2deffd8
LGTM
There was a problem hiding this comment.
I find it a bit confusing that the secret overrides the cmd-line, but that's the existing behaviour for other options, consistency wins out :-)
| @@ -110,9 +120,29 @@ func NewOIDCConfigFromSecret(secret corev1.Secret) OIDCConfig { | |||
|
|
|||
| cfg.TokenDuration = tokenDuration | |||
|
|
|||
| scopes := splitAndTrim(string(secret.Data["customScopes"])) | |||
| if len(scopes) == 0 { | |||
There was a problem hiding this comment.
I think I may have been doing the odd if slice == nil in other places, will keep the distinction in mind..
There was a problem hiding this comment.
In this case, result := []string{} will return a slice of zero length.
var result []string would mean that result could be nil.
Taking the length of a nil slice yields 0 any which way.
pkg/server/auth/server.go
Outdated
| @@ -311,7 +334,7 @@ func (s *AuthServer) Callback() http.HandlerFunc { | |||
| return | |||
| } | |||
|
|
|||
| token, err = s.oauth2Config(nil).Exchange(ctx, code) | |||
| token, err = s.oauth2Config(s.OIDCConfig.Scopes).Exchange(ctx, code) | |||
There was a problem hiding this comment.
Is it important to include the scopes when doing the exchange?
There was a problem hiding this comment.
No, you're right, they're not, I think I did a quick s/scopes/s.OIDCConfig.Scopes/ good catch :-)
This provides mechanisms for overriding the requested scopes from an OIDC server. We must always ask for "openid" see the OpenID docs https://openid.net/specs/openid-connect-basic-1_0.html#RequestParameters
bigkevmcd
force-pushed
the
customise-scopes
branch
from
2deffd8 to
818b8c6
Compare
makkes
changed the title
What changed?
This provides a mechanism for customising the requested scopes from an upstream OIDC server.
Why was this change made?
Some customers need to be able to customise the set of scopes that are requested, perhaps because they need specialised ones for a claim.
How was this change implemented?
It's a pretty trivial change, defaults to the existing defaults, can be parsed from the OIDC Secret (as the other related values can) and it drops the "must have email and groups", retaining "must have openid" because we must have that.
How did you validate the change?
Tests and got James to test it against Okta
Release notes
Support for custom OIDC scopes when authenticating with an upstream server.
Documentation Changes
We need documentation on how to configure the OIDC Secret.