Skip to content
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.

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

fix: accept login_challenge in SPA verification flows #4284

Merged
merged 2 commits into from
Jan 31, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
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
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 @@
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 @@
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

Check warning on line 504 in selfservice/flow/verification/handler.go

View check run for this annotation

Codecov / codecov/patch

selfservice/flow/verification/handler.go#L503-L504

Added lines #L503 - L504 were not covered by tests
}

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
Loading