Skip to content

Commit

Permalink
Pass OIDC claims into post-login flow to include in web hook context
Browse files Browse the repository at this point in the history
The login flow doesn't trigger a refresh of the identity when the OIDC
claims have changed. By passing the claims through to the web hook
context, this means that an external handler can be configured to
update the identity as appropriate, when there are changes.
  • Loading branch information
fenech committed Jun 4, 2024
1 parent 3c06689 commit 90747f5
Show file tree
Hide file tree
Showing 49 changed files with 290 additions and 210 deletions.
2 changes: 1 addition & 1 deletion selfservice/flow/login/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,7 @@ continueLogin:
return
}

if err := h.d.LoginHookExecutor().PostLoginHook(w, r, group, f, i, sess, ""); err != nil {
if err := h.d.LoginHookExecutor().PostLoginHook(w, r, group, f, i, sess, nil, ""); err != nil {
if errors.Is(err, ErrAddressNotVerified) {
h.d.LoginFlowErrorHandler().WriteFlowError(w, r, f, node.DefaultGroup, errors.WithStack(schema.NewAddressNotVerifiedError()))
return
Expand Down
16 changes: 9 additions & 7 deletions selfservice/flow/login/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/ory/kratos/schema"
"github.com/ory/kratos/selfservice/flow"
"github.com/ory/kratos/selfservice/sessiontokenexchange"
"github.com/ory/kratos/selfservice/strategy/oidc/claims"
"github.com/ory/kratos/session"
"github.com/ory/kratos/ui/container"
"github.com/ory/kratos/ui/node"
Expand All @@ -34,7 +35,7 @@ type (
}

PostHookExecutor interface {
ExecuteLoginPostHook(w http.ResponseWriter, r *http.Request, g node.UiNodeGroup, a *Flow, s *session.Session) error
ExecuteLoginPostHook(w http.ResponseWriter, r *http.Request, g node.UiNodeGroup, a *Flow, s *session.Session, c *claims.Claims) error
}

HooksProvider interface {
Expand Down Expand Up @@ -125,6 +126,7 @@ func (e *HookExecutor) PostLoginHook(
f *Flow,
i *identity.Identity,
s *session.Session,
c *claims.Claims,
provider string,
) (err error) {
ctx := r.Context()
Expand All @@ -140,15 +142,15 @@ func (e *HookExecutor) PostLoginHook(
return err
}

c := e.d.Config()
cfg := e.d.Config()
// Verify the redirect URL before we do any other processing.
returnTo, err := x.SecureRedirectTo(r,
c.SelfServiceBrowserDefaultReturnTo(r.Context()),
cfg.SelfServiceBrowserDefaultReturnTo(r.Context()),
x.SecureRedirectReturnTo(f.ReturnTo),
x.SecureRedirectUseSourceURL(f.RequestURL),
x.SecureRedirectAllowURLs(c.SelfServiceBrowserAllowedReturnToDomains(r.Context())),
x.SecureRedirectAllowSelfServiceURLs(c.SelfPublicURL(r.Context())),
x.SecureRedirectOverrideDefaultReturnTo(c.SelfServiceFlowLoginReturnTo(r.Context(), f.Active.String())),
x.SecureRedirectAllowURLs(cfg.SelfServiceBrowserAllowedReturnToDomains(r.Context())),
x.SecureRedirectAllowSelfServiceURLs(cfg.SelfPublicURL(r.Context())),
x.SecureRedirectOverrideDefaultReturnTo(cfg.SelfServiceFlowLoginReturnTo(r.Context(), f.Active.String())),
)
if err != nil {
return err
Expand All @@ -168,7 +170,7 @@ func (e *HookExecutor) PostLoginHook(
WithField("flow_method", f.Active).
Debug("Running ExecuteLoginPostHook.")
for k, executor := range e.d.PostLoginHooks(r.Context(), f.Active) {
if err := executor.ExecuteLoginPostHook(w, r, g, f, s); err != nil {
if err := executor.ExecuteLoginPostHook(w, r, g, f, s, c); err != nil {
if errors.Is(err, ErrHookAbortFlow) {
e.d.Logger().
WithRequest(r).
Expand Down
2 changes: 1 addition & 1 deletion selfservice/flow/login/hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestLoginExecutor(t *testing.T) {
}

testhelpers.SelfServiceHookLoginErrorHandler(t, w, r,
reg.LoginHookExecutor().PostLoginHook(w, r, strategy.ToUiNodeGroup(), loginFlow, useIdentity, sess, ""))
reg.LoginHookExecutor().PostLoginHook(w, r, strategy.ToUiNodeGroup(), loginFlow, useIdentity, sess, nil, ""))
})

ts := httptest.NewServer(router)
Expand Down
3 changes: 2 additions & 1 deletion selfservice/hook/address_verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/ory/kratos/identity"
"github.com/ory/kratos/selfservice/flow/login"
"github.com/ory/kratos/selfservice/strategy/oidc/claims"
"github.com/ory/kratos/session"
)

Expand All @@ -25,7 +26,7 @@ func NewAddressVerifier() *AddressVerifier {
return &AddressVerifier{}
}

func (e *AddressVerifier) ExecuteLoginPostHook(_ http.ResponseWriter, _ *http.Request, _ node.UiNodeGroup, f *login.Flow, s *session.Session) error {
func (e *AddressVerifier) ExecuteLoginPostHook(_ http.ResponseWriter, _ *http.Request, _ node.UiNodeGroup, f *login.Flow, s *session.Session, _ *claims.Claims) error {
// if the login happens using the password method, there must be at least one verified address
if f.Active != identity.CredentialsTypePassword {
return nil
Expand Down
2 changes: 1 addition & 1 deletion selfservice/hook/address_verifier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func TestAddressVerifier(t *testing.T) {
Identity: &identity.Identity{ID: x.NewUUID(), VerifiableAddresses: uc.verifiableAddresses},
}

err := verifier.ExecuteLoginPostHook(nil, nil, node.DefaultGroup, tc.flow, sessions)
err := verifier.ExecuteLoginPostHook(nil, nil, node.DefaultGroup, tc.flow, sessions, nil)

if tc.neverError || uc.expectedError == nil {
assert.NoError(t, err)
Expand Down
3 changes: 2 additions & 1 deletion selfservice/hook/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/ory/kratos/selfservice/flow/recovery"
"github.com/ory/kratos/selfservice/flow/verification"
"github.com/ory/kratos/selfservice/strategy/oidc/claims"
"github.com/ory/kratos/ui/node"

"github.com/ory/kratos/identity"
Expand Down Expand Up @@ -64,7 +65,7 @@ func (e Error) ExecuteSettingsPostPersistHook(w http.ResponseWriter, r *http.Req
return e.err("ExecuteSettingsPostPersistHook", settings.ErrHookAbortFlow)
}

func (e Error) ExecuteLoginPostHook(w http.ResponseWriter, r *http.Request, g node.UiNodeGroup, a *login.Flow, s *session.Session) error {
func (e Error) ExecuteLoginPostHook(w http.ResponseWriter, r *http.Request, g node.UiNodeGroup, a *login.Flow, s *session.Session, c *claims.Claims) error {
return e.err("ExecuteLoginPostHook", login.ErrHookAbortFlow)
}

Expand Down
11 changes: 7 additions & 4 deletions selfservice/hook/session_destroyer.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,17 @@ import (
"github.com/ory/kratos/selfservice/flow/login"
"github.com/ory/kratos/selfservice/flow/recovery"
"github.com/ory/kratos/selfservice/flow/settings"
"github.com/ory/kratos/selfservice/strategy/oidc/claims"
"github.com/ory/kratos/session"
"github.com/ory/kratos/ui/node"
"github.com/ory/x/otelx"
)

var _ login.PostHookExecutor = new(SessionDestroyer)
var _ recovery.PostHookExecutor = new(SessionDestroyer)
var _ settings.PostHookPostPersistExecutor = new(SessionDestroyer)
var (
_ login.PostHookExecutor = new(SessionDestroyer)
_ recovery.PostHookExecutor = new(SessionDestroyer)
_ settings.PostHookPostPersistExecutor = new(SessionDestroyer)
)

type (
sessionDestroyerDependencies interface {
Expand All @@ -34,7 +37,7 @@ func NewSessionDestroyer(r sessionDestroyerDependencies) *SessionDestroyer {
return &SessionDestroyer{r: r}
}

func (e *SessionDestroyer) ExecuteLoginPostHook(_ http.ResponseWriter, r *http.Request, _ node.UiNodeGroup, _ *login.Flow, s *session.Session) error {
func (e *SessionDestroyer) ExecuteLoginPostHook(_ http.ResponseWriter, r *http.Request, _ node.UiNodeGroup, _ *login.Flow, s *session.Session, _ *claims.Claims) error {
return otelx.WithSpan(r.Context(), "selfservice.hook.SessionDestroyer.ExecuteLoginPostHook", func(ctx context.Context) error {
if _, err := e.r.SessionPersister().RevokeSessionsIdentityExcept(ctx, s.Identity.ID, s.ID); err != nil {
return err
Expand Down
1 change: 1 addition & 0 deletions selfservice/hook/session_destroyer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ func TestSessionDestroyer(t *testing.T) {
node.DefaultGroup,
nil,
&session.Session{Identity: i},
nil,
)
},
},
Expand Down
3 changes: 2 additions & 1 deletion selfservice/hook/show_verification_ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/ory/kratos/selfservice/flow"
"github.com/ory/kratos/selfservice/flow/login"
"github.com/ory/kratos/selfservice/flow/registration"
"github.com/ory/kratos/selfservice/strategy/oidc/claims"
"github.com/ory/kratos/session"
"github.com/ory/kratos/ui/node"
"github.com/ory/kratos/x"
Expand Down Expand Up @@ -52,7 +53,7 @@ func (e *ShowVerificationUIHook) ExecutePostRegistrationPostPersistHook(_ http.R

// ExecuteLoginPostHook adds redirect headers and status code if the request is a browser request.
// If the request is not a browser request, this hook does nothing.
func (e *ShowVerificationUIHook) ExecuteLoginPostHook(_ http.ResponseWriter, r *http.Request, _ node.UiNodeGroup, f *login.Flow, _ *session.Session) error {
func (e *ShowVerificationUIHook) ExecuteLoginPostHook(_ http.ResponseWriter, r *http.Request, _ node.UiNodeGroup, f *login.Flow, _ *session.Session, _ *claims.Claims) error {
return otelx.WithSpan(r.Context(), "selfservice.hook.ShowVerificationUIHook.ExecutePostRegistrationPostPersistHook", func(ctx context.Context) error {
return e.execute(r.WithContext(ctx), f)
})
Expand Down
8 changes: 4 additions & 4 deletions selfservice/hook/show_verification_ui_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func TestExecutePostRegistrationPostPersistHook(t *testing.T) {
browserRequest := httptest.NewRequest("GET", "/", nil)
f := &login.Flow{}
rec := httptest.NewRecorder()
require.NoError(t, h.ExecuteLoginPostHook(rec, browserRequest, "", f, nil))
require.NoError(t, h.ExecuteLoginPostHook(rec, browserRequest, "", f, nil, nil))
require.Equal(t, 200, rec.Code)
})

Expand All @@ -95,7 +95,7 @@ func TestExecutePostRegistrationPostPersistHook(t *testing.T) {
browserRequest.Header.Add("Accept", "application/json")
f := &login.Flow{}
rec := httptest.NewRecorder()
require.NoError(t, h.ExecuteLoginPostHook(rec, browserRequest, "", f, nil))
require.NoError(t, h.ExecuteLoginPostHook(rec, browserRequest, "", f, nil, nil))
require.Equal(t, 200, rec.Code)
})

Expand All @@ -112,7 +112,7 @@ func TestExecutePostRegistrationPostPersistHook(t *testing.T) {
flow.NewContinueWithVerificationUI(vf, "[email protected]", ""),
}
rec := httptest.NewRecorder()
require.NoError(t, h.ExecuteLoginPostHook(rec, browserRequest, "", rf, nil))
require.NoError(t, h.ExecuteLoginPostHook(rec, browserRequest, "", rf, nil, nil))
assert.Equal(t, 200, rec.Code)
assert.Equal(t, "/verification?flow="+vf.ID.String(), rf.ReturnToVerification)
})
Expand All @@ -127,7 +127,7 @@ func TestExecutePostRegistrationPostPersistHook(t *testing.T) {
flow.NewContinueWithSetToken("token"),
}
rec := httptest.NewRecorder()
require.NoError(t, h.ExecuteLoginPostHook(rec, browserRequest, "", rf, nil))
require.NoError(t, h.ExecuteLoginPostHook(rec, browserRequest, "", rf, nil, nil))
assert.Equal(t, 200, rec.Code)
})
})
Expand Down
10 changes: 7 additions & 3 deletions selfservice/hook/stub/test_body.jsonnet
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
function(ctx) std.prune({
flow_id: ctx.flow.id,
identity_id: if std.objectHas(ctx, "identity") then ctx.identity.id,
session_id: if std.objectHas(ctx, "session") then ctx.session.id,
identity_id: if std.objectHas(ctx, 'identity') then ctx.identity.id,
session_id: if std.objectHas(ctx, 'session') then ctx.session.id,
headers: ctx.request_headers,
url: ctx.request_url,
method: ctx.request_method,
cookies: ctx.request_cookies,
transient_payload: if std.objectHas(ctx.flow, "transient_payload") then ctx.flow.transient_payload,
transient_payload: if std.objectHas(ctx.flow, 'transient_payload') then ctx.flow.transient_payload,
nickname: if std.objectHas(ctx, 'claims') then ctx.claims.nickname,
groups: if std.objectHas(ctx, 'claims') &&
std.objectHas(ctx.claims, 'raw_claims') &&
std.objectHas(ctx.claims.raw_claims, 'groups') then ctx.claims.raw_claims.groups,
})
3 changes: 2 additions & 1 deletion selfservice/hook/verification.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/ory/kratos/selfservice/flow/registration"
"github.com/ory/kratos/selfservice/flow/settings"
"github.com/ory/kratos/selfservice/flow/verification"
"github.com/ory/kratos/selfservice/strategy/oidc/claims"
"github.com/ory/kratos/session"
"github.com/ory/kratos/ui/node"
"github.com/ory/kratos/x"
Expand Down Expand Up @@ -65,7 +66,7 @@ func (e *Verifier) ExecuteSettingsPostPersistHook(w http.ResponseWriter, r *http
})
}

func (e *Verifier) ExecuteLoginPostHook(w http.ResponseWriter, r *http.Request, g node.UiNodeGroup, f *login.Flow, s *session.Session) (err error) {
func (e *Verifier) ExecuteLoginPostHook(w http.ResponseWriter, r *http.Request, g node.UiNodeGroup, f *login.Flow, s *session.Session, c *claims.Claims) (err error) {
ctx, span := e.r.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.hook.Verifier.ExecuteLoginPostHook")
r = r.WithContext(ctx)
defer otelx.End(span, &err)
Expand Down
4 changes: 2 additions & 2 deletions selfservice/hook/verification_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TestVerifier(t *testing.T) {
name: "login",
execHook: func(h *hook.Verifier, i *identity.Identity, f flow.Flow) error {
return h.ExecuteLoginPostHook(
httptest.NewRecorder(), u, node.CodeGroup, f.(*login.Flow), &session.Session{ID: x.NewUUID(), Identity: i})
httptest.NewRecorder(), u, node.CodeGroup, f.(*login.Flow), &session.Session{ID: x.NewUUID(), Identity: i}, nil)
},
originalFlow: func() flow.FlowWithContinueWith {
return &login.Flow{RequestURL: "http://foo.com/login", RequestedAAL: "aal1"}
Expand Down Expand Up @@ -126,7 +126,7 @@ func TestVerifier(t *testing.T) {
h := hook.NewVerifier(reg)
i := identity.NewIdentity(config.DefaultIdentityTraitsSchemaID)
f := &login.Flow{RequestedAAL: "aal2"}
require.NoError(t, h.ExecuteLoginPostHook(httptest.NewRecorder(), u, node.CodeGroup, f, &session.Session{ID: x.NewUUID(), Identity: i}))
require.NoError(t, h.ExecuteLoginPostHook(httptest.NewRecorder(), u, node.CodeGroup, f, &session.Session{ID: x.NewUUID(), Identity: i}, nil))

messages, err := reg.CourierPersister().NextMessages(context.Background(), 12)
require.EqualError(t, err, "queue is empty")
Expand Down
5 changes: 4 additions & 1 deletion selfservice/hook/web_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/ory/kratos/selfservice/flow/registration"
"github.com/ory/kratos/selfservice/flow/settings"
"github.com/ory/kratos/selfservice/flow/verification"
"github.com/ory/kratos/selfservice/strategy/oidc/claims"
"github.com/ory/kratos/session"
"github.com/ory/kratos/text"
"github.com/ory/kratos/ui/node"
Expand Down Expand Up @@ -82,6 +83,7 @@ type (
RequestCookies map[string]string `json:"request_cookies"`
Identity *identity.Identity `json:"identity,omitempty"`
Session *session.Session `json:"session,omitempty"`
Claims *claims.Claims `json:"claims,omitempty"`
}

WebHook struct {
Expand Down Expand Up @@ -132,7 +134,7 @@ func (e *WebHook) ExecuteLoginPreHook(_ http.ResponseWriter, req *http.Request,
})
}

func (e *WebHook) ExecuteLoginPostHook(_ http.ResponseWriter, req *http.Request, _ node.UiNodeGroup, flow *login.Flow, session *session.Session) error {
func (e *WebHook) ExecuteLoginPostHook(_ http.ResponseWriter, req *http.Request, _ node.UiNodeGroup, flow *login.Flow, session *session.Session, claims *claims.Claims) error {
return otelx.WithSpan(req.Context(), "selfservice.hook.WebHook.ExecuteLoginPostHook", func(ctx context.Context) error {
return e.execute(ctx, &templateContext{
Flow: flow,
Expand All @@ -142,6 +144,7 @@ func (e *WebHook) ExecuteLoginPostHook(_ http.ResponseWriter, req *http.Request,
RequestCookies: cookies(req),
Identity: session.Identity,
Session: session,
Claims: claims,
})
})
}
Expand Down
Loading

0 comments on commit 90747f5

Please sign in to comment.