Skip to content

Commit

Permalink
fix: accept login_challenge in SPA verification flows (#4284)
Browse files Browse the repository at this point in the history
  • Loading branch information
jonas-jonas authored Jan 31, 2025
1 parent 2830e74 commit 7ca3b6b
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 9 deletions.
34 changes: 29 additions & 5 deletions selfservice/flow/verification/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,9 @@ func (h *Handler) updateVerificationFlow(w http.ResponseWriter, r *http.Request,
return
}

if x.IsBrowserRequest(r) {
// API flows can receive requests from the browser, if the link strategy is used.
// However, x.IsBrowserRequest only checks for form submissions, not JSON requests made from a browser context
if x.IsBrowserRequest(r) || (f.Type == flow.TypeBrowser && x.IsJSONRequest(r)) {
// Special case: If we ended up here through a OAuth2 login challenge, we need to accept the login request
// and redirect back to the OAuth2 provider.
if flow.HasReachedState(flow.StatePassedChallenge, f.State) && f.OAuth2LoginChallenge.String() != "" {
Expand Down Expand Up @@ -489,12 +491,34 @@ func (h *Handler) updateVerificationFlow(w http.ResponseWriter, r *http.Request,
return
}

http.Redirect(w, r, callbackURL, http.StatusSeeOther)
if x.IsJSONRequest(r) {
// This intentionally works differently than the "form browser" flow,
// as it _does_ show the `verification success` UI, but the "Continue"
// button contains the link to the OAuth2 provider with the `login_verifier`.
continueNode := f.UI.Nodes.Find("continue")
if continueNode != nil {
if attr, ok := continueNode.Attributes.(*node.AnchorAttributes); ok {
attr.HREF = callbackURL
if err := h.d.VerificationFlowPersister().UpdateVerificationFlow(ctx, f); err != nil {
h.d.VerificationFlowErrorHandler().WriteFlowError(w, r, f, node.DefaultGroup, err)
return
}

h.d.Writer().Write(w, r, f)
return
}
}

// The flow does not have the `continue` node, which is an unknown state.
// This should never happen.
} else {
http.Redirect(w, r, callbackURL, http.StatusSeeOther)
return
}
} else if x.IsBrowserRequest(r) {
http.Redirect(w, r, f.AppendTo(h.d.Config().SelfServiceFlowVerificationUI(ctx)).String(), http.StatusSeeOther)
return
}

http.Redirect(w, r, f.AppendTo(h.d.Config().SelfServiceFlowVerificationUI(ctx)).String(), http.StatusSeeOther)
return
}

updatedFlow, err := h.d.VerificationFlowPersister().GetVerificationFlow(ctx, f.ID)
Expand Down
54 changes: 50 additions & 4 deletions selfservice/flow/verification/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"io"
"net/http"
"net/url"
"strings"
"testing"
"time"

Expand All @@ -24,6 +25,9 @@ import (
"github.com/ory/kratos/internal/testhelpers"
"github.com/ory/kratos/selfservice/flow"
"github.com/ory/kratos/selfservice/flow/verification"
"github.com/ory/kratos/text"
"github.com/ory/kratos/ui/container"
"github.com/ory/kratos/ui/node"
"github.com/ory/kratos/x"
)

Expand Down Expand Up @@ -196,7 +200,7 @@ func TestPostFlow(t *testing.T) {
_ = testhelpers.NewErrorTestServer(t, reg)
_ = testhelpers.NewRedirTS(t, "", conf)

t.Run("case=valid", func(t *testing.T) {
t.Run("client=browser/case=valid", func(t *testing.T) {
f := &verification.Flow{
ID: uuid.Must(uuid.NewV4()),
Type: "browser",
Expand All @@ -206,16 +210,40 @@ func TestPostFlow(t *testing.T) {
}
require.NoError(t, reg.VerificationFlowPersister().CreateVerificationFlow(ctx, f))

client := testhelpers.NewClientWithCookies(t)
client := testhelpers.NewNoRedirectClientWithCookies(t)

u := public.URL + verification.RouteSubmitFlow + "?flow=" + f.ID.String()
resp, err := client.PostForm(u, url.Values{"method": {"fake"}})
require.NoError(t, err)
assert.EqualValues(t, http.StatusSeeOther, resp.StatusCode)
assert.Equal(t, conf.SelfServiceFlowVerificationUI(ctx).String()+"?flow="+f.ID.String(), resp.Header.Get("Location"))
})

t.Run("client=spa/case=valid", func(t *testing.T) {
f := &verification.Flow{
ID: uuid.Must(uuid.NewV4()),
Type: "browser",
ExpiresAt: time.Now().Add(1 * time.Hour),
IssuedAt: time.Now(),
State: flow.StateChooseMethod,
}
require.NoError(t, reg.VerificationFlowPersister().CreateVerificationFlow(ctx, f))

client := testhelpers.NewClientWithCookies(t)

u := public.URL + verification.RouteSubmitFlow + "?flow=" + f.ID.String()
req, err := http.NewRequest("POST", u, strings.NewReader(`{"method": "fake"}`))
require.NoError(t, err)
req.Header.Set("Content-Type", "application/json")
req.Header.Set("Accept", "application/json")
resp, err := client.Do(req)
require.NoError(t, err)
assert.EqualValues(t, http.StatusOK, resp.StatusCode)
})

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

f := &verification.Flow{
Expand All @@ -229,10 +257,17 @@ func TestPostFlow(t *testing.T) {
IdentityID: uuid.NullUUID{UUID: s.IdentityID, Valid: true},
AMR: s.AMR,
},
UI: &container.Container{
Action: "http://action",
Nodes: []*node.Node{node.NewAnchorField("continue", "https://ory.sh", node.CodeGroup, text.NewInfoNodeLabelContinue())},
},
State: flow.StatePassedChallenge,
}
require.NoError(t, reg.VerificationFlowPersister().CreateVerificationFlow(ctx, f))

return f
}
t.Run("client=browser/case=succeeds with a session", func(t *testing.T) {
f := createFlow(t)
client := testhelpers.NewNoRedirectClientWithCookies(t)

u := public.URL + verification.RouteSubmitFlow + "?flow=" + f.ID.String()
Expand All @@ -241,6 +276,17 @@ func TestPostFlow(t *testing.T) {
assert.Equal(t, http.StatusSeeOther, resp.StatusCode)
assert.Equal(t, hydra.FakePostLoginURL, resp.Header.Get("Location"))
})
t.Run("client=spa/case=succeeds with a session", func(t *testing.T) {
f := createFlow(t)
client := testhelpers.NewNoRedirectClientWithCookies(t)

u := public.URL + verification.RouteSubmitFlow + "?flow=" + f.ID.String()
resp, err := client.Post(u, "application/json", strings.NewReader(`{"method": "fake"}`))
require.NoError(t, err)
assert.Equal(t, http.StatusOK, resp.StatusCode)
body := x.MustReadAll(resp.Body)
assert.Equal(t, hydra.FakePostLoginURL, gjson.GetBytes(body, "ui.nodes.#(attributes.id==continue).attributes.href").String(), "%s", body)
})

t.Run("case=fails without a session", func(t *testing.T) {
client := testhelpers.NewClientWithCookies(t)
Expand Down

0 comments on commit 7ca3b6b

Please sign in to comment.