Skip to content

Commit f24cf8e

Browse files
committed
feat: use subject_source pattern for OAuth2 provider subject
Replace `use_external_id` boolean config with `subject_source` enum to match the existing tokenizer pattern. The new config accepts: - "id" (default): Use identity ID as OAuth2 subject - "external_id": Use identity's external_id as OAuth2 subject Returns an error when `subject_source` is set to "external_id" but the identity's external_id is unset, ensuring predictable behavior and making it easier to identify which ID was used. This aligns the OAuth2 provider configuration with the session tokenizer implementation for consistency across the codebase. Closes #4528
1 parent 57dcfe1 commit f24cf8e

7 files changed

Lines changed: 68 additions & 20 deletions

File tree

driver/config/config.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ const (
192192
ViperKeyOAuth2ProviderURL = "oauth2_provider.url"
193193
ViperKeyOAuth2ProviderHeader = "oauth2_provider.headers"
194194
ViperKeyOAuth2ProviderOverrideReturnTo = "oauth2_provider.override_return_to"
195-
ViperKeyOAuth2ProviderUseExternalID = "oauth2_provider.use_external_id"
195+
ViperKeyOAuth2ProviderSubjectSource = "oauth2_provider.subject_source"
196196
ViperKeyClientHTTPNoPrivateIPRanges = "clients.http.disallow_private_ip_ranges"
197197
ViperKeyClientHTTPPrivateIPExceptionURLs = "clients.http.private_ip_exception_urls"
198198
ViperKeyWebhookHeaderAllowlist = "clients.web_hook.header_allowlist"
@@ -961,8 +961,8 @@ func (p *Config) OAuth2ProviderOverrideReturnTo(ctx context.Context) bool {
961961
return p.GetProvider(ctx).Bool(ViperKeyOAuth2ProviderOverrideReturnTo)
962962
}
963963

964-
func (p *Config) OAuth2ProviderUseExternalID(ctx context.Context) bool {
965-
return p.GetProvider(ctx).Bool(ViperKeyOAuth2ProviderUseExternalID)
964+
func (p *Config) OAuth2ProviderSubjectSource(ctx context.Context) string {
965+
return p.GetProvider(ctx).String(ViperKeyOAuth2ProviderSubjectSource)
966966
}
967967

968968
func (p *Config) OAuth2ProviderURL(ctx context.Context) *url.URL {

driver/config/config_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1299,15 +1299,15 @@ func TestOAuth2Provider(t *testing.T) {
12991299
assert.Equal(t, "https://oauth2_provider/", conf.OAuth2ProviderURL(ctx).String())
13001300
assert.Equal(t, http.Header{"Authorization": {"Basic"}}, conf.OAuth2ProviderHeader(ctx))
13011301
assert.True(t, conf.OAuth2ProviderOverrideReturnTo(ctx))
1302-
assert.True(t, conf.OAuth2ProviderUseExternalID(ctx))
1302+
assert.Equal(t, "external_id", conf.OAuth2ProviderSubjectSource(ctx))
13031303
})
13041304

13051305
t.Run("case=defaults", func(t *testing.T) {
13061306
conf, _ := config.New(ctx, logrusx.New("", ""), os.Stderr, &contextx.Default{}, configx.SkipValidation())
13071307
assert.Empty(t, conf.OAuth2ProviderURL(ctx))
13081308
assert.Empty(t, conf.OAuth2ProviderHeader(ctx))
13091309
assert.False(t, conf.OAuth2ProviderOverrideReturnTo(ctx))
1310-
assert.False(t, conf.OAuth2ProviderUseExternalID(ctx))
1310+
assert.Equal(t, "id", conf.OAuth2ProviderSubjectSource(ctx))
13111311
})
13121312
}
13131313

driver/config/stub/.kratos.oauth2_provider.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@ oauth2_provider:
33
headers:
44
Authorization: Basic
55
override_return_to: true
6-
use_external_id: true
6+
subject_source: external_id

embedx/config.schema.json

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2286,11 +2286,12 @@
22862286
"default": false,
22872287
"description": "Override the return_to query parameter with the OAuth2 provider request URL when perfoming an OAuth2 login flow."
22882288
},
2289-
"use_external_id": {
2290-
"title": "Use external_id as subject",
2291-
"type": "boolean",
2292-
"default": false,
2293-
"description": "If set, the external_id of the identity will be used as the subject in the OAuth2 login request. If no external_id is set, the identity ID will be used."
2289+
"subject_source": {
2290+
"title": "Subject source for OAuth2 login",
2291+
"type": "string",
2292+
"enum": ["id", "external_id"],
2293+
"default": "id",
2294+
"description": "Determines which identifier to use as the subject in OAuth2 login requests. Can be either 'id' (identity ID, default) or 'external_id' (identity's external ID). If 'external_id' is selected but not set on the identity, an error will be returned."
22942295
}
22952296
},
22962297
"additionalProperties": false

hydra/fake.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@ const (
2020
var ErrFakeAcceptLoginRequestFailed = errors.New("failed to accept login request")
2121

2222
type FakeHydra struct {
23-
Skip bool
24-
RequestURL string
25-
params []AcceptLoginRequestParams
23+
Skip bool
24+
RequestURL string
25+
SubjectSource string
26+
params []AcceptLoginRequestParams
2627
}
2728

2829
func (h *FakeHydra) Params() []AcceptLoginRequestParams {
@@ -42,6 +43,19 @@ func (h *FakeHydra) AcceptLoginRequest(_ context.Context, params AcceptLoginRequ
4243
if params.SessionID == "" {
4344
return "", errors.New("session id must not be empty")
4445
}
46+
47+
// Validate subject source just like DefaultHydra does
48+
switch h.SubjectSource {
49+
case "", "id":
50+
// Use identity ID - no validation needed
51+
case "external_id":
52+
if params.ExternalID == "" {
53+
return "", herodot.ErrBadRequest.WithReasonf("The identity does not have an external ID set, but it is required for the OAuth2 provider subject.")
54+
}
55+
default:
56+
return "", herodot.ErrBadRequest.WithReasonf("Unknown OAuth2 provider subject source %q", h.SubjectSource)
57+
}
58+
4559
switch params.LoginChallenge {
4660
case FakeInvalidLoginChallenge:
4761
return "", ErrFakeAcceptLoginRequestFailed

hydra/hydra.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,17 @@ func (h *DefaultHydra) AcceptLoginRequest(ctx context.Context, params AcceptLogi
9494
remember := h.d.Config().SessionPersistentCookie(ctx)
9595
rememberFor := int64(h.d.Config().SessionLifespan(ctx) / time.Second)
9696

97-
subject := params.IdentityID
98-
if h.d.Config().OAuth2ProviderUseExternalID(ctx) && params.ExternalID != "" {
97+
var subject string
98+
switch h.d.Config().OAuth2ProviderSubjectSource(ctx) {
99+
case "", "id":
100+
subject = params.IdentityID
101+
case "external_id":
102+
if params.ExternalID == "" {
103+
return "", errors.WithStack(herodot.ErrBadRequest.WithReasonf("The identity does not have an external ID set, but it is required for the OAuth2 provider subject."))
104+
}
99105
subject = params.ExternalID
106+
default:
107+
return "", errors.WithStack(herodot.ErrBadRequest.WithReasonf("Unknown OAuth2 provider subject source %q", h.d.Config().OAuth2ProviderSubjectSource(ctx)))
100108
}
101109

102110
alr := hydraclientgo.NewAcceptOAuth2LoginRequest(subject)

selfservice/flow/login/hook_external_id_test.go

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,9 @@ func TestLoginExecutorWithExternalID(t *testing.T) {
4444
}
4545
require.NoError(t, reg.Persister().CreateIdentity(ctx, i))
4646

47-
t.Run("case=use_external_id=false", func(t *testing.T) {
48-
conf.MustSet(ctx, config.ViperKeyOAuth2ProviderUseExternalID, false)
47+
t.Run("case=subject_source=id", func(t *testing.T) {
48+
conf.MustSet(ctx, config.ViperKeyOAuth2ProviderSubjectSource, "id")
49+
fakeHydra.SubjectSource = "id"
4950
loginFlow, err := login.NewFlow(conf, time.Minute, hydra.FakeValidLoginChallenge, &http.Request{URL: &url.URL{Path: "/", RawQuery: "login_challenge=" + hydra.FakeValidLoginChallenge}}, flow.TypeBrowser)
5051
require.NoError(t, err)
5152
loginFlow.OAuth2LoginChallenge = hydra.FakeValidLoginChallenge
@@ -63,8 +64,9 @@ func TestLoginExecutorWithExternalID(t *testing.T) {
6364
assert.Equal(t, "external-id", fakeHydra.Params()[0].ExternalID)
6465
})
6566

66-
t.Run("case=use_external_id=true", func(t *testing.T) {
67-
conf.MustSet(ctx, config.ViperKeyOAuth2ProviderUseExternalID, true)
67+
t.Run("case=subject_source=external_id", func(t *testing.T) {
68+
conf.MustSet(ctx, config.ViperKeyOAuth2ProviderSubjectSource, "external_id")
69+
fakeHydra.SubjectSource = "external_id"
6870
loginFlow, err := login.NewFlow(conf, time.Minute, hydra.FakeValidLoginChallenge, &http.Request{URL: &url.URL{Path: "/", RawQuery: "login_challenge=" + hydra.FakeValidLoginChallenge}}, flow.TypeBrowser)
6971
require.NoError(t, err)
7072
loginFlow.OAuth2LoginChallenge = hydra.FakeValidLoginChallenge
@@ -85,4 +87,27 @@ func TestLoginExecutorWithExternalID(t *testing.T) {
8587
assert.Equal(t, i.ID.String(), lastParams.IdentityID)
8688
assert.Equal(t, "external-id", lastParams.ExternalID)
8789
})
90+
91+
t.Run("case=subject_source=external_id without external_id set", func(t *testing.T) {
92+
iWithoutExtID := &identity.Identity{
93+
ID: uuid.Must(uuid.NewV4()),
94+
SchemaID: config.DefaultIdentityTraitsSchemaID,
95+
State: identity.StateActive,
96+
}
97+
require.NoError(t, reg.Persister().CreateIdentity(ctx, iWithoutExtID))
98+
99+
conf.MustSet(ctx, config.ViperKeyOAuth2ProviderSubjectSource, "external_id")
100+
fakeHydra.SubjectSource = "external_id"
101+
loginFlow, err := login.NewFlow(conf, time.Minute, hydra.FakeValidLoginChallenge, &http.Request{URL: &url.URL{Path: "/", RawQuery: "login_challenge=" + hydra.FakeValidLoginChallenge}}, flow.TypeBrowser)
102+
require.NoError(t, err)
103+
loginFlow.OAuth2LoginChallenge = hydra.FakeValidLoginChallenge
104+
105+
w := httptest.NewRecorder()
106+
r := &http.Request{URL: &url.URL{Path: "/login/post"}}
107+
sess := session.NewInactiveSession()
108+
sess.CompletedLoginFor(identity.CredentialsTypePassword, identity.AuthenticatorAssuranceLevel1)
109+
110+
err = reg.LoginHookExecutor().PostLoginHook(w, r, identity.CredentialsTypePassword.ToUiNodeGroup(), loginFlow, iWithoutExtID, sess, "")
111+
require.Error(t, err)
112+
})
88113
}

0 commit comments

Comments
 (0)