Skip to content

Commit

Permalink
OIDC config to allow mismatched discovery / issuer
Browse files Browse the repository at this point in the history
 - There are a number of cases where the OIDC discovery url returns one
   issuer, but its desirable to use a separately configured / named
   issuer for validation instead.

   There are cases in Azure where this is necessary due to their
   non-standard OIDC configuration -- which is why this was originally
   added:
   coreos/go-oidc#315

   There are also cases where it's necessary to use an in-cluster
   service address, but browser clients are using the external ingress
   address. Due to cluster DNS configuration, it's possible that
   flyteadmin may be unable to resolve or use the public ingress
   address for an Idp, but the internal service address is available.
   This configuration change allows for that.

Signed-off-by: ddl-ebrown <[email protected]>
  • Loading branch information
ddl-ebrown committed Aug 31, 2024
1 parent 7a91799 commit d897dfb
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 0 deletions.
6 changes: 6 additions & 0 deletions flyteadmin/auth/auth_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,12 @@ func NewAuthenticationContext(ctx context.Context, sm core.SecretManager, oauth2
// Construct an oidc Provider, which needs its own http Client.
oidcCtx := oidc.ClientContext(ctx, httpClient)
baseURL := options.UserAuth.OpenID.BaseURL.String()
// use a different issuer for token validation if configured
// this allows discovery to work when issuer_url from upstream is mismatched
// see https://github.com/coreos/go-oidc/pull/315
if iss := options.UserAuth.OpenID.IssuerURL.String(); iss != "" {
oidcCtx = oidc.InsecureIssuerURLContext(oidcCtx, iss)

Check warning on line 148 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L147-L148

Added lines #L147 - L148 were not covered by tests
}
provider, err := oidc.NewProvider(oidcCtx, baseURL)
if err != nil {
return Context{}, errors.Wrapf(ErrauthCtx, err, "Error creating oidc provider w/ issuer [%v]", baseURL)
Expand Down
6 changes: 6 additions & 0 deletions flyteadmin/auth/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,12 @@ type OpenIDOptions struct {
// will look something like https://company.okta.com/oauth2/abcdef123456789/
BaseURL config.URL `json:"baseUrl"`

// Allows discovery to work when the issuer_url reported by upstream is mismatched with baseUrl. This may be the
// case with Azure *or* when baseUrl refers to an in-cluster service like https://keycloak/auth/realms/MyRealm but
// the issuer is a public ingress address accessible to the OIDC client
// Refer to https://github.com/coreos/go-oidc/pull/315 for additional details
IssuerURL config.URL `json:"issuerUrl" pflag:",OPTIONAL: Use this issuer URL for request validation rather than baseUrl."`

// Provides a list of scopes to request from the IDP when authenticating. Default value requests claims that should
// be supported by any OIdC server. Refer to https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims for
// a complete list. Other providers might support additional scopes that you can define in a config.
Expand Down
3 changes: 3 additions & 0 deletions flyteadmin/auth/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ func TestParseClientSecretConfig(t *testing.T) {
func TestDefaultConfig(t *testing.T) {
assert.Equal(t, len(DefaultConfig.AppAuth.SelfAuthServer.StaticClients), 3)
assert.Equal(t, DefaultConfig.AppAuth.SelfAuthServer.StaticClients["flyte-cli"].ID, "flyte-cli")

// oidc issuer config should only be used when baseUrl is mismatched
assert.Equal(t, DefaultConfig.UserAuth.OpenID.IssuerURL.String(), "")
}

func TestCompare(t *testing.T) {
Expand Down

0 comments on commit d897dfb

Please sign in to comment.