Skip to content

Commit

Permalink
fix: issue session after verification after registration with OIDC SSO (
Browse files Browse the repository at this point in the history
  • Loading branch information
hperl authored Aug 31, 2023
1 parent bafc47d commit a28b523
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 22 deletions.
4 changes: 3 additions & 1 deletion selfservice/flow/registration/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,9 @@ func (e *HookExecutor) PostRegistrationHook(w http.ResponseWriter, r *http.Reque
return err
}

if err != nil {
// We persist the session here so that subsequent hooks (like verification) can use it.
s.AuthenticatedAt = time.Now().UTC()
if err := e.d.SessionPersister().UpsertSession(r.Context(), s); err != nil {
return err
}

Expand Down
11 changes: 11 additions & 0 deletions selfservice/flow/verification/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,17 @@ func (h *Handler) updateVerificationFlow(w http.ResponseWriter, r *http.Request,
return
}

sess, err := h.d.SessionPersister().GetSession(ctx, f.SessionID.UUID, session.ExpandDefault)
if err != nil {
h.d.VerificationFlowErrorHandler().WriteFlowError(w, r, f, node.DefaultGroup, err)
return
}
err = h.d.SessionManager().IssueCookie(ctx, w, r, sess)
if err != nil {
h.d.VerificationFlowErrorHandler().WriteFlowError(w, r, f, node.DefaultGroup, err)
return
}

http.Redirect(w, r, callbackURL, http.StatusSeeOther)
return
}
Expand Down
10 changes: 5 additions & 5 deletions selfservice/flow/verification/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,10 @@ import (

"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/verification"
"github.com/ory/kratos/session"
"github.com/ory/kratos/x"
)

Expand Down Expand Up @@ -218,16 +216,18 @@ func TestPostFlow(t *testing.T) {

t.Run("suite=with OIDC login challenge", func(t *testing.T) {
t.Run("case=succeeds with a session", func(t *testing.T) {
s := testhelpers.CreateSession(t, reg)

f := &verification.Flow{
ID: uuid.Must(uuid.NewV4()),
Type: "browser",
ExpiresAt: time.Now().Add(1 * time.Hour),
IssuedAt: time.Now(),
OAuth2LoginChallenge: hydra.FakeValidLoginChallenge,
OAuth2LoginChallengeParams: verification.OAuth2LoginChallengeParams{
SessionID: uuid.NullUUID{UUID: uuid.Must(uuid.NewV4()), Valid: true},
IdentityID: uuid.NullUUID{UUID: uuid.Must(uuid.NewV4()), Valid: true},
AMR: session.AuthenticationMethods{{Method: identity.CredentialsTypePassword}},
SessionID: uuid.NullUUID{UUID: s.ID, Valid: true},
IdentityID: uuid.NullUUID{UUID: s.IdentityID, Valid: true},
AMR: s.AMR,
},
State: flow.StatePassedChallenge,
}
Expand Down
6 changes: 0 additions & 6 deletions selfservice/hook/session_issuer.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package hook
import (
"context"
"net/http"
"time"

"github.com/ory/kratos/identity"
"github.com/ory/kratos/ui/node"
Expand Down Expand Up @@ -53,11 +52,6 @@ func (e *SessionIssuer) ExecutePostRegistrationPostPersistHook(w http.ResponseWr
}

func (e *SessionIssuer) executePostRegistrationPostPersistHook(w http.ResponseWriter, r *http.Request, a *registration.Flow, s *session.Session) error {
s.AuthenticatedAt = time.Now().UTC()
if err := e.r.SessionPersister().UpsertSession(r.Context(), s); err != nil {
return err
}

if a.Type == flow.TypeAPI {
if s.AuthenticatedVia(identity.CredentialsTypeOIDC) {
if handled, err := e.r.SessionManager().MaybeRedirectAPICodeFlow(w, r, a, s.ID, node.OpenIDConnectGroup); err != nil {
Expand Down
31 changes: 21 additions & 10 deletions selfservice/hook/session_issuer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,17 @@ func TestSessionIssuer(t *testing.T) {
t.Run("method=sign-up", func(t *testing.T) {
t.Run("flow=browser", func(t *testing.T) {
w := httptest.NewRecorder()
sid := x.NewUUID()

i := identity.NewIdentity(config.DefaultIdentityTraitsSchemaID)
require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), i))

s := testhelpers.CreateSession(t, reg)
f := &registration.Flow{Type: flow.TypeBrowser}

require.NoError(t, h.ExecutePostRegistrationPostPersistHook(w, &r,
f, &session.Session{ID: sid, Identity: i, Token: randx.MustString(12, randx.AlphaLowerNum)}))
f, &session.Session{ID: s.ID, Identity: s.Identity, Token: randx.MustString(12, randx.AlphaLowerNum)}))

require.Empty(t, f.ContinueWithItems)

got, err := reg.SessionPersister().GetSession(context.Background(), sid, session.ExpandNothing)
got, err := reg.SessionPersister().GetSession(context.Background(), s.ID, session.ExpandNothing)
require.NoError(t, err)
assert.Equal(t, sid, got.ID)
assert.Equal(t, s.ID, got.ID)
assert.True(t, got.AuthenticatedAt.After(time.Now().Add(-time.Minute)))

assert.Contains(t, w.Header().Get("Set-Cookie"), config.DefaultSessionCookieName)
Expand All @@ -63,10 +59,17 @@ func TestSessionIssuer(t *testing.T) {
w := httptest.NewRecorder()

i := identity.NewIdentity(config.DefaultIdentityTraitsSchemaID)
s := &session.Session{ID: x.NewUUID(), Identity: i, Token: randx.MustString(12, randx.AlphaLowerNum), LogoutToken: randx.MustString(12, randx.AlphaLowerNum)}
s := &session.Session{
ID: x.NewUUID(),
Identity: i,
Token: randx.MustString(12, randx.AlphaLowerNum),
LogoutToken: randx.MustString(12, randx.AlphaLowerNum),
AuthenticatedAt: time.Now().UTC(),
}
f := &registration.Flow{Type: flow.TypeAPI}

require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), i))
require.NoError(t, reg.SessionPersister().UpsertSession(ctx, s))

err := h.ExecutePostRegistrationPostPersistHook(w, &http.Request{Header: http.Header{"Accept": {"application/json"}}}, f, s)
require.ErrorIs(t, err, registration.ErrHookAbortFlow, "%+v", err)
Expand All @@ -92,10 +95,18 @@ func TestSessionIssuer(t *testing.T) {
w := httptest.NewRecorder()

i := identity.NewIdentity(config.DefaultIdentityTraitsSchemaID)
s := &session.Session{ID: x.NewUUID(), Identity: i, Token: randx.MustString(12, randx.AlphaLowerNum), LogoutToken: randx.MustString(12, randx.AlphaLowerNum)}
s := &session.Session{
ID: x.NewUUID(),
Identity: i,
Token: randx.MustString(12, randx.AlphaLowerNum),
LogoutToken: randx.MustString(12, randx.AlphaLowerNum),
AuthenticatedAt: time.Now().UTC(),
}
f := &registration.Flow{Type: flow.TypeBrowser}

require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), i))
require.NoError(t, reg.SessionPersister().UpsertSession(ctx, s))

err := h.ExecutePostRegistrationPostPersistHook(w, &http.Request{Header: http.Header{"Accept": {"application/json"}}}, f, s)
require.ErrorIs(t, err, registration.ErrHookAbortFlow, "%+v", err)
require.Empty(t, f.ContinueWithItems)
Expand Down

0 comments on commit a28b523

Please sign in to comment.