Merge pull request #249 from canonical/IAM-860-implement-multi-factor…

…-authentication

Support multi factor authentication

docker/kratos/kratos.yml

@@ -22,10 +22,7 @@ identity:
           url: file:///etc/config/kratos/identity.schema.json
 selfservice:
     allowed_return_urls:
-      - http://localhost:4455/ui/login
-      - http://localhost:4455/ui/reset_email
-      - http://localhost:4455/ui/reset_password
-      - http://localhost:4455/ui/reset_complete
+      - http://localhost:4455/ui/
     default_browser_return_url:
         http://localhost:4455/ui/
     flows:
@@ -43,12 +40,17 @@ selfservice:
                     - hook: revoke_active_sessions
         settings:
             ui_url: http://localhost:4455/ui/reset_password
+            required_aal: highest_available
         registration:
             after:
                 oidc:
                     hooks:
                     - hook: session
     methods:
+        totp:
+         enabled: true
+         config:
+           issuer: GoogleAuthenticator
         password:
             enabled: True
             config:

pkg/kratos/handlers.go

@@ -3,14 +3,13 @@ package kratos
 import (
 	"context"
 	"encoding/json"
-	"fmt"
 	"net/http"
 	"net/url"
 	"strconv"
-	"strings"
 
-	"github.com/canonical/identity-platform-login-ui/internal/logging"
 	"github.com/go-chi/chi/v5"
+
+	"github.com/canonical/identity-platform-login-ui/internal/logging"
 )
 
 type API struct {
@@ -72,7 +71,7 @@ func (a *API) handleCreateFlow(w http.ResponseWriter, r *http.Request) {
 
 	returnTo, err := url.JoinPath(a.baseURL, "/ui/login")
 	if err != nil {
-		a.logger.Fatal("Failed to construct returnTo URL: ", err)
+		a.logger.Errorf("Failed to construct returnTo URL: %v", err)
 	}
 	returnTo = returnTo + "?login_challenge=" + loginChallenge
 
@@ -231,24 +230,28 @@ func (a *API) handleUpdateRecoveryFlow(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	flow, cookies, err := a.service.UpdateRecoveryFlow(context.Background(), flowId, *body, r.Cookies())
+	flow, cookies, err := a.service.UpdateRecoveryFlow(r.Context(), flowId, *body, r.Cookies())
 	if err != nil {
 		a.logger.Errorf("Error when updating recovery flow: %v\n", err)
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
-	resp, err := json.Marshal(flow)
-	if err != nil {
-		a.logger.Errorf("Error when marshalling json: %v\n", err)
-		http.Error(w, "Failed to parse recovery flow", http.StatusInternalServerError)
+	if !flow.HasRedirectTo() && flow.HasError() {
+		a.logger.Errorf("Error when updating recovery flow: %v\n", flow)
+		w.WriteHeader(http.StatusInternalServerError)
+		_ = json.NewEncoder(w).Encode(flow)
 		return
 	}
+
 	setCookies(w, cookies)
-	// Kratos returns us a '422' response but we tranform it to a '200',
-	// because this is the expected behavior for us.
+	// Kratos '422' response maps to 200 OK, it is expected
 	w.WriteHeader(http.StatusOK)
-	w.Write(resp)
+	_ = json.NewEncoder(w).Encode(
+		BrowserLocationChangeRequired{
+			RedirectTo: flow.RedirectTo,
+		},
+	)
 }
 
 func (a *API) handleCreateRecoveryFlow(w http.ResponseWriter, r *http.Request) {
@@ -280,20 +283,32 @@ func (a *API) handleCreateRecoveryFlow(w http.ResponseWriter, r *http.Request) {
 func (a *API) handleGetSettingsFlow(w http.ResponseWriter, r *http.Request) {
 	q := r.URL.Query()
 
-	flow, cookies, err := a.service.GetSettingsFlow(context.Background(), q.Get("id"), r.Cookies())
+	flow, response, err := a.service.GetSettingsFlow(context.Background(), q.Get("id"), r.Cookies())
 	if err != nil {
 		a.logger.Errorf("Error when getting settings flow: %v\n", err)
 		http.Error(w, "Failed to get settings flow", http.StatusInternalServerError)
 		return
 	}
 
+	// If aal1, redirect to complete second factor auth
+	if response != nil && response.HasRedirectTo() {
+		a.logger.Errorf("Failed to get settings flow due to insufficient aal: %v\n", response)
+		w.WriteHeader(http.StatusForbidden)
+		_ = json.NewEncoder(w).Encode(
+			ErrorBrowserLocationChangeRequired{
+				Error:             response.Error,
+				RedirectBrowserTo: response.RedirectTo,
+			},
+		)
+		return
+	}
+
 	resp, err := flow.MarshalJSON()
 	if err != nil {
 		a.logger.Errorf("Error when marshalling json: %v\n", err)
-		http.Error(w, "Failed to parse settings flow", http.StatusInternalServerError)
+		http.Error(w, "Failed to marshal json", http.StatusInternalServerError)
 		return
 	}
-	setCookies(w, cookies)
 	w.WriteHeader(http.StatusOK)
 	w.Write(resp)
 }
@@ -328,26 +343,38 @@ func (a *API) handleUpdateSettingsFlow(w http.ResponseWriter, r *http.Request) {
 }
 
 func (a *API) handleCreateSettingsFlow(w http.ResponseWriter, r *http.Request) {
-	returnTo, err := url.JoinPath(a.baseURL, "/ui/reset_complete")
+	returnTo, err := url.JoinPath(a.baseURL, "/ui/setup_complete")
 	if err != nil {
 		a.logger.Errorf("Failed to construct returnTo URL: ", err)
 		http.Error(w, "Failed to construct returnTo URL", http.StatusBadRequest)
 	}
 
-	flow, cookies, err := a.service.CreateBrowserSettingsFlow(context.Background(), returnTo, r.Cookies())
+	flow, response, err := a.service.CreateBrowserSettingsFlow(context.Background(), returnTo, r.Cookies())
 	if err != nil {
 		a.logger.Errorf("Failed to create settings flow: %v", err)
 		http.Error(w, "Failed to create settings flow", http.StatusInternalServerError)
 		return
 	}
 
+	// If aal1, redirect to complete second factor auth
+	if response != nil && response.HasRedirectTo() {
+		a.logger.Errorf("Failed to create settings flow due to insufficient aal: %v\n", response)
+		w.WriteHeader(http.StatusForbidden)
+		_ = json.NewEncoder(w).Encode(
+			ErrorBrowserLocationChangeRequired{
+				Error:             response.Error,
+				RedirectBrowserTo: response.RedirectTo,
+			},
+		)
+		return
+	}
+
 	resp, err := flow.MarshalJSON()
 	if err != nil {
 		a.logger.Errorf("Error when marshalling json: %v\n", err)
 		http.Error(w, "Failed to marshal json", http.StatusInternalServerError)
 		return
 	}
-	setCookies(w, cookies)
 	w.WriteHeader(http.StatusOK)
 	w.Write(resp)
 }
@@ -368,11 +395,3 @@ func setCookies(w http.ResponseWriter, cookies []*http.Cookie) {
 		http.SetCookie(w, c)
 	}
 }
-
-func cookiesToString(cookies []*http.Cookie) string {
-	var ret = make([]string, len(cookies))
-	for i, c := range cookies {
-		ret[i] = fmt.Sprintf("%s=%s", c.Name, c.Value)
-	}
-	return strings.Join(ret, "; ")
-}

pkg/kratos/handlers_test.go

@@ -784,14 +784,14 @@ func TestHandleCreateSettingsFlow(t *testing.T) {
 	mockLogger := NewMockLoggerInterface(ctrl)
 	mockService := NewMockServiceInterface(ctrl)
 
-	redirect := "https://example.com/ui/reset_complete"
+	redirect := "https://example.com/ui/setup_complete"
 
 	req := httptest.NewRequest(http.MethodGet, HANDLE_CREATE_SETTINGS_FLOW_URL, nil)
 	values := req.URL.Query()
 	req.URL.RawQuery = values.Encode()
 
 	flow := kClient.NewSettingsFlowWithDefaults()
-	mockService.EXPECT().CreateBrowserSettingsFlow(gomock.Any(), redirect, req.Cookies()).Return(flow, req.Cookies(), nil)
+	mockService.EXPECT().CreateBrowserSettingsFlow(gomock.Any(), redirect, req.Cookies()).Return(flow, nil, nil)
 
 	w := httptest.NewRecorder()
 	mux := chi.NewMux()
@@ -809,14 +809,51 @@ func TestHandleCreateSettingsFlow(t *testing.T) {
 	}
 }
 
+func TestHandleCreateSettingsFlowWithRedirect(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	mockLogger := NewMockLoggerInterface(ctrl)
+	mockService := NewMockServiceInterface(ctrl)
+
+	redirect := "https://example.com/ui/setup_complete"
+
+	redirectErrorBrowserTo := "https://some/path/to/somewhere"
+	redirectFlow := new(BrowserLocationChangeRequired)
+	redirectFlow.RedirectTo = &redirectErrorBrowserTo
+
+	req := httptest.NewRequest(http.MethodGet, HANDLE_CREATE_SETTINGS_FLOW_URL, nil)
+	values := req.URL.Query()
+	req.URL.RawQuery = values.Encode()
+
+	flow := kClient.NewSettingsFlowWithDefaults()
+	mockService.EXPECT().CreateBrowserSettingsFlow(gomock.Any(), redirect, req.Cookies()).Return(flow, redirectFlow, nil)
+	mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).Times(1)
+
+	w := httptest.NewRecorder()
+	mux := chi.NewMux()
+	NewAPI(mockService, BASE_URL, mockLogger).RegisterEndpoints(mux)
+
+	mux.ServeHTTP(w, req)
+
+	res := w.Result()
+
+	if _, err := json.Marshal(flow); err != nil {
+		t.Fatalf("Expected error to be nil got %v", err)
+	}
+	if res.StatusCode != http.StatusForbidden {
+		t.Fatal("Expected HTTP status code 403, got: ", res.Status)
+	}
+}
+
 func TestHandleCreateSettingsFlowFailOnCreateBrowserSettingsFlow(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 
 	mockLogger := NewMockLoggerInterface(ctrl)
 	mockService := NewMockServiceInterface(ctrl)
 
-	redirect := "https://example.com/ui/reset_complete"
+	redirect := "https://example.com/ui/setup_complete"
 
 	req := httptest.NewRequest(http.MethodGet, HANDLE_CREATE_SETTINGS_FLOW_URL, nil)
 	values := req.URL.Query()
@@ -855,7 +892,7 @@ func TestHandleGetSettingsFlow(t *testing.T) {
 	values.Add("id", id)
 	req.URL.RawQuery = values.Encode()
 
-	mockService.EXPECT().GetSettingsFlow(gomock.Any(), id, req.Cookies()).Return(flow, req.Cookies(), nil)
+	mockService.EXPECT().GetSettingsFlow(gomock.Any(), id, req.Cookies()).Return(flow, nil, nil)
 
 	w := httptest.NewRecorder()
 	mux := chi.NewMux()
@@ -881,6 +918,47 @@ func TestHandleGetSettingsFlow(t *testing.T) {
 	}
 }
 
+func TestHandleGetSettingsFlowWithRedirect(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	mockLogger := NewMockLoggerInterface(ctrl)
+	mockService := NewMockServiceInterface(ctrl)
+
+	id := "test"
+	flow := kClient.NewSettingsFlowWithDefaults()
+	flow.SetId(id)
+	flow.SetState("show_form")
+
+	req := httptest.NewRequest(http.MethodGet, HANDLE_GET_SETTINGS_FLOW_URL, nil)
+	values := req.URL.Query()
+	values.Add("id", id)
+	req.URL.RawQuery = values.Encode()
+
+	redirectErrorBrowserTo := "https://some/path/to/somewhere"
+	redirectFlow := new(BrowserLocationChangeRequired)
+	redirectFlow.RedirectTo = &redirectErrorBrowserTo
+
+	mockService.EXPECT().GetSettingsFlow(gomock.Any(), id, req.Cookies()).Return(flow, redirectFlow, nil)
+	mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).Times(1)
+
+	w := httptest.NewRecorder()
+	mux := chi.NewMux()
+	NewAPI(mockService, BASE_URL, mockLogger).RegisterEndpoints(mux)
+
+	mux.ServeHTTP(w, req)
+
+	res := w.Result()
+
+	if res.StatusCode != http.StatusForbidden {
+		t.Fatal("Expected HTTP status code 403, got: ", res.Status)
+	}
+	_, err := io.ReadAll(res.Body)
+	if err != nil {
+		t.Fatalf("Expected error to be nil got %v", err)
+	}
+}
+
 func TestHandleGetSettingsFlowFail(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()

pkg/kratos/interfaces.go

@@ -27,10 +27,10 @@ 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)
 	CreateBrowserRecoveryFlow(context.Context, string, []*http.Cookie) (*kClient.RecoveryFlow, []*http.Cookie, error)
-	CreateBrowserSettingsFlow(context.Context, string, []*http.Cookie) (*kClient.SettingsFlow, []*http.Cookie, error)
+	CreateBrowserSettingsFlow(context.Context, string, []*http.Cookie) (*kClient.SettingsFlow, *BrowserLocationChangeRequired, error)
 	GetLoginFlow(context.Context, string, []*http.Cookie) (*kClient.LoginFlow, []*http.Cookie, error)
 	GetRecoveryFlow(context.Context, string, []*http.Cookie) (*kClient.RecoveryFlow, []*http.Cookie, error)
-	GetSettingsFlow(context.Context, string, []*http.Cookie) (*kClient.SettingsFlow, []*http.Cookie, error)
+	GetSettingsFlow(context.Context, string, []*http.Cookie) (*kClient.SettingsFlow, *BrowserLocationChangeRequired, error)
 	UpdateLoginFlow(context.Context, string, kClient.UpdateLoginFlowBody, []*http.Cookie) (*BrowserLocationChangeRequired, []*http.Cookie, error)
 	UpdateRecoveryFlow(context.Context, string, kClient.UpdateRecoveryFlowBody, []*http.Cookie) (*BrowserLocationChangeRequired, []*http.Cookie, error)
 	UpdateSettingsFlow(context.Context, string, kClient.UpdateSettingsFlowBody, []*http.Cookie) (*kClient.SettingsFlow, []*http.Cookie, error)