Merge pull request #126 from canonical/IAM-349-incorrect-content-type

Tranform Kratos 422 HTTP resp to 200

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)
 }
 

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)

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)
 }

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) {

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)

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;