fix: accept login_challenge after verification

persistence/sql/migrations/sql/20230811000000000001_verification_add_oauth2_login_challenge.down.sql

@@ -0,0 +1 @@
+ALTER TABLE selfservice_verification_flows DROP COLUMN oauth2_login_challenge;

persistence/sql/migrations/sql/20230811000000000001_verification_add_oauth2_login_challenge.up.sql

@@ -0,0 +1 @@
+ALTER TABLE selfservice_verification_flows ADD COLUMN oauth2_login_challenge TEXT NULL;

selfservice/flow/flow.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/ory/kratos/driver/config"
 	"github.com/ory/kratos/ui/container"
+	"github.com/ory/x/sqlxx"
 
 	"github.com/ory/herodot"
 	"github.com/ory/kratos/x"
@@ -41,6 +42,10 @@ type Flow interface {
 	GetUI() *container.Container
 }
 
+type Challenger interface {
+	GetOAuth2LoginChallenge() sqlxx.NullString
+}
+
 type FlowWithRedirect interface {
 	SecureRedirectToOpts(ctx context.Context, cfg config.Provider) (opts []x.SecureRedirectOption)
 }

selfservice/flow/registration/flow.go

@@ -229,6 +229,10 @@ func (f *Flow) ContinueWith() []flow.ContinueWith {
 	return f.ContinueWithItems
 }
 
+func (f Flow) GetOAuth2LoginChallenge() sqlxx.NullString {
+	return f.OAuth2LoginChallenge
+}
+
 func (f *Flow) SecureRedirectToOpts(ctx context.Context, cfg config.Provider) (opts []x.SecureRedirectOption) {
 	return []x.SecureRedirectOption{
 		x.SecureRedirectReturnTo(f.ReturnTo),

selfservice/flow/registration/hook.go

@@ -7,7 +7,6 @@ import (
 	"context"
 	"fmt"
 	"net/http"
-	"net/url"
 	"time"
 
 	"go.opentelemetry.io/otel/trace"
@@ -249,22 +248,15 @@ func (e *HookExecutor) PostRegistrationHook(w http.ResponseWriter, r *http.Reque
 
 	finalReturnTo := returnTo.String()
 	if a.OAuth2LoginChallenge != "" {
-		callbackURL, err := e.d.Hydra().AcceptLoginRequest(r.Context(), string(a.OAuth2LoginChallenge), i.ID.String(), s.AMR)
-		if err != nil {
-			return err
-		}
 		if a.ReturnToVerification != "" {
 			// Special case: If Kratos is used as a login UI *and* we want to show the verification UI,
 			// redirect to the verification URL first and then return to Hydra.
-			verificationURL, err := url.Parse(a.ReturnToVerification)
+			finalReturnTo = a.ReturnToVerification
+		} else {
+			callbackURL, err := e.d.Hydra().AcceptLoginRequest(r.Context(), string(a.OAuth2LoginChallenge), i.ID.String(), s.AMR)
 			if err != nil {
 				return err
 			}
-			q := verificationURL.Query()
-			q.Set("return_to", callbackURL)
-			verificationURL.RawQuery = q.Encode()
-			finalReturnTo = verificationURL.String()
-		} else {
 			finalReturnTo = callbackURL
 		}
 	} else if a.ReturnToVerification != "" {

selfservice/flow/registration/hook_test.go

@@ -203,24 +203,6 @@ func TestRegistrationExecutor(t *testing.T) {
 					assert.EqualValues(t, http.StatusOK, res.StatusCode)
 					assert.Contains(t, res.Request.URL.String(), verificationTS.URL)
 					assert.NotEmpty(t, res.Request.URL.Query().Get("flow"))
-					assert.Equal(t, hydra.FakePostLoginURL, res.Request.URL.Query().Get("return_to"))
-				})
-
-				t.Run("case=should not redirect to verification UI if the login_challenge is invalid", func(t *testing.T) {
-					t.Cleanup(testhelpers.SelfServiceHookConfigReset(t, conf))
-					conf.MustSet(ctx, config.ViperKeySelfServiceVerificationEnabled, true)
-					conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationAfter+".hooks", []map[string]interface{}{{
-						"hook": hook.KeyVerificationUI,
-					}})
-					i := testhelpers.SelfServiceHookFakeIdentity(t)
-					i.Traits = identity.Traits(`{"email": "verifiable-invalid-login_challenge@ory.sh"}`)
-
-					withOAuthChallenge := func(f *registration.Flow) {
-						f.OAuth2LoginChallenge = hydra.FakeInvalidLoginChallenge
-					}
-					res, body := makeRequestPost(t, newServer(t, i, flow.TypeBrowser, withOAuthChallenge), false, url.Values{})
-					assert.EqualValues(t, http.StatusInternalServerError, res.StatusCode)
-					assert.Equal(t, hydra.ErrFakeAcceptLoginRequestFailed.Error(), body, "%s", body)
 				})
 
 				t.Run("case=should redirect to first verification UI if show_verification_ui hook is set and multiple verifiable addresses", func(t *testing.T) {

selfservice/flow/verification/flow.go

@@ -79,6 +79,8 @@ type Flow struct {
 	// required: true
 	State State `json:"state" faker:"-" db:"state"`
 
+	OAuth2LoginChallenge sqlxx.NullString `json:"-" db:"oauth2_login_challenge"`
+
 	// CSRFToken contains the anti-csrf token associated with this request.
 	CSRFToken string `json:"-" db:"csrf_token"`
 

selfservice/flow/verification/handler.go

@@ -7,6 +7,8 @@ import (
 	"net/http"
 	"time"
 
+	"github.com/ory/kratos/hydra"
+	"github.com/ory/kratos/session"
 	"github.com/ory/nosurf"
 
 	"github.com/ory/kratos/schema"
@@ -44,6 +46,8 @@ type (
 		identity.ManagementProvider
 		identity.PrivilegedPoolProvider
 		config.Provider
+		hydra.Provider
+		session.ManagementProvider
 
 		x.CSRFTokenGeneratorProvider
 		x.WriterProvider
@@ -186,7 +190,7 @@ type createBrowserVerificationFlow struct {
 //	  200: verificationFlow
 //	  303: emptyResponse
 //	  default: errorGeneric
-func (h *Handler) createBrowserVerificationFlow(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
+func (h *Handler) createBrowserVerificationFlow(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
 	if !h.d.Config().SelfServiceFlowVerificationEnabled(r.Context()) {
 		h.d.SelfServiceErrorManager().Forward(r.Context(), w, r, errors.WithStack(herodot.ErrBadRequest.WithReasonf("Verification is not allowed because it was disabled.")))
 		return
@@ -385,7 +389,7 @@ type updateVerificationFlowBody struct{}
 //	  400: verificationFlow
 //	  410: errorGeneric
 //	  default: errorGeneric
-func (h *Handler) updateVerificationFlow(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
+func (h *Handler) updateVerificationFlow(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
 	rid, err := flow.GetFlowID(r)
 	if err != nil {
 		h.d.VerificationFlowErrorHandler().WriteFlowError(w, r, nil, node.DefaultGroup, err)
@@ -435,6 +439,25 @@ func (h *Handler) updateVerificationFlow(w http.ResponseWriter, r *http.Request,
 	}
 
 	if x.IsBrowserRequest(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 found && f.OAuth2LoginChallenge.String() != "" {
+			s, err := h.d.SessionManager().FetchFromRequest(r.Context(), r)
+			if err != nil {
+				h.d.VerificationFlowErrorHandler().WriteFlowError(w, r, f, node.DefaultGroup, err)
+				return
+			}
+
+			callbackURL, err := h.d.Hydra().AcceptLoginRequest(r.Context(), string(f.OAuth2LoginChallenge), s.IdentityID.String(), s.AMR)
+			if err != nil {
+				h.d.VerificationFlowErrorHandler().WriteFlowError(w, r, f, node.DefaultGroup, err)
+				return
+			}
+
+			http.Redirect(w, r, callbackURL, http.StatusSeeOther)
+			return
+		}
+
 		http.Redirect(w, r, f.AppendTo(h.d.Config().SelfServiceFlowVerificationUI(r.Context())).String(), http.StatusSeeOther)
 		return
 	}

selfservice/hook/show_verification_ui.go

@@ -58,6 +58,7 @@ func (e *ShowVerificationUIHook) execute(r *http.Request, f *registration.Flow)
 	for _, c := range f.ContinueWithItems {
 		if item, ok := c.(*flow.ContinueWithVerificationUI); ok {
 			vf = item
+			break
 		}
 	}
 

selfservice/hook/verification.go

@@ -83,6 +83,10 @@ func (e *Verifier) do(w http.ResponseWriter, r *http.Request, i *identity.Identi
 			return err
 		}
 
+		if f, ok := f.(flow.Challenger); ok {
+			verificationFlow.OAuth2LoginChallenge = f.GetOAuth2LoginChallenge()
+		}
+
 		verificationFlow.State = verification.StateEmailSent
 
 		if err := strategy.PopulateVerificationMethod(r, verificationFlow); err != nil {

session/handler.go

@@ -190,7 +190,7 @@ type toSession struct {
 //	  401: errorGeneric
 //	  403: errorGeneric
 //	  default: errorGeneric
-func (h *Handler) whoami(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
+func (h *Handler) whoami(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
 	s, err := h.r.SessionManager().FetchFromRequest(r.Context(), r)
 	c := h.r.Config()
 	if err != nil {