Skip to content

feat: allow using identity external_id as oauth2 subject #4529

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.

Sign up for GitHub

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

Open
Micaso wants to merge 2 commits into
base: master
Choose a base branch
from
+168 −3
Open

feat: allow using identity external_id as oauth2 subject #4529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
57dcfe1
feat: allow using identity external_id as oauth2 subject
Micaso Jan 23, 2026
afad954
feat: use subject_source pattern for OAuth2 provider subject
Micaso Feb 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions driver/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ const (
ViperKeyOAuth2ProviderURL = "oauth2_provider.url"
ViperKeyOAuth2ProviderHeader = "oauth2_provider.headers"
ViperKeyOAuth2ProviderOverrideReturnTo = "oauth2_provider.override_return_to"
ViperKeyOAuth2ProviderSubjectSource = "oauth2_provider.subject_source"
ViperKeyClientHTTPNoPrivateIPRanges = "clients.http.disallow_private_ip_ranges"
ViperKeyClientHTTPPrivateIPExceptionURLs = "clients.http.private_ip_exception_urls"
ViperKeyWebhookHeaderAllowlist = "clients.web_hook.header_allowlist"
Expand Down Expand Up @@ -960,6 +961,10 @@ func (p *Config) OAuth2ProviderOverrideReturnTo(ctx context.Context) bool {
return p.GetProvider(ctx).Bool(ViperKeyOAuth2ProviderOverrideReturnTo)
}

func (p *Config) OAuth2ProviderSubjectSource(ctx context.Context) string {
return p.GetProvider(ctx).String(ViperKeyOAuth2ProviderSubjectSource)
}

func (p *Config) OAuth2ProviderURL(ctx context.Context) *url.URL {
k := ViperKeyOAuth2ProviderURL
v := p.GetProvider(ctx).String(k)
Expand Down
2 changes: 2 additions & 0 deletions driver/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1299,13 +1299,15 @@ func TestOAuth2Provider(t *testing.T) {
assert.Equal(t, "https://oauth2_provider/", conf.OAuth2ProviderURL(ctx).String())
assert.Equal(t, http.Header{"Authorization": {"Basic"}}, conf.OAuth2ProviderHeader(ctx))
assert.True(t, conf.OAuth2ProviderOverrideReturnTo(ctx))
assert.Equal(t, "external_id", conf.OAuth2ProviderSubjectSource(ctx))
})

t.Run("case=defaults", func(t *testing.T) {
conf, _ := config.New(ctx, logrusx.New("", ""), os.Stderr, &contextx.Default{}, configx.SkipValidation())
assert.Empty(t, conf.OAuth2ProviderURL(ctx))
assert.Empty(t, conf.OAuth2ProviderHeader(ctx))
assert.False(t, conf.OAuth2ProviderOverrideReturnTo(ctx))
assert.Equal(t, "id", conf.OAuth2ProviderSubjectSource(ctx))
})
}

Expand Down
1 change: 1 addition & 0 deletions driver/config/stub/.kratos.oauth2_provider.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ oauth2_provider:
headers:
Authorization: Basic
override_return_to: true
subject_source: external_id
7 changes: 7 additions & 0 deletions embedx/config.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2285,6 +2285,13 @@
"type": "boolean",
"default": false,
"description": "Override the return_to query parameter with the OAuth2 provider request URL when perfoming an OAuth2 login flow."
},
"subject_source": {
"title": "Subject source for OAuth2 login",
"type": "string",
"enum": ["id", "external_id"],
"default": "id",
"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."
Comment on lines +2289 to +2294

@coderabbitai coderabbitai Bot Apr 28, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

external_id error semantics conflict with the stated fallback compatibility behavior.

Line 2294 documents a hard error when external_id is missing, but the PR objectives require fallback to identity UUID for backward compatibility. Please align this config contract with fallback behavior.

Suggested schema wording update 
-          "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."
+          "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 missing on the identity, Kratos falls back to the identity ID."
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"subject_source": {
"title": "Subject source for OAuth2 login",
"type": "string",
"enum": ["id", "external_id"],
"default": "id",
"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."
"subject_source": {
"title": "Subject source for OAuth2 login",
"type": "string",
"enum": ["id", "external_id"],
"default": "id",
"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 missing on the identity, Kratos falls back to the identity ID."
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@embedx/config.schema.json` around lines 2289 - 2294, The schema description
for the "subject_source" property currently claims a hard error when
"external_id" is missing; update the description to reflect the actual fallback
behavior (use the identity UUID/"id" when external_id is unset) so the config
contract matches runtime semantics. Locate the "subject_source" JSON schema
entry and replace the description text to state that "external_id" will be used
when present otherwise the system falls back to the identity "id" (no error),
and keep the enum/default values unchanged.

@Micaso Micaso Apr 28, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The error behavior is intentional — @jonas-jonas explicitly requested in his review that we return an error (rather than fall back) when external_id is missing, to match the tokenizer pattern and keep behavior predictable. The description is correct as-is.

@coderabbitai coderabbitai Bot Apr 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

}
},
"additionalProperties": false
Expand Down
26 changes: 24 additions & 2 deletions hydra/fake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,16 @@ const (
var ErrFakeAcceptLoginRequestFailed = errors.New("failed to accept login request")

type FakeHydra struct {
Skip bool
RequestURL string
Skip bool
RequestURL string
SubjectSource string
params []AcceptLoginRequestParams
}

func (h *FakeHydra) Params() []AcceptLoginRequestParams {
out := make([]AcceptLoginRequestParams, len(h.params))
copy(out, h.params)
return out
}

var _ Hydra = &FakeHydra{}
Expand All @@ -33,9 +41,23 @@ func NewFake() *FakeHydra {
}

func (h *FakeHydra) AcceptLoginRequest(_ context.Context, params AcceptLoginRequestParams) (string, error) {
h.params = append(h.params, params)
if params.SessionID == "" {
return "", errors.New("session id must not be empty")
}

// Validate subject source just like DefaultHydra does
switch h.SubjectSource {
case "", "id":
// Use identity ID - no validation needed
case "external_id":
if params.ExternalID == "" {
return "", herodot.ErrBadRequest.WithReasonf("The identity does not have an external ID set, but it is required for the OAuth2 provider subject.")
}
Comment on lines +53 to +56

@coderabbitai coderabbitai Bot Apr 28, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

FakeHydra enforces hard failure instead of fallback for missing external_id.

Line 54-Line 56 rejects requests when external_id is missing, which contradicts the fallback behavior required for compatibility. The fake should mirror fallback behavior so tests validate the intended contract.

Suggested behavior alignment 
 	case "external_id":
-		if params.ExternalID == "" {
-			return "", herodot.ErrBadRequest.WithReasonf("The identity does not have an external ID set, but it is required for the OAuth2 provider subject.")
-		}
+		// Allow missing external_id and fall back to identity ID behavior.
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@hydra/fake.go` around lines 53 - 56, The fake hydra implementation currently
returns a hard error in the "external_id" case when params.ExternalID is empty;
change this to mirror real Hydra's fallback behavior by returning a derived
subject instead of failing — detect if params.ExternalID == "" and then return
params.Subject (or another existing identifier field used as the OAuth2 subject
in this fake) as the fallback value; update the switch/case handling in fake.go
(the "external_id" branch) to use params.ExternalID when present and fall back
to params.Subject (or the equivalent identifier field) when not, rather than
returning herodot.ErrBadRequest.

@Micaso Micaso Apr 28, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Same reasoning as above — the fake intentionally mirrors the real DefaultHydra behavior, which returns an error when external_id is missing. This is by design per the reviewer's feedback, and the test in hook_external_id_test.go specifically validates this error path.

@coderabbitai coderabbitai Bot Apr 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@Micaso, you're right — I apologize for the incorrect review comment. Looking at the DefaultHydra implementation in hydra/hydra.go, it also returns herodot.ErrBadRequest when subject_source is "external_id" but ExternalID is empty (no fallback). The FakeHydra correctly mirrors this contract, and the schema description likewise documents that an error is expected in this case. The test in hook_external_id_test.go validating this error path confirms the intended behavior. My suggestion to add a fallback was mistaken.

✏️ Learnings added 
Learnt from: Micaso
Repo: ory/kratos PR: 4529
File: hydra/fake.go:53-56
Timestamp: 2026-04-28T09:16:22.325Z
Learning: In the ory/kratos repository, `FakeHydra` in `hydra/fake.go` intentionally mirrors the real `DefaultHydra` behavior: when `SubjectSource` is `"external_id"` and `params.ExternalID` is empty, it returns `herodot.ErrBadRequest` (no fallback to identity ID). This is by design — the schema (`embedx/config.schema.json`) documents that using `external_id` requires the identity to have an external ID set, otherwise an error is expected. The test `TestLoginExecutorWithExternalID` in `selfservice/flow/login/hook_external_id_test.go` validates this error path.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

default:
return "", herodot.ErrBadRequest.WithReasonf("Unknown OAuth2 provider subject source %q", h.SubjectSource)
}

switch params.LoginChallenge {
case FakeInvalidLoginChallenge:
return "", ErrFakeAcceptLoginRequestFailed
Expand Down
16 changes: 15 additions & 1 deletion hydra/hydra.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type (
AcceptLoginRequestParams struct {
LoginChallenge string
IdentityID string
ExternalID string
SessionID string
AuthenticationMethods session.AuthenticationMethods
}
Expand Down Expand Up @@ -93,7 +94,20 @@ func (h *DefaultHydra) AcceptLoginRequest(ctx context.Context, params AcceptLogi
remember := h.d.Config().SessionPersistentCookie(ctx)
rememberFor := int64(h.d.Config().SessionLifespan(ctx) / time.Second)

alr := hydraclientgo.NewAcceptOAuth2LoginRequest(params.IdentityID)
var subject string
switch h.d.Config().OAuth2ProviderSubjectSource(ctx) {
case "", "id":
subject = params.IdentityID
case "external_id":
if params.ExternalID == "" {
return "", errors.WithStack(herodot.ErrBadRequest.WithReasonf("The identity does not have an external ID set, but it is required for the OAuth2 provider subject."))
}
subject = params.ExternalID
default:
return "", errors.WithStack(herodot.ErrBadRequest.WithReasonf("Unknown OAuth2 provider subject source %q", h.d.Config().OAuth2ProviderSubjectSource(ctx)))
}

alr := hydraclientgo.NewAcceptOAuth2LoginRequest(subject)
alr.IdentityProviderSessionId = &params.SessionID
alr.Remember = &remember
alr.RememberFor = &rememberFor
Expand Down
2 changes: 2 additions & 0 deletions selfservice/flow/login/hook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ func (e *HookExecutor) PostLoginHook(
hydra.AcceptLoginRequestParams{
LoginChallenge: string(f.OAuth2LoginChallenge),
IdentityID: i.ID.String(),
ExternalID: string(i.ExternalID),
SessionID: s.ID.String(),
AuthenticationMethods: s.AMR,
})
Expand Down Expand Up @@ -373,6 +374,7 @@ func (e *HookExecutor) PostLoginHook(
hydra.AcceptLoginRequestParams{
LoginChallenge: string(f.OAuth2LoginChallenge),
IdentityID: i.ID.String(),
ExternalID: string(i.ExternalID),
SessionID: s.ID.String(),
AuthenticationMethods: s.AMR,
})
Expand Down
112 changes: 112 additions & 0 deletions selfservice/flow/login/hook_external_id_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// Copyright © 2026 Ory Corp
// SPDX-License-Identifier: Apache-2.0

package login_test

import (
"context"
"net/http"
"net/http/httptest"
"net/url"
"testing"
"time"

"github.com/gofrs/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/ory/kratos/driver/config"
"github.com/ory/kratos/hydra"
"github.com/ory/kratos/identity"
"github.com/ory/kratos/pkg"
"github.com/ory/kratos/pkg/testhelpers"
"github.com/ory/kratos/selfservice/flow"
"github.com/ory/kratos/selfservice/flow/login"
"github.com/ory/kratos/session"
"github.com/ory/x/sqlxx"
)

func TestLoginExecutorWithExternalID(t *testing.T) {
ctx := context.Background()
conf, reg := pkg.NewFastRegistryWithMocks(t)
fakeHydra := hydra.NewFake()
reg.SetHydra(fakeHydra)

testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/login.schema.json")
conf.MustSet(ctx, config.ViperKeySelfServiceBrowserDefaultReturnTo, "https://www.ory.sh/kratos/return_to")
conf.MustSet(ctx, config.ViperKeyOAuth2ProviderURL, "https://hydra.example.com")

i := &identity.Identity{
ID: uuid.Must(uuid.NewV4()),
ExternalID: sqlxx.NullString("external-id"),
SchemaID: config.DefaultIdentityTraitsSchemaID,
State: identity.StateActive,
}
require.NoError(t, reg.Persister().CreateIdentity(ctx, i))

t.Run("case=subject_source=id", func(t *testing.T) {
conf.MustSet(ctx, config.ViperKeyOAuth2ProviderSubjectSource, "id")
fakeHydra.SubjectSource = "id"
loginFlow, err := login.NewFlow(conf, time.Minute, hydra.FakeValidLoginChallenge, &http.Request{URL: &url.URL{Path: "/", RawQuery: "login_challenge=" + hydra.FakeValidLoginChallenge}}, flow.TypeBrowser)
require.NoError(t, err)
loginFlow.OAuth2LoginChallenge = hydra.FakeValidLoginChallenge

w := httptest.NewRecorder()
r := &http.Request{URL: &url.URL{Path: "/login/post"}}
sess := session.NewInactiveSession()
sess.CompletedLoginFor(identity.CredentialsTypePassword, identity.AuthenticatorAssuranceLevel1)

err = reg.LoginHookExecutor().PostLoginHook(w, r, identity.CredentialsTypePassword.ToUiNodeGroup(), loginFlow, i, sess, "")
require.NoError(t, err)

require.Len(t, fakeHydra.Params(), 1)
assert.Equal(t, i.ID.String(), fakeHydra.Params()[0].IdentityID)
assert.Equal(t, "external-id", fakeHydra.Params()[0].ExternalID)
})

t.Run("case=subject_source=external_id", func(t *testing.T) {
conf.MustSet(ctx, config.ViperKeyOAuth2ProviderSubjectSource, "external_id")
fakeHydra.SubjectSource = "external_id"
loginFlow, err := login.NewFlow(conf, time.Minute, hydra.FakeValidLoginChallenge, &http.Request{URL: &url.URL{Path: "/", RawQuery: "login_challenge=" + hydra.FakeValidLoginChallenge}}, flow.TypeBrowser)
require.NoError(t, err)
loginFlow.OAuth2LoginChallenge = hydra.FakeValidLoginChallenge

w := httptest.NewRecorder()
r := &http.Request{URL: &url.URL{Path: "/login/post"}}
sess := session.NewInactiveSession()
sess.CompletedLoginFor(identity.CredentialsTypePassword, identity.AuthenticatorAssuranceLevel1)

err = reg.LoginHookExecutor().PostLoginHook(w, r, identity.CredentialsTypePassword.ToUiNodeGroup(), loginFlow, i, sess, "")
require.NoError(t, err)

params := fakeHydra.Params()
require.NotEmpty(t, params)
lastParams := params[len(params)-1]
assert.Equal(t, i.ID.String(), lastParams.IdentityID)
assert.Equal(t, "external-id", lastParams.ExternalID)
})

t.Run("case=subject_source=external_id without external_id set", func(t *testing.T) {
iWithoutExtID := &identity.Identity{
ID: uuid.Must(uuid.NewV4()),
SchemaID: config.DefaultIdentityTraitsSchemaID,
State: identity.StateActive,
}
require.NoError(t, reg.Persister().CreateIdentity(ctx, iWithoutExtID))

conf.MustSet(ctx, config.ViperKeyOAuth2ProviderSubjectSource, "external_id")
fakeHydra.SubjectSource = "external_id"
loginFlow, err := login.NewFlow(conf, time.Minute, hydra.FakeValidLoginChallenge, &http.Request{URL: &url.URL{Path: "/", RawQuery: "login_challenge=" + hydra.FakeValidLoginChallenge}}, flow.TypeBrowser)
require.NoError(t, err)
loginFlow.OAuth2LoginChallenge = hydra.FakeValidLoginChallenge

w := httptest.NewRecorder()
r := &http.Request{URL: &url.URL{Path: "/login/post"}}
sess := session.NewInactiveSession()
sess.CompletedLoginFor(identity.CredentialsTypePassword, identity.AuthenticatorAssuranceLevel1)

err = reg.LoginHookExecutor().PostLoginHook(w, r, identity.CredentialsTypePassword.ToUiNodeGroup(), loginFlow, iWithoutExtID, sess, "")
require.Error(t, err)
assert.Contains(t, err.Error(), "external ID set")
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Loading