From e03095c73f8f16889c63f4d633ffb1523c5c4804 Mon Sep 17 00:00:00 2001 From: Kim Date: Wed, 28 Oct 2026 09:41:13 +0000 Subject: [PATCH 1/2] feat(oidc): preserve public provider IDs during namespace migration --- embedx/config.schema.json | 6 + selfservice/strategy/oidc/provider_config.go | 37 ++- .../strategy/oidc/provider_config_test.go | 60 ++++ selfservice/strategy/oidc/provider_test.go | 9 + selfservice/strategy/oidc/public_id_test.go | 266 ++++++++++++++++++ selfservice/strategy/oidc/state.go | 2 +- selfservice/strategy/oidc/state_test.go | 14 + selfservice/strategy/oidc/strategy_login.go | 2 +- selfservice/strategy/oidc/types.go | 2 +- ...selfServiceOIDCProvider.emptyPublicID.yaml | 6 + ...lfServiceOIDCProvider.invalidPublicID.yaml | 6 + .../selfServiceOIDCProvider.publicID.yaml | 6 + 12 files changed, 403 insertions(+), 13 deletions(-) create mode 100644 selfservice/strategy/oidc/public_id_test.go create mode 100644 test/schema/fixtures/config.schema.test.failure/selfServiceOIDCProvider.emptyPublicID.yaml create mode 100644 test/schema/fixtures/config.schema.test.failure/selfServiceOIDCProvider.invalidPublicID.yaml create mode 100644 test/schema/fixtures/config.schema.test.success/selfServiceOIDCProvider.publicID.yaml diff --git a/embedx/config.schema.json b/embedx/config.schema.json index cff95817190d..4aa8a05ab471 100644 --- a/embedx/config.schema.json +++ b/embedx/config.schema.json @@ -426,6 +426,12 @@ "selfServiceOIDCProvider": { "type": "object", "properties": { + "public_id": { + "type": "string", + "minLength": 1, + "description": "Optional public provider identifier accepted in requests and emitted in browser forms, OAuth state and callback URLs. Credentials and sessions retain id. It must not collide with another configured provider identifier.", + "examples": ["google"] + }, "id": { "type": "string", "examples": ["google"] diff --git a/selfservice/strategy/oidc/provider_config.go b/selfservice/strategy/oidc/provider_config.go index b98c24972ae4..53053305a116 100644 --- a/selfservice/strategy/oidc/provider_config.go +++ b/selfservice/strategy/oidc/provider_config.go @@ -17,9 +17,12 @@ import ( ) type Configuration struct { - // ID is the provider's ID + // ID is the canonical provider ID stored in identity credentials and sessions. ID string `json:"id"` + // PublicID preserves an existing request/form/callback identifier when ID changes. + PublicID string `json:"public_id,omitempty"` + // Provider is either "generic" for a generic OAuth 2.0 / OpenID Connect Provider or one of: // - generic // - google @@ -138,6 +141,13 @@ type Configuration struct { IDTokenVerificationEnabled bool `json:"id_token_verification_enabled"` } +func (p Configuration) RequestID() string { + if p.PublicID != "" { + return p.PublicID + } + return p.ID +} + func (p Configuration) Redir(public *url.URL) string { if p.PKCE == "force" { return urlx.AppendPaths(public, RouteCallbackGeneric).String() @@ -145,12 +155,12 @@ func (p Configuration) Redir(public *url.URL) string { if p.OrganizationID != "" { route := RouteOrganizationCallback - route = strings.Replace(route, ":provider", p.ID, 1) + route = strings.Replace(route, ":provider", p.RequestID(), 1) route = strings.Replace(route, ":organization", p.OrganizationID, 1) return urlx.AppendPaths(public, route).String() } - return urlx.AppendPaths(public, strings.Replace(RouteCallback, ":provider", p.ID, 1)).String() + return urlx.AppendPaths(public, strings.Replace(RouteCallback, ":provider", p.RequestID(), 1)).String() } type ConfigurationCollection struct { @@ -188,15 +198,22 @@ var supportedProviders = map[string]func(config *Configuration, reg Dependencies } func (c ConfigurationCollection) Provider(id string, reg Dependencies) (Provider, error) { + var matched *Configuration for k := range c.Providers { p := c.Providers[k] - if p.ID == id { - if f, ok := supportedProviders[p.Provider]; ok { - return f(&p, reg), nil - } - - return nil, errors.Errorf("provider type %s is not supported, supported are: %v", p.Provider, maps.Keys(supportedProviders)) + if p.ID != id && (p.PublicID == "" || p.PublicID != id) { + continue + } + if matched != nil { + return nil, errors.Errorf("OpenID Connect Provider %q matches multiple configured providers", id) } + matched = &p + } + if matched == nil { + return nil, errors.WithStack(herodot.ErrNotFound.WithReasonf(`OpenID Connect Provider "%s" is unknown or has not been configured`, id)) + } + if f, ok := supportedProviders[matched.Provider]; ok { + return f(matched, reg), nil } - return nil, errors.WithStack(herodot.ErrNotFound.WithReasonf(`OpenID Connect Provider "%s" is unknown or has not been configured`, id)) + return nil, errors.Errorf("provider type %s is not supported, supported are: %v", matched.Provider, maps.Keys(supportedProviders)) } diff --git a/selfservice/strategy/oidc/provider_config_test.go b/selfservice/strategy/oidc/provider_config_test.go index ba9cc4a11ffe..9b91c3e6a6cb 100644 --- a/selfservice/strategy/oidc/provider_config_test.go +++ b/selfservice/strategy/oidc/provider_config_test.go @@ -7,10 +7,15 @@ import ( "bytes" "context" "encoding/json" + "net/url" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" + + "github.com/ory/kratos/text" + "github.com/ory/kratos/ui/container" "github.com/ory/kratos/driver/config" "github.com/ory/kratos/identity" @@ -34,3 +39,58 @@ func TestConfig(t *testing.T) { require.Len(t, collection.Providers, 1) assert.Equal(t, "generic", collection.Providers[0].Provider) } + +func TestProviderPublicIDResolvesToCanonicalProvider(t *testing.T) { + var collection oidc.ConfigurationCollection + require.NoError(t, json.Unmarshal([]byte(`{"providers":[{"id":"google-goodsystems","public_id":"google","provider":"google","client_id":"goodmeet-client"}]}`), &collection)) + + provider, err := collection.Provider("google", nil) + require.NoError(t, err) + assert.Equal(t, "google-goodsystems", provider.Config().ID) + assert.Equal(t, "goodmeet-client", provider.Config().ClientID) +} + +func TestProviderPublicIDKeepsRegisteredCallback(t *testing.T) { + provider := oidc.Configuration{ID: "google-goodsystems", PublicID: "google"} + public, err := url.Parse("https://kratos.goodmeet.test/") + require.NoError(t, err) + + assert.Equal(t, "https://kratos.goodmeet.test/self-service/methods/oidc/callback/google", provider.Redir(public)) +} + +func TestProviderPublicIDKeepsBrowserSubmitValue(t *testing.T) { + ui := container.New("/self-service/login") + oidc.AddProviders(ui, []oidc.Configuration{{ID: "google-goodsystems", PublicID: "google"}}, text.NewInfoLoginWith) + raw, err := json.Marshal(ui.Nodes) + require.NoError(t, err) + assert.Equal(t, "google", gjson.GetBytes(raw, "0.attributes.value").String()) +} + +func TestProviderPublicIDRejectsAmbiguousSelection(t *testing.T) { + for _, providers := range [][]oidc.Configuration{ + {{ID: "google", Provider: "google"}, {ID: "google-goodsystems", PublicID: "google", Provider: "google"}}, + {{ID: "google-goodsystems", PublicID: "google", Provider: "google"}, {ID: "google", Provider: "google"}}, + } { + _, err := (oidc.ConfigurationCollection{Providers: providers}).Provider("google", nil) + require.Error(t, err) + } +} + +func TestProviderWithoutPublicIDKeepsExistingBehavior(t *testing.T) { + for _, id := range []string{"google", "google-goodsystems"} { + t.Run(id, func(t *testing.T) { + collection := oidc.ConfigurationCollection{Providers: []oidc.Configuration{{ID: id, Provider: "google", ClientID: "client"}}} + provider, err := collection.Provider(id, nil) + require.NoError(t, err) + assert.Equal(t, id, provider.Config().ID) + assert.Equal(t, id, provider.Config().RequestID()) + public, err := url.Parse("https://kratos.example.test") + require.NoError(t, err) + assert.Equal(t, "https://kratos.example.test/self-service/methods/oidc/callback/"+id, provider.Config().Redir(public)) + if id != "google" { + _, err = collection.Provider("google", nil) + require.Error(t, err) + } + }) + } +} diff --git a/selfservice/strategy/oidc/provider_test.go b/selfservice/strategy/oidc/provider_test.go index 4e45f42c72f4..61e998fa8da1 100644 --- a/selfservice/strategy/oidc/provider_test.go +++ b/selfservice/strategy/oidc/provider_test.go @@ -135,3 +135,12 @@ func TestLocale(t *testing.T) { }) } } + +func RegisterTestGoogleProvider(t *testing.T, id, jwksURL string) { + supportedProviders[id] = func(c *Configuration, reg Dependencies) Provider { + provider := NewProviderGoogle(c, reg).(*ProviderGoogle) + provider.JWKSUrl = jwksURL + return provider + } + t.Cleanup(func() { delete(supportedProviders, id) }) +} diff --git a/selfservice/strategy/oidc/public_id_test.go b/selfservice/strategy/oidc/public_id_test.go new file mode 100644 index 000000000000..fc0b6f2837c2 --- /dev/null +++ b/selfservice/strategy/oidc/public_id_test.go @@ -0,0 +1,266 @@ +// Copyright © 2026 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package oidc_test + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/cookiejar" + "net/http/httptest" + "net/url" + "strings" + "testing" + "time" + + "github.com/golang-jwt/jwt/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" + + "github.com/ory/kratos/driver/config" + "github.com/ory/kratos/identity" + "github.com/ory/kratos/internal" + "github.com/ory/kratos/internal/testhelpers" + "github.com/ory/kratos/selfservice/flow" + "github.com/ory/kratos/selfservice/flow/login" + "github.com/ory/kratos/selfservice/strategy/oidc" + "github.com/ory/kratos/x" +) + +func TestPublicProviderIDNativeTokenRequests(t *testing.T) { + for _, entry := range []string{"login", "registration"} { + t.Run(entry, func(t *testing.T) { + ctx := context.Background() + conf, reg := internal.NewFastRegistryWithMocks(t) + jwks := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write(publicJWKS) + })) + t.Cleanup(jwks.Close) + oidc.RegisterTestGoogleProvider(t, "test-google", jwks.URL) + viperSetProviderConfig(t, conf, oidc.Configuration{ + ID: "google-goodsystems", PublicID: "google", Provider: "test-google", + ClientID: "goodmeet-client", Mapper: "file://./stub/oidc.hydra.jsonnet", PKCE: "never", + }) + conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationEnabled, true) + testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/stub.schema.json") + conf.MustSet(ctx, config.HookStrategyKey(config.ViperKeySelfServiceRegistrationAfter, identity.CredentialsTypeOIDC.String()), []config.SelfServiceHook{{Name: "session"}}) + server, _ := testhelpers.NewKratosServer(t, reg) + + subject := "goodmeet-" + entry + "@example.test" + classic := identity.NewIdentity("default") + classic.Traits = identity.Traits(`{"subject":"` + subject + `"}`) + classicCredentials, err := identity.NewCredentialsOIDC(new(identity.CredentialsOIDCEncryptedTokens), "google", subject, "") + require.NoError(t, err) + classic.SetCredentials(identity.CredentialsTypeOIDC, *classicCredentials) + require.NoError(t, reg.IdentityManager().Create(ctx, classic)) + + token := createIdToken(t, jwt.RegisteredClaims{ + Issuer: "https://accounts.google.com", Subject: subject, + Audience: jwt.ClaimStrings{"goodmeet-client"}, ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)), + }) + client := &http.Client{CheckRedirect: func(r *http.Request, via []*http.Request) error { return http.ErrUseLastResponse }} + var productID string + for _, providerID := range []string{"google", "google-goodsystems", "google"} { + response, err := client.Get(server.URL + "/self-service/" + entry + "/api") + require.NoError(t, err) + flowBody, err := io.ReadAll(response.Body) + require.NoError(t, response.Body.Close()) + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode, "%s", flowBody) + action := gjson.GetBytes(flowBody, "ui.action").String() + require.NotEmpty(t, action) + payload, err := json.Marshal(map[string]string{"method": "OIDC", "provider": providerID, "id_token": token}) + require.NoError(t, err) + request, err := http.NewRequest(http.MethodPost, action, strings.NewReader(string(payload))) + require.NoError(t, err) + request.Header.Set("Content-Type", "application/json") + request.Header.Set("Accept", "application/json") + response, err = client.Do(request) + require.NoError(t, err) + body, err := io.ReadAll(response.Body) + require.NoError(t, response.Body.Close()) + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode, "%s", body) + require.NotEmpty(t, gjson.GetBytes(body, "session_token").String()) + actualID := gjson.GetBytes(body, "session.identity.id").String() + require.NotEmpty(t, actualID) + assert.NotEqual(t, classic.ID.String(), actualID) + assert.Equal(t, "google-goodsystems", gjson.GetBytes(body, "session.authentication_methods.0.provider").String()) + if productID == "" { + productID = actualID + } else { + assert.Equal(t, productID, actualID) + } + + product, _, err := reg.PrivilegedIdentityPool().FindByCredentialsIdentifier(ctx, identity.CredentialsTypeOIDC, "google-goodsystems:"+subject) + require.NoError(t, err) + assert.Equal(t, productID, product.ID.String()) + original, _, err := reg.PrivilegedIdentityPool().FindByCredentialsIdentifier(ctx, identity.CredentialsTypeOIDC, "google:"+subject) + require.NoError(t, err) + assert.Equal(t, classic.ID, original.ID) + } + }) + } +} + +func TestPublicProviderIDBrowserCallbacks(t *testing.T) { + for _, selection := range []string{"form", "query"} { + t.Run(selection, func(t *testing.T) { + ctx := context.Background() + conf, reg := internal.NewFastRegistryWithMocks(t) + const subject = "browser-goodmeet@example.test" + var issuer *httptest.Server + var callback string + var browserToken string + issuer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/openid-configuration": + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "issuer": issuer.URL, "authorization_endpoint": issuer.URL + "/authorize", + "token_endpoint": issuer.URL + "/token", "jwks_uri": issuer.URL + "/keys", + "response_types_supported": []string{"code"}, "subject_types_supported": []string{"public"}, + "id_token_signing_alg_values_supported": []string{"RS256"}, + }) + case "/keys": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write(publicJWKS) + case "/authorize": + if r.URL.Query().Get("redirect_uri") != callback { + http.Error(w, "unregistered callback", http.StatusBadRequest) + return + } + destination, _ := url.Parse(callback) + query := destination.Query() + query.Set("code", "synthetic-code") + query.Set("state", r.URL.Query().Get("state")) + destination.RawQuery = query.Encode() + http.Redirect(w, r, destination.String(), http.StatusSeeOther) + case "/token": + _ = r.ParseForm() + if r.Form.Get("redirect_uri") != callback || r.Form.Get("code") != "synthetic-code" { + http.Error(w, "invalid code exchange", http.StatusBadRequest) + return + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]string{"access_token": "synthetic-token", "token_type": "Bearer", "id_token": browserToken}) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(issuer.Close) + browserToken = createIdToken(t, jwt.RegisteredClaims{ + Issuer: issuer.URL, Subject: subject, Audience: jwt.ClaimStrings{"goodmeet-client"}, + ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)), + }) + viperSetProviderConfig(t, conf, oidc.Configuration{ + ID: "google-goodsystems", PublicID: "google", Provider: "generic", ClientID: "goodmeet-client", + ClientSecret: "synthetic-test-secret", IssuerURL: issuer.URL, Mapper: "file://./stub/oidc.hydra.jsonnet", PKCE: "never", + }) + conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationEnabled, true) + testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/stub.schema.json") + conf.MustSet(ctx, config.HookStrategyKey(config.ViperKeySelfServiceRegistrationAfter, identity.CredentialsTypeOIDC.String()), []config.SelfServiceHook{{Name: "session"}}) + conf.MustSet(ctx, config.ViperKeyClientHTTPNoPrivateIPRanges, false) + server, _ := testhelpers.NewKratosServer(t, reg) + callback = server.URL + "/self-service/methods/oidc/callback/google" + conf.MustSet(ctx, config.ViperKeySelfServiceLoginUI, server.URL+"/login-ui") + conf.MustSet(ctx, config.ViperKeySelfServiceBrowserDefaultReturnTo, server.URL+"/finished") + conf.MustSet(ctx, config.ViperKeyURLsAllowedReturnToDomains, []string{server.URL}) + jar, err := cookiejar.New(nil) + require.NoError(t, err) + client := &http.Client{Jar: jar, CheckRedirect: func(r *http.Request, via []*http.Request) error { return http.ErrUseLastResponse }} + + response, err := client.Get(server.URL + "/self-service/login/browser") + require.NoError(t, err) + require.NoError(t, response.Body.Close()) + require.Equal(t, http.StatusSeeOther, response.StatusCode) + location, err := response.Location() + require.NoError(t, err) + flowID := location.Query().Get("flow") + require.NotEmpty(t, flowID) + response, err = client.Get(server.URL + "/self-service/login/flows?id=" + flowID) + require.NoError(t, err) + raw, err := io.ReadAll(response.Body) + require.NoError(t, response.Body.Close()) + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode, "%s", raw) + assert.Equal(t, "google", gjson.GetBytes(raw, `ui.nodes.#(attributes.name=="provider").attributes.value`).String()) + action := gjson.GetBytes(raw, "ui.action").String() + form := url.Values{"method": {"oidc"}, "csrf_token": {"fake-csrf-token"}} + if selection == "form" { + form.Set("provider", "google") + } else { + action += "&provider=google" + } + response, err = client.PostForm(action, form) + require.NoError(t, err) + raw, err = io.ReadAll(response.Body) + require.NoError(t, response.Body.Close()) + require.NoError(t, err) + require.Equal(t, http.StatusSeeOther, response.StatusCode, "%s", raw) + authorize, err := response.Location() + require.NoError(t, err) + assert.Equal(t, callback, authorize.Query().Get("redirect_uri")) + response, err = client.Get(authorize.String()) + require.NoError(t, err) + require.NoError(t, response.Body.Close()) + require.Equal(t, http.StatusSeeOther, response.StatusCode) + returned, err := response.Location() + require.NoError(t, err) + assert.Equal(t, "/self-service/methods/oidc/callback/google", returned.Path) + mismatch := *returned + mismatch.Path = "/self-service/methods/oidc/callback/google-goodsystems" + request, err := http.NewRequest(http.MethodGet, mismatch.String(), nil) + require.NoError(t, err) + request.Header.Set("Accept", "application/json") + response, err = client.Do(request) + require.NoError(t, err) + raw, err = io.ReadAll(response.Body) + require.NoError(t, response.Body.Close()) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, response.StatusCode, "%s", raw) + assert.Contains(t, string(raw), "provider mismatch") + response, err = client.Get(returned.String()) + require.NoError(t, err) + raw, err = io.ReadAll(response.Body) + require.NoError(t, response.Body.Close()) + require.NoError(t, err) + require.Equal(t, http.StatusSeeOther, response.StatusCode, "%s", raw) + finished, err := response.Location() + require.NoError(t, err) + assert.Equal(t, "/finished", finished.Path) + response, err = client.Get(server.URL + "/sessions/whoami") + require.NoError(t, err) + raw, err = io.ReadAll(response.Body) + require.NoError(t, response.Body.Close()) + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode, "%s", raw) + assert.Equal(t, "google-goodsystems", gjson.GetBytes(raw, "authentication_methods.0.provider").String()) + product, _, err := reg.PrivilegedIdentityPool().FindByCredentialsIdentifier(ctx, identity.CredentialsTypeOIDC, "google-goodsystems:"+subject) + require.NoError(t, err) + assert.Equal(t, product.ID.String(), gjson.GetBytes(raw, "identity.id").String()) + }) + } +} + +func TestPublicProviderIDIdentifierFirstForm(t *testing.T) { + ctx := context.Background() + conf, reg := internal.NewFastRegistryWithMocks(t) + testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/stub.schema.json") + conf.MustSet(ctx, config.ViperKeySecurityAccountEnumerationMitigate, false) + viperSetProviderConfig(t, conf, oidc.Configuration{ + ID: "google-goodsystems", PublicID: "google", Provider: "google", ClientID: "goodmeet-client", + }) + id := createIdentity(t, ctx, reg, x.NewUUID(), "google-goodsystems") + request := httptest.NewRequest(http.MethodGet, "/self-service/login/browser", nil) + f, err := login.NewFlow(conf, time.Minute, "csrf", request, flow.TypeBrowser) + require.NoError(t, err) + require.NoError(t, oidc.NewStrategy(reg).PopulateLoginMethodIdentifierFirstCredentials(request, f, login.WithIdentityHint(id))) + raw, err := json.Marshal(f.UI.Nodes) + require.NoError(t, err) + assert.Equal(t, "google", gjson.GetBytes(raw, `#(attributes.name=="provider").attributes.value`).String()) +} diff --git a/selfservice/strategy/oidc/state.go b/selfservice/strategy/oidc/state.go index ce8cb17bc653..1149a9af6811 100644 --- a/selfservice/strategy/oidc/state.go +++ b/selfservice/strategy/oidc/state.go @@ -42,7 +42,7 @@ func (s *Strategy) GenerateState(ctx context.Context, p Provider, flowID uuid.UU state := oidcv1.State{ FlowId: flowID.Bytes(), SessionTokenExchangeCodeSha512: x.NewUUID().Bytes(), - ProviderId: p.Config().ID, + ProviderId: p.Config().RequestID(), PkceVerifier: maybePKCE(ctx, s.d, p), } if code, hasCode, _ := s.d.SessionTokenExchangePersister().CodeForFlow(ctx, flowID); hasCode { diff --git a/selfservice/strategy/oidc/state_test.go b/selfservice/strategy/oidc/state_test.go index bf0366571a6b..4b7d61b1cd40 100644 --- a/selfservice/strategy/oidc/state_test.go +++ b/selfservice/strategy/oidc/state_test.go @@ -44,3 +44,17 @@ type testProvider struct{} func (t *testProvider) Config() *oidc.Configuration { return &oidc.Configuration{ID: "test-provider", PKCE: "never"} } + +func TestGenerateStateWithPublicProviderID(t *testing.T) { + _, reg := internal.NewFastRegistryWithMocks(t) + strat := oidc.NewStrategy(reg) + ctx := context.Background() + provider := oidc.NewProviderGoogle(&oidc.Configuration{ID: "google-goodsystems", PublicID: "google", PKCE: "never"}, reg) + + encrypted, _, err := strat.GenerateState(ctx, provider, x.NewUUID()) + require.NoError(t, err) + state, err := oidc.DecryptState(ctx, reg.Cipher(ctx), encrypted) + require.NoError(t, err) + assert.Equal(t, "google", state.ProviderId) + assert.Equal(t, "google-goodsystems", provider.Config().ID) +} diff --git a/selfservice/strategy/oidc/strategy_login.go b/selfservice/strategy/oidc/strategy_login.go index a701315bf289..0bb34ec3ab1b 100644 --- a/selfservice/strategy/oidc/strategy_login.go +++ b/selfservice/strategy/oidc/strategy_login.go @@ -388,7 +388,7 @@ func (s *Strategy) PopulateLoginMethodIdentifierFirstCredentials(r *http.Request for _, l := range linked { lc := l.Config() - AddProvider(f.UI, lc.ID, text.NewInfoLoginWith(stringsx.Coalesce(lc.Label, lc.ID), lc.ID)) + AddProvider(f.UI, lc.RequestID(), text.NewInfoLoginWith(stringsx.Coalesce(lc.Label, lc.RequestID()), lc.RequestID())) } } diff --git a/selfservice/strategy/oidc/types.go b/selfservice/strategy/oidc/types.go index 768e20d23531..e42d66d67e14 100644 --- a/selfservice/strategy/oidc/types.go +++ b/selfservice/strategy/oidc/types.go @@ -20,7 +20,7 @@ type FlowMethod struct { func AddProviders(c *container.Container, providers []Configuration, message func(provider string, providerId string) *text.Message) { for _, p := range providers { - AddProvider(c, p.ID, message(stringsx.Coalesce(p.Label, p.ID), p.ID)) + AddProvider(c, p.RequestID(), message(stringsx.Coalesce(p.Label, p.RequestID()), p.RequestID())) } } diff --git a/test/schema/fixtures/config.schema.test.failure/selfServiceOIDCProvider.emptyPublicID.yaml b/test/schema/fixtures/config.schema.test.failure/selfServiceOIDCProvider.emptyPublicID.yaml new file mode 100644 index 000000000000..7e14e20a290f --- /dev/null +++ b/test/schema/fixtures/config.schema.test.failure/selfServiceOIDCProvider.emptyPublicID.yaml @@ -0,0 +1,6 @@ +id: google-goodsystems +public_id: "" +provider: google +client_id: goodmeet-client +client_secret: synthetic-test-secret +mapper_url: file:///etc/config/google.jsonnet diff --git a/test/schema/fixtures/config.schema.test.failure/selfServiceOIDCProvider.invalidPublicID.yaml b/test/schema/fixtures/config.schema.test.failure/selfServiceOIDCProvider.invalidPublicID.yaml new file mode 100644 index 000000000000..c20cbe62f955 --- /dev/null +++ b/test/schema/fixtures/config.schema.test.failure/selfServiceOIDCProvider.invalidPublicID.yaml @@ -0,0 +1,6 @@ +id: google-goodsystems +public_id: 42 +provider: google +client_id: goodmeet-client +client_secret: synthetic-test-secret +mapper_url: file:///etc/config/google.jsonnet diff --git a/test/schema/fixtures/config.schema.test.success/selfServiceOIDCProvider.publicID.yaml b/test/schema/fixtures/config.schema.test.success/selfServiceOIDCProvider.publicID.yaml new file mode 100644 index 000000000000..2bd8b48026b4 --- /dev/null +++ b/test/schema/fixtures/config.schema.test.success/selfServiceOIDCProvider.publicID.yaml @@ -0,0 +1,6 @@ +id: google-goodsystems +public_id: google +provider: google +client_id: goodmeet-client +client_secret: synthetic-test-secret +mapper_url: file:///etc/config/google.jsonnet From 46de0a01ba68a7ce43b3a505f4b875fa1ae97538 Mon Sep 17 00:00:00 2001 From: Kim Date: Wed, 28 Oct 2026 10:30:14 +0000 Subject: [PATCH 2/2] refactor(oidc): limit provider aliases to ID-token authentication --- embedx/config.schema.json | 15 +- .../oidc/id_token_provider_alias_test.go | 139 +++++++++ selfservice/strategy/oidc/provider_config.go | 43 +-- .../strategy/oidc/provider_config_test.go | 60 ---- selfservice/strategy/oidc/public_id_test.go | 266 ------------------ selfservice/strategy/oidc/state.go | 2 +- selfservice/strategy/oidc/state_test.go | 14 - selfservice/strategy/oidc/strategy.go | 17 +- selfservice/strategy/oidc/strategy_login.go | 4 +- .../strategy/oidc/strategy_registration.go | 2 +- selfservice/strategy/oidc/types.go | 2 +- .../root.idTokenProviderAliasEmptyTarget.yaml | 22 ++ ...oot.idTokenProviderAliasInvalidTarget.yaml | 22 ++ ...selfServiceOIDCProvider.emptyPublicID.yaml | 6 - ...lfServiceOIDCProvider.invalidPublicID.yaml | 6 - .../root.idTokenProviderAliases.yaml | 22 ++ .../selfServiceOIDCProvider.publicID.yaml | 6 - 17 files changed, 245 insertions(+), 403 deletions(-) create mode 100644 selfservice/strategy/oidc/id_token_provider_alias_test.go delete mode 100644 selfservice/strategy/oidc/public_id_test.go create mode 100644 test/schema/fixtures/config.schema.test.failure/root.idTokenProviderAliasEmptyTarget.yaml create mode 100644 test/schema/fixtures/config.schema.test.failure/root.idTokenProviderAliasInvalidTarget.yaml delete mode 100644 test/schema/fixtures/config.schema.test.failure/selfServiceOIDCProvider.emptyPublicID.yaml delete mode 100644 test/schema/fixtures/config.schema.test.failure/selfServiceOIDCProvider.invalidPublicID.yaml create mode 100644 test/schema/fixtures/config.schema.test.success/root.idTokenProviderAliases.yaml delete mode 100644 test/schema/fixtures/config.schema.test.success/selfServiceOIDCProvider.publicID.yaml diff --git a/embedx/config.schema.json b/embedx/config.schema.json index 4aa8a05ab471..6901e80446ac 100644 --- a/embedx/config.schema.json +++ b/embedx/config.schema.json @@ -426,12 +426,6 @@ "selfServiceOIDCProvider": { "type": "object", "properties": { - "public_id": { - "type": "string", - "minLength": 1, - "description": "Optional public provider identifier accepted in requests and emitted in browser forms, OAuth state and callback URLs. Credentials and sessions retain id. It must not collide with another configured provider identifier.", - "examples": ["google"] - }, "id": { "type": "string", "examples": ["google"] @@ -1952,6 +1946,15 @@ "type": "object", "additionalProperties": false, "properties": { + "id_token_provider_aliases": { + "type": "object", + "description": "Maps incoming provider IDs to configured provider IDs only for login or registration submissions with an id_token. Browser authorization, callbacks and settings keep exact provider-ID lookup.", + "additionalProperties": { + "type": "string", + "minLength": 1 + }, + "examples": [{"google": "google-goodsystems"}] + }, "base_redirect_uri": { "type": "string", "title": "Base URL for OAuth2 Redirect URIs", diff --git a/selfservice/strategy/oidc/id_token_provider_alias_test.go b/selfservice/strategy/oidc/id_token_provider_alias_test.go new file mode 100644 index 000000000000..0f24e48a4681 --- /dev/null +++ b/selfservice/strategy/oidc/id_token_provider_alias_test.go @@ -0,0 +1,139 @@ +// Copyright © 2026 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package oidc_test + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/golang-jwt/jwt/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" + + "github.com/ory/kratos/driver/config" + "github.com/ory/kratos/identity" + "github.com/ory/kratos/internal" + "github.com/ory/kratos/internal/testhelpers" + "github.com/ory/kratos/selfservice/strategy/oidc" +) + +func TestIDTokenProviderAliasesNativeRequests(t *testing.T) { + for _, entry := range []string{"login", "registration"} { + t.Run(entry, func(t *testing.T) { + ctx := context.Background() + conf, reg := internal.NewFastRegistryWithMocks(t) + jwks := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write(publicJWKS) + })) + t.Cleanup(jwks.Close) + oidc.RegisterTestGoogleProvider(t, "test-google", jwks.URL) + provider := oidc.Configuration{ + ID: "google-goodsystems", Provider: "test-google", + ClientID: "goodmeet-client", Mapper: "file://./stub/oidc.hydra.jsonnet", PKCE: "never", + } + viperSetProviderConfig(t, conf, provider) + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".oidc.config", &oidc.ConfigurationCollection{ + Providers: []oidc.Configuration{provider}, + IDTokenProviderAliases: map[string]string{"google": "google-goodsystems"}, + }) + conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationEnabled, true) + testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/stub.schema.json") + conf.MustSet(ctx, config.HookStrategyKey(config.ViperKeySelfServiceRegistrationAfter, identity.CredentialsTypeOIDC.String()), []config.SelfServiceHook{{Name: "session"}}) + server, _ := testhelpers.NewKratosServer(t, reg) + + subject := "goodmeet-" + entry + "@example.test" + classic := identity.NewIdentity("default") + classic.Traits = identity.Traits(`{"subject":"` + subject + `"}`) + classicCredentials, err := identity.NewCredentialsOIDC(new(identity.CredentialsOIDCEncryptedTokens), "google", subject, "") + require.NoError(t, err) + classic.SetCredentials(identity.CredentialsTypeOIDC, *classicCredentials) + require.NoError(t, reg.IdentityManager().Create(ctx, classic)) + + token := createIdToken(t, jwt.RegisteredClaims{ + Issuer: "https://accounts.google.com", Subject: subject, + Audience: jwt.ClaimStrings{"goodmeet-client"}, ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)), + }) + client := &http.Client{CheckRedirect: func(r *http.Request, via []*http.Request) error { return http.ErrUseLastResponse }} + submit := func(providerID, idToken string) (int, []byte) { + t.Helper() + response, err := client.Get(server.URL + "/self-service/" + entry + "/api") + require.NoError(t, err) + flowBody, err := io.ReadAll(response.Body) + require.NoError(t, response.Body.Close()) + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode, "%s", flowBody) + action := gjson.GetBytes(flowBody, "ui.action").String() + require.NotEmpty(t, action) + payload, err := json.Marshal(map[string]string{"method": "OIDC", "provider": providerID, "id_token": idToken}) + require.NoError(t, err) + request, err := http.NewRequest(http.MethodPost, action, strings.NewReader(string(payload))) + require.NoError(t, err) + request.Header.Set("Content-Type", "application/json") + request.Header.Set("Accept", "application/json") + response, err = client.Do(request) + require.NoError(t, err) + body, err := io.ReadAll(response.Body) + require.NoError(t, response.Body.Close()) + require.NoError(t, err) + return response.StatusCode, body + } + var productID string + for _, providerID := range []string{"google", "google-goodsystems", "google"} { + status, body := submit(providerID, token) + require.Equal(t, http.StatusOK, status, "%s", body) + require.NotEmpty(t, gjson.GetBytes(body, "session_token").String()) + actualID := gjson.GetBytes(body, "session.identity.id").String() + require.NotEmpty(t, actualID) + assert.NotEqual(t, classic.ID.String(), actualID) + assert.Equal(t, "google-goodsystems", gjson.GetBytes(body, "session.authentication_methods.0.provider").String()) + if productID == "" { + productID = actualID + } else { + assert.Equal(t, productID, actualID) + } + + product, _, err := reg.PrivilegedIdentityPool().FindByCredentialsIdentifier(ctx, identity.CredentialsTypeOIDC, "google-goodsystems:"+subject) + require.NoError(t, err) + assert.Equal(t, productID, product.ID.String()) + original, _, err := reg.PrivilegedIdentityPool().FindByCredentialsIdentifier(ctx, identity.CredentialsTypeOIDC, "google:"+subject) + require.NoError(t, err) + assert.Equal(t, classic.ID, original.ID) + } + + // A browser-style submission has no ID token, so its provider stays exact. + status, body := submit("google", "") + require.Equal(t, http.StatusNotFound, status, "%s", body) + assert.Contains(t, string(body), "unknown or has not been configured") + + status, body = submit("google", "invalid-id-token") + require.GreaterOrEqual(t, status, http.StatusBadRequest, "%s", body) + assert.Contains(t, string(body), "Could not verify id_token") + assert.Empty(t, gjson.GetBytes(body, "session_token").String()) + + // A product deployment without the opt-in does not accept legacy Google. + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".oidc.config", &oidc.ConfigurationCollection{Providers: []oidc.Configuration{provider}}) + status, body = submit("google", token) + require.Equal(t, http.StatusNotFound, status, "%s", body) + status, body = submit("google-goodsystems", token) + require.Equal(t, http.StatusOK, status, "%s", body) + assert.Equal(t, productID, gjson.GetBytes(body, "session.identity.id").String()) + + // Classic still authenticates its original identity with the same subject. + provider.ID = "google" + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".oidc.config", &oidc.ConfigurationCollection{Providers: []oidc.Configuration{provider}}) + status, body = submit("google", token) + require.Equal(t, http.StatusOK, status, "%s", body) + assert.Equal(t, classic.ID.String(), gjson.GetBytes(body, "session.identity.id").String()) + assert.Equal(t, "google", gjson.GetBytes(body, "session.authentication_methods.0.provider").String()) + }) + } +} diff --git a/selfservice/strategy/oidc/provider_config.go b/selfservice/strategy/oidc/provider_config.go index 53053305a116..880c3914f5b2 100644 --- a/selfservice/strategy/oidc/provider_config.go +++ b/selfservice/strategy/oidc/provider_config.go @@ -17,12 +17,9 @@ import ( ) type Configuration struct { - // ID is the canonical provider ID stored in identity credentials and sessions. + // ID is the provider's ID ID string `json:"id"` - // PublicID preserves an existing request/form/callback identifier when ID changes. - PublicID string `json:"public_id,omitempty"` - // Provider is either "generic" for a generic OAuth 2.0 / OpenID Connect Provider or one of: // - generic // - google @@ -141,13 +138,6 @@ type Configuration struct { IDTokenVerificationEnabled bool `json:"id_token_verification_enabled"` } -func (p Configuration) RequestID() string { - if p.PublicID != "" { - return p.PublicID - } - return p.ID -} - func (p Configuration) Redir(public *url.URL) string { if p.PKCE == "force" { return urlx.AppendPaths(public, RouteCallbackGeneric).String() @@ -155,17 +145,19 @@ func (p Configuration) Redir(public *url.URL) string { if p.OrganizationID != "" { route := RouteOrganizationCallback - route = strings.Replace(route, ":provider", p.RequestID(), 1) + route = strings.Replace(route, ":provider", p.ID, 1) route = strings.Replace(route, ":organization", p.OrganizationID, 1) return urlx.AppendPaths(public, route).String() } - return urlx.AppendPaths(public, strings.Replace(RouteCallback, ":provider", p.RequestID(), 1)).String() + return urlx.AppendPaths(public, strings.Replace(RouteCallback, ":provider", p.ID, 1)).String() } type ConfigurationCollection struct { - BaseRedirectURI string `json:"base_redirect_uri"` - Providers []Configuration `json:"providers"` + // IDTokenProviderAliases maps legacy request IDs only for ID-token authentication. + IDTokenProviderAliases map[string]string `json:"id_token_provider_aliases,omitempty"` + BaseRedirectURI string `json:"base_redirect_uri"` + Providers []Configuration `json:"providers"` } // !!! WARNING !!! @@ -198,22 +190,15 @@ var supportedProviders = map[string]func(config *Configuration, reg Dependencies } func (c ConfigurationCollection) Provider(id string, reg Dependencies) (Provider, error) { - var matched *Configuration for k := range c.Providers { p := c.Providers[k] - if p.ID != id && (p.PublicID == "" || p.PublicID != id) { - continue - } - if matched != nil { - return nil, errors.Errorf("OpenID Connect Provider %q matches multiple configured providers", id) + if p.ID == id { + if f, ok := supportedProviders[p.Provider]; ok { + return f(&p, reg), nil + } + + return nil, errors.Errorf("provider type %s is not supported, supported are: %v", p.Provider, maps.Keys(supportedProviders)) } - matched = &p - } - if matched == nil { - return nil, errors.WithStack(herodot.ErrNotFound.WithReasonf(`OpenID Connect Provider "%s" is unknown or has not been configured`, id)) - } - if f, ok := supportedProviders[matched.Provider]; ok { - return f(matched, reg), nil } - return nil, errors.Errorf("provider type %s is not supported, supported are: %v", matched.Provider, maps.Keys(supportedProviders)) + return nil, errors.WithStack(herodot.ErrNotFound.WithReasonf(`OpenID Connect Provider "%s" is unknown or has not been configured`, id)) } diff --git a/selfservice/strategy/oidc/provider_config_test.go b/selfservice/strategy/oidc/provider_config_test.go index 9b91c3e6a6cb..ba9cc4a11ffe 100644 --- a/selfservice/strategy/oidc/provider_config_test.go +++ b/selfservice/strategy/oidc/provider_config_test.go @@ -7,15 +7,10 @@ import ( "bytes" "context" "encoding/json" - "net/url" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/tidwall/gjson" - - "github.com/ory/kratos/text" - "github.com/ory/kratos/ui/container" "github.com/ory/kratos/driver/config" "github.com/ory/kratos/identity" @@ -39,58 +34,3 @@ func TestConfig(t *testing.T) { require.Len(t, collection.Providers, 1) assert.Equal(t, "generic", collection.Providers[0].Provider) } - -func TestProviderPublicIDResolvesToCanonicalProvider(t *testing.T) { - var collection oidc.ConfigurationCollection - require.NoError(t, json.Unmarshal([]byte(`{"providers":[{"id":"google-goodsystems","public_id":"google","provider":"google","client_id":"goodmeet-client"}]}`), &collection)) - - provider, err := collection.Provider("google", nil) - require.NoError(t, err) - assert.Equal(t, "google-goodsystems", provider.Config().ID) - assert.Equal(t, "goodmeet-client", provider.Config().ClientID) -} - -func TestProviderPublicIDKeepsRegisteredCallback(t *testing.T) { - provider := oidc.Configuration{ID: "google-goodsystems", PublicID: "google"} - public, err := url.Parse("https://kratos.goodmeet.test/") - require.NoError(t, err) - - assert.Equal(t, "https://kratos.goodmeet.test/self-service/methods/oidc/callback/google", provider.Redir(public)) -} - -func TestProviderPublicIDKeepsBrowserSubmitValue(t *testing.T) { - ui := container.New("/self-service/login") - oidc.AddProviders(ui, []oidc.Configuration{{ID: "google-goodsystems", PublicID: "google"}}, text.NewInfoLoginWith) - raw, err := json.Marshal(ui.Nodes) - require.NoError(t, err) - assert.Equal(t, "google", gjson.GetBytes(raw, "0.attributes.value").String()) -} - -func TestProviderPublicIDRejectsAmbiguousSelection(t *testing.T) { - for _, providers := range [][]oidc.Configuration{ - {{ID: "google", Provider: "google"}, {ID: "google-goodsystems", PublicID: "google", Provider: "google"}}, - {{ID: "google-goodsystems", PublicID: "google", Provider: "google"}, {ID: "google", Provider: "google"}}, - } { - _, err := (oidc.ConfigurationCollection{Providers: providers}).Provider("google", nil) - require.Error(t, err) - } -} - -func TestProviderWithoutPublicIDKeepsExistingBehavior(t *testing.T) { - for _, id := range []string{"google", "google-goodsystems"} { - t.Run(id, func(t *testing.T) { - collection := oidc.ConfigurationCollection{Providers: []oidc.Configuration{{ID: id, Provider: "google", ClientID: "client"}}} - provider, err := collection.Provider(id, nil) - require.NoError(t, err) - assert.Equal(t, id, provider.Config().ID) - assert.Equal(t, id, provider.Config().RequestID()) - public, err := url.Parse("https://kratos.example.test") - require.NoError(t, err) - assert.Equal(t, "https://kratos.example.test/self-service/methods/oidc/callback/"+id, provider.Config().Redir(public)) - if id != "google" { - _, err = collection.Provider("google", nil) - require.Error(t, err) - } - }) - } -} diff --git a/selfservice/strategy/oidc/public_id_test.go b/selfservice/strategy/oidc/public_id_test.go deleted file mode 100644 index fc0b6f2837c2..000000000000 --- a/selfservice/strategy/oidc/public_id_test.go +++ /dev/null @@ -1,266 +0,0 @@ -// Copyright © 2026 Ory Corp -// SPDX-License-Identifier: Apache-2.0 - -package oidc_test - -import ( - "context" - "encoding/json" - "io" - "net/http" - "net/http/cookiejar" - "net/http/httptest" - "net/url" - "strings" - "testing" - "time" - - "github.com/golang-jwt/jwt/v4" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "github.com/tidwall/gjson" - - "github.com/ory/kratos/driver/config" - "github.com/ory/kratos/identity" - "github.com/ory/kratos/internal" - "github.com/ory/kratos/internal/testhelpers" - "github.com/ory/kratos/selfservice/flow" - "github.com/ory/kratos/selfservice/flow/login" - "github.com/ory/kratos/selfservice/strategy/oidc" - "github.com/ory/kratos/x" -) - -func TestPublicProviderIDNativeTokenRequests(t *testing.T) { - for _, entry := range []string{"login", "registration"} { - t.Run(entry, func(t *testing.T) { - ctx := context.Background() - conf, reg := internal.NewFastRegistryWithMocks(t) - jwks := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write(publicJWKS) - })) - t.Cleanup(jwks.Close) - oidc.RegisterTestGoogleProvider(t, "test-google", jwks.URL) - viperSetProviderConfig(t, conf, oidc.Configuration{ - ID: "google-goodsystems", PublicID: "google", Provider: "test-google", - ClientID: "goodmeet-client", Mapper: "file://./stub/oidc.hydra.jsonnet", PKCE: "never", - }) - conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationEnabled, true) - testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/stub.schema.json") - conf.MustSet(ctx, config.HookStrategyKey(config.ViperKeySelfServiceRegistrationAfter, identity.CredentialsTypeOIDC.String()), []config.SelfServiceHook{{Name: "session"}}) - server, _ := testhelpers.NewKratosServer(t, reg) - - subject := "goodmeet-" + entry + "@example.test" - classic := identity.NewIdentity("default") - classic.Traits = identity.Traits(`{"subject":"` + subject + `"}`) - classicCredentials, err := identity.NewCredentialsOIDC(new(identity.CredentialsOIDCEncryptedTokens), "google", subject, "") - require.NoError(t, err) - classic.SetCredentials(identity.CredentialsTypeOIDC, *classicCredentials) - require.NoError(t, reg.IdentityManager().Create(ctx, classic)) - - token := createIdToken(t, jwt.RegisteredClaims{ - Issuer: "https://accounts.google.com", Subject: subject, - Audience: jwt.ClaimStrings{"goodmeet-client"}, ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)), - }) - client := &http.Client{CheckRedirect: func(r *http.Request, via []*http.Request) error { return http.ErrUseLastResponse }} - var productID string - for _, providerID := range []string{"google", "google-goodsystems", "google"} { - response, err := client.Get(server.URL + "/self-service/" + entry + "/api") - require.NoError(t, err) - flowBody, err := io.ReadAll(response.Body) - require.NoError(t, response.Body.Close()) - require.NoError(t, err) - require.Equal(t, http.StatusOK, response.StatusCode, "%s", flowBody) - action := gjson.GetBytes(flowBody, "ui.action").String() - require.NotEmpty(t, action) - payload, err := json.Marshal(map[string]string{"method": "OIDC", "provider": providerID, "id_token": token}) - require.NoError(t, err) - request, err := http.NewRequest(http.MethodPost, action, strings.NewReader(string(payload))) - require.NoError(t, err) - request.Header.Set("Content-Type", "application/json") - request.Header.Set("Accept", "application/json") - response, err = client.Do(request) - require.NoError(t, err) - body, err := io.ReadAll(response.Body) - require.NoError(t, response.Body.Close()) - require.NoError(t, err) - require.Equal(t, http.StatusOK, response.StatusCode, "%s", body) - require.NotEmpty(t, gjson.GetBytes(body, "session_token").String()) - actualID := gjson.GetBytes(body, "session.identity.id").String() - require.NotEmpty(t, actualID) - assert.NotEqual(t, classic.ID.String(), actualID) - assert.Equal(t, "google-goodsystems", gjson.GetBytes(body, "session.authentication_methods.0.provider").String()) - if productID == "" { - productID = actualID - } else { - assert.Equal(t, productID, actualID) - } - - product, _, err := reg.PrivilegedIdentityPool().FindByCredentialsIdentifier(ctx, identity.CredentialsTypeOIDC, "google-goodsystems:"+subject) - require.NoError(t, err) - assert.Equal(t, productID, product.ID.String()) - original, _, err := reg.PrivilegedIdentityPool().FindByCredentialsIdentifier(ctx, identity.CredentialsTypeOIDC, "google:"+subject) - require.NoError(t, err) - assert.Equal(t, classic.ID, original.ID) - } - }) - } -} - -func TestPublicProviderIDBrowserCallbacks(t *testing.T) { - for _, selection := range []string{"form", "query"} { - t.Run(selection, func(t *testing.T) { - ctx := context.Background() - conf, reg := internal.NewFastRegistryWithMocks(t) - const subject = "browser-goodmeet@example.test" - var issuer *httptest.Server - var callback string - var browserToken string - issuer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/.well-known/openid-configuration": - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(map[string]any{ - "issuer": issuer.URL, "authorization_endpoint": issuer.URL + "/authorize", - "token_endpoint": issuer.URL + "/token", "jwks_uri": issuer.URL + "/keys", - "response_types_supported": []string{"code"}, "subject_types_supported": []string{"public"}, - "id_token_signing_alg_values_supported": []string{"RS256"}, - }) - case "/keys": - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write(publicJWKS) - case "/authorize": - if r.URL.Query().Get("redirect_uri") != callback { - http.Error(w, "unregistered callback", http.StatusBadRequest) - return - } - destination, _ := url.Parse(callback) - query := destination.Query() - query.Set("code", "synthetic-code") - query.Set("state", r.URL.Query().Get("state")) - destination.RawQuery = query.Encode() - http.Redirect(w, r, destination.String(), http.StatusSeeOther) - case "/token": - _ = r.ParseForm() - if r.Form.Get("redirect_uri") != callback || r.Form.Get("code") != "synthetic-code" { - http.Error(w, "invalid code exchange", http.StatusBadRequest) - return - } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(map[string]string{"access_token": "synthetic-token", "token_type": "Bearer", "id_token": browserToken}) - default: - http.NotFound(w, r) - } - })) - t.Cleanup(issuer.Close) - browserToken = createIdToken(t, jwt.RegisteredClaims{ - Issuer: issuer.URL, Subject: subject, Audience: jwt.ClaimStrings{"goodmeet-client"}, - ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)), - }) - viperSetProviderConfig(t, conf, oidc.Configuration{ - ID: "google-goodsystems", PublicID: "google", Provider: "generic", ClientID: "goodmeet-client", - ClientSecret: "synthetic-test-secret", IssuerURL: issuer.URL, Mapper: "file://./stub/oidc.hydra.jsonnet", PKCE: "never", - }) - conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationEnabled, true) - testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/stub.schema.json") - conf.MustSet(ctx, config.HookStrategyKey(config.ViperKeySelfServiceRegistrationAfter, identity.CredentialsTypeOIDC.String()), []config.SelfServiceHook{{Name: "session"}}) - conf.MustSet(ctx, config.ViperKeyClientHTTPNoPrivateIPRanges, false) - server, _ := testhelpers.NewKratosServer(t, reg) - callback = server.URL + "/self-service/methods/oidc/callback/google" - conf.MustSet(ctx, config.ViperKeySelfServiceLoginUI, server.URL+"/login-ui") - conf.MustSet(ctx, config.ViperKeySelfServiceBrowserDefaultReturnTo, server.URL+"/finished") - conf.MustSet(ctx, config.ViperKeyURLsAllowedReturnToDomains, []string{server.URL}) - jar, err := cookiejar.New(nil) - require.NoError(t, err) - client := &http.Client{Jar: jar, CheckRedirect: func(r *http.Request, via []*http.Request) error { return http.ErrUseLastResponse }} - - response, err := client.Get(server.URL + "/self-service/login/browser") - require.NoError(t, err) - require.NoError(t, response.Body.Close()) - require.Equal(t, http.StatusSeeOther, response.StatusCode) - location, err := response.Location() - require.NoError(t, err) - flowID := location.Query().Get("flow") - require.NotEmpty(t, flowID) - response, err = client.Get(server.URL + "/self-service/login/flows?id=" + flowID) - require.NoError(t, err) - raw, err := io.ReadAll(response.Body) - require.NoError(t, response.Body.Close()) - require.NoError(t, err) - require.Equal(t, http.StatusOK, response.StatusCode, "%s", raw) - assert.Equal(t, "google", gjson.GetBytes(raw, `ui.nodes.#(attributes.name=="provider").attributes.value`).String()) - action := gjson.GetBytes(raw, "ui.action").String() - form := url.Values{"method": {"oidc"}, "csrf_token": {"fake-csrf-token"}} - if selection == "form" { - form.Set("provider", "google") - } else { - action += "&provider=google" - } - response, err = client.PostForm(action, form) - require.NoError(t, err) - raw, err = io.ReadAll(response.Body) - require.NoError(t, response.Body.Close()) - require.NoError(t, err) - require.Equal(t, http.StatusSeeOther, response.StatusCode, "%s", raw) - authorize, err := response.Location() - require.NoError(t, err) - assert.Equal(t, callback, authorize.Query().Get("redirect_uri")) - response, err = client.Get(authorize.String()) - require.NoError(t, err) - require.NoError(t, response.Body.Close()) - require.Equal(t, http.StatusSeeOther, response.StatusCode) - returned, err := response.Location() - require.NoError(t, err) - assert.Equal(t, "/self-service/methods/oidc/callback/google", returned.Path) - mismatch := *returned - mismatch.Path = "/self-service/methods/oidc/callback/google-goodsystems" - request, err := http.NewRequest(http.MethodGet, mismatch.String(), nil) - require.NoError(t, err) - request.Header.Set("Accept", "application/json") - response, err = client.Do(request) - require.NoError(t, err) - raw, err = io.ReadAll(response.Body) - require.NoError(t, response.Body.Close()) - require.NoError(t, err) - require.Equal(t, http.StatusBadRequest, response.StatusCode, "%s", raw) - assert.Contains(t, string(raw), "provider mismatch") - response, err = client.Get(returned.String()) - require.NoError(t, err) - raw, err = io.ReadAll(response.Body) - require.NoError(t, response.Body.Close()) - require.NoError(t, err) - require.Equal(t, http.StatusSeeOther, response.StatusCode, "%s", raw) - finished, err := response.Location() - require.NoError(t, err) - assert.Equal(t, "/finished", finished.Path) - response, err = client.Get(server.URL + "/sessions/whoami") - require.NoError(t, err) - raw, err = io.ReadAll(response.Body) - require.NoError(t, response.Body.Close()) - require.NoError(t, err) - require.Equal(t, http.StatusOK, response.StatusCode, "%s", raw) - assert.Equal(t, "google-goodsystems", gjson.GetBytes(raw, "authentication_methods.0.provider").String()) - product, _, err := reg.PrivilegedIdentityPool().FindByCredentialsIdentifier(ctx, identity.CredentialsTypeOIDC, "google-goodsystems:"+subject) - require.NoError(t, err) - assert.Equal(t, product.ID.String(), gjson.GetBytes(raw, "identity.id").String()) - }) - } -} - -func TestPublicProviderIDIdentifierFirstForm(t *testing.T) { - ctx := context.Background() - conf, reg := internal.NewFastRegistryWithMocks(t) - testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/stub.schema.json") - conf.MustSet(ctx, config.ViperKeySecurityAccountEnumerationMitigate, false) - viperSetProviderConfig(t, conf, oidc.Configuration{ - ID: "google-goodsystems", PublicID: "google", Provider: "google", ClientID: "goodmeet-client", - }) - id := createIdentity(t, ctx, reg, x.NewUUID(), "google-goodsystems") - request := httptest.NewRequest(http.MethodGet, "/self-service/login/browser", nil) - f, err := login.NewFlow(conf, time.Minute, "csrf", request, flow.TypeBrowser) - require.NoError(t, err) - require.NoError(t, oidc.NewStrategy(reg).PopulateLoginMethodIdentifierFirstCredentials(request, f, login.WithIdentityHint(id))) - raw, err := json.Marshal(f.UI.Nodes) - require.NoError(t, err) - assert.Equal(t, "google", gjson.GetBytes(raw, `#(attributes.name=="provider").attributes.value`).String()) -} diff --git a/selfservice/strategy/oidc/state.go b/selfservice/strategy/oidc/state.go index 1149a9af6811..ce8cb17bc653 100644 --- a/selfservice/strategy/oidc/state.go +++ b/selfservice/strategy/oidc/state.go @@ -42,7 +42,7 @@ func (s *Strategy) GenerateState(ctx context.Context, p Provider, flowID uuid.UU state := oidcv1.State{ FlowId: flowID.Bytes(), SessionTokenExchangeCodeSha512: x.NewUUID().Bytes(), - ProviderId: p.Config().RequestID(), + ProviderId: p.Config().ID, PkceVerifier: maybePKCE(ctx, s.d, p), } if code, hasCode, _ := s.d.SessionTokenExchangePersister().CodeForFlow(ctx, flowID); hasCode { diff --git a/selfservice/strategy/oidc/state_test.go b/selfservice/strategy/oidc/state_test.go index 4b7d61b1cd40..bf0366571a6b 100644 --- a/selfservice/strategy/oidc/state_test.go +++ b/selfservice/strategy/oidc/state_test.go @@ -44,17 +44,3 @@ type testProvider struct{} func (t *testProvider) Config() *oidc.Configuration { return &oidc.Configuration{ID: "test-provider", PKCE: "never"} } - -func TestGenerateStateWithPublicProviderID(t *testing.T) { - _, reg := internal.NewFastRegistryWithMocks(t) - strat := oidc.NewStrategy(reg) - ctx := context.Background() - provider := oidc.NewProviderGoogle(&oidc.Configuration{ID: "google-goodsystems", PublicID: "google", PKCE: "never"}, reg) - - encrypted, _, err := strat.GenerateState(ctx, provider, x.NewUUID()) - require.NoError(t, err) - state, err := oidc.DecryptState(ctx, reg.Cipher(ctx), encrypted) - require.NoError(t, err) - assert.Equal(t, "google", state.ProviderId) - assert.Equal(t, "google-goodsystems", provider.Config().ID) -} diff --git a/selfservice/strategy/oidc/strategy.go b/selfservice/strategy/oidc/strategy.go index 901579aa4c00..c228b256ab22 100644 --- a/selfservice/strategy/oidc/strategy.go +++ b/selfservice/strategy/oidc/strategy.go @@ -544,13 +544,20 @@ func (s *Strategy) Config(ctx context.Context) (*ConfigurationCollection, error) } func (s *Strategy) provider(ctx context.Context, id string) (Provider, error) { - if c, err := s.Config(ctx); err != nil { - return nil, err - } else if provider, err := c.Provider(id, s.d); err != nil { + return s.providerForAuthentication(ctx, id, false) +} + +func (s *Strategy) providerForAuthentication(ctx context.Context, id string, hasIDToken bool) (Provider, error) { + c, err := s.Config(ctx) + if err != nil { return nil, err - } else { - return provider, nil } + if hasIDToken { + if canonicalID, ok := c.IDTokenProviderAliases[id]; ok { + id = canonicalID + } + } + return c.Provider(id, s.d) } func (s *Strategy) forwardError(ctx context.Context, w http.ResponseWriter, r *http.Request, f flow.Flow, err error) { diff --git a/selfservice/strategy/oidc/strategy_login.go b/selfservice/strategy/oidc/strategy_login.go index 0bb34ec3ab1b..59c55f78ccbd 100644 --- a/selfservice/strategy/oidc/strategy_login.go +++ b/selfservice/strategy/oidc/strategy_login.go @@ -232,7 +232,7 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, return nil, s.handleError(ctx, w, r, f, pid, nil, err) } - provider, err := s.provider(ctx, pid) + provider, err := s.providerForAuthentication(ctx, pid, p.IDToken != "") if err != nil { return nil, s.handleError(ctx, w, r, f, pid, nil, err) } @@ -388,7 +388,7 @@ func (s *Strategy) PopulateLoginMethodIdentifierFirstCredentials(r *http.Request for _, l := range linked { lc := l.Config() - AddProvider(f.UI, lc.RequestID(), text.NewInfoLoginWith(stringsx.Coalesce(lc.Label, lc.RequestID()), lc.RequestID())) + AddProvider(f.UI, lc.ID, text.NewInfoLoginWith(stringsx.Coalesce(lc.Label, lc.ID), lc.ID)) } } diff --git a/selfservice/strategy/oidc/strategy_registration.go b/selfservice/strategy/oidc/strategy_registration.go index 4edab83a351e..1698d6a1fce1 100644 --- a/selfservice/strategy/oidc/strategy_registration.go +++ b/selfservice/strategy/oidc/strategy_registration.go @@ -184,7 +184,7 @@ func (s *Strategy) Register(w http.ResponseWriter, r *http.Request, f *registrat return s.handleError(ctx, w, r, f, pid, nil, err) } - provider, err := s.provider(ctx, pid) + provider, err := s.providerForAuthentication(ctx, pid, p.IDToken != "") if err != nil { return s.handleError(ctx, w, r, f, pid, nil, err) } diff --git a/selfservice/strategy/oidc/types.go b/selfservice/strategy/oidc/types.go index e42d66d67e14..768e20d23531 100644 --- a/selfservice/strategy/oidc/types.go +++ b/selfservice/strategy/oidc/types.go @@ -20,7 +20,7 @@ type FlowMethod struct { func AddProviders(c *container.Container, providers []Configuration, message func(provider string, providerId string) *text.Message) { for _, p := range providers { - AddProvider(c, p.RequestID(), message(stringsx.Coalesce(p.Label, p.RequestID()), p.RequestID())) + AddProvider(c, p.ID, message(stringsx.Coalesce(p.Label, p.ID), p.ID)) } } diff --git a/test/schema/fixtures/config.schema.test.failure/root.idTokenProviderAliasEmptyTarget.yaml b/test/schema/fixtures/config.schema.test.failure/root.idTokenProviderAliasEmptyTarget.yaml new file mode 100644 index 000000000000..6f91a209cd22 --- /dev/null +++ b/test/schema/fixtures/config.schema.test.failure/root.idTokenProviderAliasEmptyTarget.yaml @@ -0,0 +1,22 @@ +serve: + public: + base_url: "#/definitions/baseUrl" + +selfservice: + methods: + oidc: + config: + id_token_provider_aliases: + google: "" + default_browser_return_url: "#/definitions/defaultReturnTo" + +dsn: foo + +identity: + schemas: + - id: default + url: https://example.com + +session: + cookie: + domain: foobar diff --git a/test/schema/fixtures/config.schema.test.failure/root.idTokenProviderAliasInvalidTarget.yaml b/test/schema/fixtures/config.schema.test.failure/root.idTokenProviderAliasInvalidTarget.yaml new file mode 100644 index 000000000000..fe07f3a2c5c3 --- /dev/null +++ b/test/schema/fixtures/config.schema.test.failure/root.idTokenProviderAliasInvalidTarget.yaml @@ -0,0 +1,22 @@ +serve: + public: + base_url: "#/definitions/baseUrl" + +selfservice: + methods: + oidc: + config: + id_token_provider_aliases: + google: 123 + default_browser_return_url: "#/definitions/defaultReturnTo" + +dsn: foo + +identity: + schemas: + - id: default + url: https://example.com + +session: + cookie: + domain: foobar diff --git a/test/schema/fixtures/config.schema.test.failure/selfServiceOIDCProvider.emptyPublicID.yaml b/test/schema/fixtures/config.schema.test.failure/selfServiceOIDCProvider.emptyPublicID.yaml deleted file mode 100644 index 7e14e20a290f..000000000000 --- a/test/schema/fixtures/config.schema.test.failure/selfServiceOIDCProvider.emptyPublicID.yaml +++ /dev/null @@ -1,6 +0,0 @@ -id: google-goodsystems -public_id: "" -provider: google -client_id: goodmeet-client -client_secret: synthetic-test-secret -mapper_url: file:///etc/config/google.jsonnet diff --git a/test/schema/fixtures/config.schema.test.failure/selfServiceOIDCProvider.invalidPublicID.yaml b/test/schema/fixtures/config.schema.test.failure/selfServiceOIDCProvider.invalidPublicID.yaml deleted file mode 100644 index c20cbe62f955..000000000000 --- a/test/schema/fixtures/config.schema.test.failure/selfServiceOIDCProvider.invalidPublicID.yaml +++ /dev/null @@ -1,6 +0,0 @@ -id: google-goodsystems -public_id: 42 -provider: google -client_id: goodmeet-client -client_secret: synthetic-test-secret -mapper_url: file:///etc/config/google.jsonnet diff --git a/test/schema/fixtures/config.schema.test.success/root.idTokenProviderAliases.yaml b/test/schema/fixtures/config.schema.test.success/root.idTokenProviderAliases.yaml new file mode 100644 index 000000000000..258b0ff2ae8c --- /dev/null +++ b/test/schema/fixtures/config.schema.test.success/root.idTokenProviderAliases.yaml @@ -0,0 +1,22 @@ +serve: + public: + base_url: "#/definitions/baseUrl" + +selfservice: + methods: + oidc: + config: + id_token_provider_aliases: + google: google-goodsystems + default_browser_return_url: "#/definitions/defaultReturnTo" + +dsn: foo + +identity: + schemas: + - id: default + url: https://example.com + +session: + cookie: + domain: foobar diff --git a/test/schema/fixtures/config.schema.test.success/selfServiceOIDCProvider.publicID.yaml b/test/schema/fixtures/config.schema.test.success/selfServiceOIDCProvider.publicID.yaml deleted file mode 100644 index 2bd8b48026b4..000000000000 --- a/test/schema/fixtures/config.schema.test.success/selfServiceOIDCProvider.publicID.yaml +++ /dev/null @@ -1,6 +0,0 @@ -id: google-goodsystems -public_id: google -provider: google -client_id: goodmeet-client -client_secret: synthetic-test-secret -mapper_url: file:///etc/config/google.jsonnet