Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ const (
ViperKeyOAuth2ProviderURL = "oauth2_provider.url"
ViperKeyOAuth2ProviderHeader = "oauth2_provider.headers"
ViperKeyOAuth2ProviderOverrideReturnTo = "oauth2_provider.override_return_to"
ViperKeyOAuth2ProviderUseExternalID = "oauth2_provider.use_external_id"
ViperKeyClientHTTPNoPrivateIPRanges = "clients.http.disallow_private_ip_ranges"
ViperKeyClientHTTPPrivateIPExceptionURLs = "clients.http.private_ip_exception_urls"
ViperKeyWebhookHeaderAllowlist = "clients.web_hook.header_allowlist"
Expand Down Expand Up @@ -962,6 +963,10 @@ func (p *Config) OAuth2ProviderOverrideReturnTo(ctx context.Context) bool {
return p.GetProvider(ctx).Bool(ViperKeyOAuth2ProviderOverrideReturnTo)
}

func (p *Config) OAuth2ProviderUseExternalID(ctx context.Context) bool {
return p.GetProvider(ctx).Bool(ViperKeyOAuth2ProviderUseExternalID)
}

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
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.True(t, conf.OAuth2ProviderUseExternalID(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.False(t, conf.OAuth2ProviderUseExternalID(ctx))
})
}

Expand Down
1 change: 1 addition & 0 deletions driver/config/stub/.kratos.oauth2_provider.yaml
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
use_external_id: true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We have the same pattern for the tokenizer, but there the setting is

subject_source: external_id # or id (default)

Could you adjust the code here?

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.

Hi there! Thx for your review and apologies for the delay in getting back to this—I’ve been a bit tied up lately. I changed it like you requested

6 changes: 6 additions & 0 deletions embedx/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -2285,6 +2285,12 @@
"type": "boolean",
"default": false,
"description": "Override the return_to query parameter with the OAuth2 provider request URL when perfoming an OAuth2 login flow."
},
"use_external_id": {
"title": "Use external_id as subject",
"type": "boolean",
"default": false,
"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."
}
},
"additionalProperties": false
Expand Down
6 changes: 6 additions & 0 deletions hydra/fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ var ErrFakeAcceptLoginRequestFailed = errors.New("failed to accept login request
type FakeHydra struct {
Skip bool
RequestURL string
params []AcceptLoginRequestParams
}

func (h *FakeHydra) Params() []AcceptLoginRequestParams {
return h.params
}

var _ Hydra = &FakeHydra{}
Expand All @@ -33,6 +38,7 @@ 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")
}
Expand Down
8 changes: 7 additions & 1 deletion hydra/hydra.go
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,12 @@ 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)
subject := params.IdentityID

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This definition of subject is never used.
if h.d.Config().OAuth2ProviderUseExternalID(ctx) && params.ExternalID != "" {
subject = params.ExternalID
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In the tokenizer we return an error if the identity's external_id is unset, and the subject_source is set to external_id.

I think that would be appropriate here, too, as it's otherwise difficult to figure out which ID was used.

Alternatively, we could probably add another claim, that describes the subject_source to the token, WDYT?

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.

That makes total sense. I agree that consistency with the tokenizer is the best path forward here to avoid confusion over which ID is being used.

I’ve updated the code to return an error if external_id is unset while subject_source is set to external_id. This keeps the behavior predictable across the codebase. Thanks for pointing that out


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
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
88 changes: 88 additions & 0 deletions selfservice/flow/login/hook_external_id_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// 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/internal"
"github.com/ory/kratos/internal/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 := internal.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=use_external_id=false", func(t *testing.T) {
conf.MustSet(ctx, config.ViperKeyOAuth2ProviderUseExternalID, false)
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=use_external_id=true", func(t *testing.T) {
conf.MustSet(ctx, config.ViperKeyOAuth2ProviderUseExternalID, true)
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)

fakeHydra.Params()

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)
})
}
Loading