diff --git a/pkg/kratos/handlers.go b/pkg/kratos/handlers.go index ed9cb9d23..2ea5179ff 100644 --- a/pkg/kratos/handlers.go +++ b/pkg/kratos/handlers.go @@ -86,7 +86,7 @@ func (a *API) handleCreateFlow(w http.ResponseWriter, r *http.Request) { return } setCookies(w, cookies) - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) w.Write(resp) } @@ -108,7 +108,7 @@ func (a *API) handleGetLoginFlow(w http.ResponseWriter, r *http.Request) { return } setCookies(w, cookies) - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) w.Write(resp) } @@ -137,7 +137,9 @@ func (a *API) handleUpdateFlow(w http.ResponseWriter, r *http.Request) { return } setCookies(w, cookies) - w.WriteHeader(422) + // Kratos returns us a '422' response but we tranform it to a '200', + // because this is the expected behavior for us. + w.WriteHeader(http.StatusOK) w.Write(resp) } @@ -160,7 +162,7 @@ func (a *API) handleKratosError(w http.ResponseWriter, r *http.Request) { return } setCookies(w, cookies) - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) w.Write(resp) } diff --git a/pkg/kratos/handlers_test.go b/pkg/kratos/handlers_test.go index 35bc2dba0..17c961af9 100644 --- a/pkg/kratos/handlers_test.go +++ b/pkg/kratos/handlers_test.go @@ -272,8 +272,8 @@ func TestHandleUpdateFlow(t *testing.T) { flowId := "test" redirectTo := "https://some/path/to/somewhere" - flow := new(ErrorBrowserLocationChangeRequired) - flow.RedirectBrowserTo = &redirectTo + flow := new(BrowserLocationChangeRequired) + flow.RedirectTo = &redirectTo flowBody := new(kClient.UpdateLoginFlowBody) flowBody.UpdateLoginFlowWithOidcMethod = kClient.NewUpdateLoginFlowWithOidcMethod("oidc", "oidc") @@ -294,8 +294,8 @@ func TestHandleUpdateFlow(t *testing.T) { res := w.Result() - if res.StatusCode != http.StatusUnprocessableEntity { - t.Fatal("Expected HTTP status code 422, got: ", res.Status) + if res.StatusCode != http.StatusOK { + t.Fatal("Expected HTTP status code 200, got: ", res.Status) } data, err := ioutil.ReadAll(res.Body) diff --git a/pkg/kratos/interfaces.go b/pkg/kratos/interfaces.go index 2df89e965..1fefc84d6 100644 --- a/pkg/kratos/interfaces.go +++ b/pkg/kratos/interfaces.go @@ -21,7 +21,7 @@ type ServiceInterface interface { AcceptLoginRequest(context.Context, string, string) (*hClient.OAuth2RedirectTo, []*http.Cookie, error) CreateBrowserLoginFlow(context.Context, string, string, string, bool, []*http.Cookie) (*kClient.LoginFlow, []*http.Cookie, error) GetLoginFlow(context.Context, string, []*http.Cookie) (*kClient.LoginFlow, []*http.Cookie, error) - UpdateOIDCLoginFlow(context.Context, string, kClient.UpdateLoginFlowBody, []*http.Cookie) (*ErrorBrowserLocationChangeRequired, []*http.Cookie, error) + UpdateOIDCLoginFlow(context.Context, string, kClient.UpdateLoginFlowBody, []*http.Cookie) (*BrowserLocationChangeRequired, []*http.Cookie, error) GetFlowError(context.Context, string) (*kClient.FlowError, []*http.Cookie, error) ParseLoginFlowMethodBody(*http.Request) (*kClient.UpdateLoginFlowBody, error) } diff --git a/pkg/kratos/service.go b/pkg/kratos/service.go index 9b52afc3c..24459c9cd 100644 --- a/pkg/kratos/service.go +++ b/pkg/kratos/service.go @@ -31,6 +31,11 @@ type ErrorBrowserLocationChangeRequired struct { RedirectBrowserTo *string `json:"redirect_browser_to,omitempty"` } +type BrowserLocationChangeRequired struct { + // Points to where to redirect the user to next. + RedirectTo *string `json:"redirect_to,omitempty"` +} + func (s *Service) CheckSession(ctx context.Context, cookies []*http.Cookie) (*kClient.Session, []*http.Cookie, error) { ctx, span := s.tracer.Start(ctx, "kratos.FrontendApi.ToSession") defer span.End() @@ -100,7 +105,7 @@ func (s *Service) GetLoginFlow(ctx context.Context, id string, cookies []*http.C Id(id). Cookie(cookiesToString(cookies)). Execute() - if err != nil && resp.StatusCode != 422 { + if err != nil { s.logger.Debugf("full HTTP response: %v", resp) return nil, nil, err } @@ -110,7 +115,7 @@ func (s *Service) GetLoginFlow(ctx context.Context, id string, cookies []*http.C func (s *Service) UpdateOIDCLoginFlow( ctx context.Context, flow string, body kClient.UpdateLoginFlowBody, cookies []*http.Cookie, -) (*ErrorBrowserLocationChangeRequired, []*http.Cookie, error) { +) (*BrowserLocationChangeRequired, []*http.Cookie, error) { ctx, span := s.tracer.Start(ctx, "kratos.FrontendApi.UpdateLoginFlow") defer span.End() @@ -120,6 +125,11 @@ func (s *Service) UpdateOIDCLoginFlow( UpdateLoginFlowBody(body). Cookie(cookiesToString(cookies)). Execute() + // We expect to get a 422 response from Kratos. The sdk forces us to + // make the request with an 'application/json' content-type, whereas Kratos + // expects the 'Content-Type' and 'Accept' to be 'application/x-www-form-urlencoded'. + // This is not a real error, as we still get the URL to which the user needs to be + // redirected to. if err != nil && resp.StatusCode != 422 { s.logger.Debugf("full HTTP response: %v", resp) return nil, nil, err @@ -131,7 +141,13 @@ func (s *Service) UpdateOIDCLoginFlow( s.logger.Debugf("Failed to unmarshal JSON: %s", err) return nil, nil, err } - return redirectResp, resp.Cookies(), nil + + // We trasform the kratos response to our own custom response here. + // The original kratos response contains an 'Error' field, which we remove + // because this is not a real error. + returnToResp := BrowserLocationChangeRequired{redirectResp.RedirectBrowserTo} + + return &returnToResp, resp.Cookies(), nil } func (s *Service) GetFlowError(ctx context.Context, id string) (*kClient.FlowError, []*http.Cookie, error) { diff --git a/pkg/kratos/service_test.go b/pkg/kratos/service_test.go index 1e6646e90..6eac64a0b 100644 --- a/pkg/kratos/service_test.go +++ b/pkg/kratos/service_test.go @@ -463,8 +463,8 @@ func TestUpdateLoginFlowSuccess(t *testing.T) { f, c, err := NewService(mockKratos, mockHydra, mockTracer, mockMonitor, mockLogger).UpdateOIDCLoginFlow(ctx, flowId, *body, cookies) - if !reflect.DeepEqual(*f, flow) { - t.Fatalf("expected flow to be %+v not %+v", flow, *f) + if *f.RedirectTo != *flow.RedirectBrowserTo { + t.Fatalf("expected redirectTo to be %s not %s", *flow.RedirectBrowserTo, *f.RedirectTo) } if !reflect.DeepEqual(c, resp.Cookies()) { t.Fatalf("expected cookies to be %v not %v", resp.Cookies(), c) diff --git a/ui/pages/login.tsx b/ui/pages/login.tsx index 37d07b311..51cccec84 100644 --- a/ui/pages/login.tsx +++ b/ui/pages/login.tsx @@ -77,7 +77,11 @@ const Login: NextPage = () => { updateLoginFlowBody: values, }) // We logged in successfully! Let's bring the user home. - .then(async () => { + .then(async ({ data }) => { + if ("redirect_to" in data) { + window.location.href = data.redirect_to as string; + return; + } if (flow?.return_to) { window.location.href = flow.return_to; return;