Merge pull request #348 from canonical/IAM-933-fix-post-login-logout-…

…redirect

IAM 933 - Fix UI serving and implement `next` param passing for FE deep redirection

README.md

@@ -22,7 +22,7 @@ This is the Admin UI for the Canonical Identity Platform.
   to `error`
 - `LOG_FILE`: file where to dump logs, defaults to `log.txt`
 - `PORT`: http server port, defaults to `8080`
-- `BASE_URL`: the base url that the application will be running on, needed if the application runs under a path
+- `CONTEXT_PATH`: the context path that the application will be served on, needed to perform redirection correctly
 - `DEBUG`: debugging flag for hydra and kratos clients
 - `KUBECONFIG_FILE`: optional path of kube config file, default to empty string
 - `KRATOS_PUBLIC_URL`: Kratos public endpoints address

cmd/serve.go

@@ -122,8 +122,7 @@ func serve() {
 	rulesConfig := rules.NewConfig(specs.RulesConfigMapName, specs.RulesConfigFileName, specs.RulesConfigMapNamespace, k8sCoreV1, externalConfig.OathkeeperPublic().ApiApi())
 
 	uiConfig := &ui.Config{
-		DistFS:  distFS,
-		BaseURL: specs.BaseURL,
+		DistFS: distFS,
 	}
 
 	if specs.AuthorizationEnabled {
@@ -156,7 +155,7 @@ func serve() {
 
 	ollyConfig := web.NewO11yConfig(tracer, monitor, logger)
 
-	routerConfig := web.NewRouterConfig(specs.PayloadValidationEnabled, idpConfig, schemasConfig, rulesConfig, uiConfig, externalConfig, oauth2Config, ollyConfig)
+	routerConfig := web.NewRouterConfig(specs.ContextPath, specs.PayloadValidationEnabled, idpConfig, schemasConfig, rulesConfig, uiConfig, externalConfig, oauth2Config, ollyConfig)
 
 	wpool := pool.NewWorkerPool(specs.OpenFGAWorkersTotal, tracer, monitor, logger)
 	defer wpool.Stop()

internal/config/specs.go

@@ -12,8 +12,8 @@ type EnvSpec struct {
 	LogLevel string `envconfig:"log_level" default:"error"`
 	LogFile  string `envconfig:"log_file" default:"log.txt"`
 
-	Port    int    `envconfig:"port" default:"8080"`
-	BaseURL string `envconfig:"base_url" default:""`
+	Port        int    `envconfig:"port" default:"8080"`
+	ContextPath string `envconfig:"context_path" default:"/"`
 
 	Debug bool `envconfig:"debug" default:"false"`
 

pkg/authentication/cookies.go

@@ -15,6 +15,7 @@ const (
 	authCookiePath    = "/api/v0/auth/callback"
 	nonceCookieName   = "nonce"
 	stateCookieName   = "state"
+	nextToCookieName  = "nextTo"
 	itCookieName      = "id-token"
 	atCookieName      = "access-token"
 	rtCookieName      = "refresh-token"
@@ -92,6 +93,18 @@ func (a *AuthCookieManager) ClearStateCookie(w http.ResponseWriter) {
 	a.clearCookie(w, stateCookieName, authCookiePath)
 }
 
+func (a *AuthCookieManager) SetNextToCookie(w http.ResponseWriter, next string) {
+	a.setCookie(w, nextToCookieName, next, authCookiePath, a.authCookiesTTL)
+}
+
+func (a *AuthCookieManager) GetNextToCookie(r *http.Request) string {
+	return a.getCookie(r, nextToCookieName)
+}
+
+func (a *AuthCookieManager) ClearNextToCookie(w http.ResponseWriter) {
+	a.clearCookie(w, nextToCookieName, authCookiePath)
+}
+
 func (a *AuthCookieManager) setCookie(w http.ResponseWriter, name, value string, path string, ttl time.Duration) {
 	if value == "" {
 		return
@@ -135,7 +148,6 @@ func (a *AuthCookieManager) clearCookie(w http.ResponseWriter, name string, path
 func (a *AuthCookieManager) getCookie(r *http.Request, name string) string {
 	cookie, err := r.Cookie(name)
 	if err != nil {
-		a.logger.Errorf("can't get cookie %s, %v", name, err)
 		return ""
 	}
 

pkg/authentication/cookies_test.go

@@ -154,7 +154,6 @@ func TestAuthCookieManager_GetNonceCookieFailure(t *testing.T) {
 	ctrl := gomock.NewController(t)
 
 	mockLogger := NewMockLoggerInterface(ctrl)
-	mockLogger.EXPECT().Errorf("can't get cookie %s, %v", "nonce", gomock.Any()).Times(1)
 	mockRequest := httptest.NewRequest(http.MethodGet, "/", nil)
 
 	manager := NewAuthCookieManager(5, 5, nil, mockLogger)
@@ -208,7 +207,6 @@ func TestAuthCookieManager_GetStateCookieFailure(t *testing.T) {
 	ctrl := gomock.NewController(t)
 
 	mockLogger := NewMockLoggerInterface(ctrl)
-	mockLogger.EXPECT().Errorf("can't get cookie %s, %v", "state", gomock.Any()).Times(1)
 	mockRequest := httptest.NewRequest(http.MethodGet, "/", nil)
 
 	manager := NewAuthCookieManager(5, 5, nil, mockLogger)
@@ -262,7 +260,6 @@ func TestAuthCookieManager_GetIDTokenCookieFailure(t *testing.T) {
 	ctrl := gomock.NewController(t)
 
 	mockLogger := NewMockLoggerInterface(ctrl)
-	mockLogger.EXPECT().Errorf("can't get cookie %s, %v", "id-token", gomock.Any()).Times(1)
 	mockRequest := httptest.NewRequest(http.MethodGet, "/", nil)
 
 	manager := NewAuthCookieManager(5, 5, nil, mockLogger)
@@ -316,7 +313,6 @@ func TestAuthCookieManager_GetAccessTokenCookieFailure(t *testing.T) {
 	ctrl := gomock.NewController(t)
 
 	mockLogger := NewMockLoggerInterface(ctrl)
-	mockLogger.EXPECT().Errorf("can't get cookie %s, %v", "access-token", gomock.Any()).Times(1)
 	mockRequest := httptest.NewRequest(http.MethodGet, "/", nil)
 
 	manager := NewAuthCookieManager(5, 5, nil, mockLogger)
@@ -370,7 +366,6 @@ func TestAuthCookieManager_GetRefreshTokenCookieFailure(t *testing.T) {
 	ctrl := gomock.NewController(t)
 
 	mockLogger := NewMockLoggerInterface(ctrl)
-	mockLogger.EXPECT().Errorf("can't get cookie %s, %v", "refresh-token", gomock.Any()).Times(1)
 	mockRequest := httptest.NewRequest(http.MethodGet, "/", nil)
 
 	manager := NewAuthCookieManager(5, 5, nil, mockLogger)

pkg/authentication/handlers.go

@@ -7,6 +7,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"net/url"
 
 	"github.com/go-chi/chi/v5"
 	"go.opentelemetry.io/otel/trace"
@@ -67,6 +68,7 @@ func NewAuthenticationConfig(
 
 type API struct {
 	apiKey           string
+	contextPath      string
 	payloadValidator validation.PayloadValidatorInterface
 	oauth2           OAuth2ContextInterface
 	helper           OAuth2HelperInterface
@@ -87,6 +89,10 @@ func (a *API) handleLogin(w http.ResponseWriter, r *http.Request) {
 	// add the Otel HTTP Client
 	r = r.WithContext(OtelHTTPClientContext(r.Context()))
 
+	if nextTo := r.URL.Query().Get("next"); nextTo != "" {
+		a.cookieManager.SetNextToCookie(w, nextTo)
+	}
+
 	nonce := a.helper.RandomURLString()
 	state := a.helper.RandomURLString()
 
@@ -158,7 +164,9 @@ func (a *API) handleCallback(w http.ResponseWriter, r *http.Request) {
 	a.cookieManager.SetAccessTokenCookie(w, oauth2Token.AccessToken)
 	a.cookieManager.SetRefreshTokenCookie(w, oauth2Token.RefreshToken)
 
-	http.Redirect(w, r, ui.UIPrefix, http.StatusFound)
+	nextTo := a.cookieManager.GetNextToCookie(r)
+	a.cookieManager.ClearNextToCookie(w)
+	a.uiRedirect(w, r, nextTo)
 }
 
 func (a *API) checkNonce(r *http.Request, idToken *Principal) error {
@@ -194,6 +202,7 @@ func (a *API) checkState(r *http.Request, state string) error {
 func (a *API) badRequest(w http.ResponseWriter, err error) {
 	a.cookieManager.ClearNonceCookie(w)
 	a.cookieManager.ClearStateCookie(w)
+	a.cookieManager.ClearNextToCookie(w)
 
 	w.WriteHeader(http.StatusBadRequest)
 	_ = json.NewEncoder(w).Encode(
@@ -244,10 +253,29 @@ func (a *API) handleLogout(w http.ResponseWriter, r *http.Request) {
 	a.cookieManager.ClearAccessTokenCookie(w)
 	a.cookieManager.ClearRefreshTokenCookie(w)
 
-	http.Redirect(w, r, ui.UIPrefix, http.StatusFound)
+	nextTo := r.URL.Query().Get("next")
+	a.uiRedirect(w, r, nextTo)
+}
+
+func (a *API) uiRedirect(w http.ResponseWriter, r *http.Request, nextTo string) {
+	redirect := ui.UIPrefix
+
+	if nextTo != "" {
+		redirectURL, _ := url.Parse(redirect)
+		query := redirectURL.Query()
+		query.Set("next", nextTo)
+		redirectURL.RawQuery = query.Encode()
+		redirect = redirectURL.String()
+	}
+
+	// handle context path in redirection response
+	r.URL.Path = a.contextPath
+
+	http.Redirect(w, r, redirect, http.StatusFound)
 }
 
 func NewAPI(
+	contextPath string,
 	oauth2Context OAuth2ContextInterface,
 	helper OAuth2HelperInterface,
 	cookieManager AuthCookieManagerInterface,
@@ -256,6 +284,7 @@ func NewAPI(
 ) *API {
 	a := new(API)
 	a.apiKey = "authentication"
+	a.contextPath = contextPath
 	a.oauth2 = oauth2Context
 	a.helper = helper
 	a.cookieManager = cookieManager

pkg/authentication/handlers_test.go

@@ -63,6 +63,7 @@ func TestHandleLogin(t *testing.T) {
 	}
 
 	api := NewAPI(
+		"",
 		NewOAuth2Context(config, mockOIDCProviderSupplier(&oidc.Provider{}, nil), mockTracer, mockLogger, mockMonitor),
 		mockHelper,
 		NewAuthCookieManager(mockTTLSeconds, mockTTLSeconds, mockEncrypt, mockLogger),
@@ -143,6 +144,7 @@ func TestHandleLoginCallback(t *testing.T) {
 	mockResponse := httptest.NewRecorder()
 
 	api := NewAPI(
+		"",
 		mockOauth2Ctx,
 		mockHelper,
 		NewAuthCookieManager(mockTTLSeconds, mockTTLSeconds, mockEncrypt, mockLogger),
@@ -244,7 +246,6 @@ func TestHandleLoginCallbackFailures(t *testing.T) {
 			request: mockRequestNoStateCookie,
 			setupMocks: func(oauth2Ctx *MockOAuth2ContextInterface, logger *MockLoggerInterface, verifier *MockTokenVerifier, encrypt *MockEncryptInterface) {
 				logger.EXPECT().Error("state cookie not found")
-				logger.EXPECT().Errorf("can't get cookie %s, %v", "state", gomock.Any())
 			},
 			errorMessage: "state cookie not found",
 		},
@@ -301,7 +302,6 @@ func TestHandleLoginCallbackFailures(t *testing.T) {
 			setupMocks: func(oauth2Ctx *MockOAuth2ContextInterface, logger *MockLoggerInterface, verifier *MockTokenVerifier, encrypt *MockEncryptInterface) {
 				logger.EXPECT().Debugf("user login second leg with code '%s'", "mock-code").Times(1)
 				logger.EXPECT().Error("nonce cookie not found")
-				logger.EXPECT().Errorf("can't get cookie %s, %v", "nonce", gomock.Any())
 				mockToken = mockToken.WithExtra(map[string]interface{}{"id_token": "mock-id-token"})
 				oauth2Ctx.EXPECT().RetrieveTokens(gomock.Any(), gomock.Eq("mock-code")).Return(mockToken, nil)
 
@@ -351,6 +351,7 @@ func TestHandleLoginCallbackFailures(t *testing.T) {
 			mockResponse := httptest.NewRecorder()
 
 			api := NewAPI(
+				"",
 				mockOauth2Ctx,
 				mockHelper,
 				NewAuthCookieManager(mockTTLSeconds, mockTTLSeconds, mockEncrypt, mockLogger),
@@ -406,6 +407,7 @@ func TestHandleMe(t *testing.T) {
 	mockResponse := httptest.NewRecorder()
 
 	api := NewAPI(
+		"",
 		mockOauth2Ctx,
 		mockHelper,
 		NewAuthCookieManager(mockTTLSeconds, mockTTLSeconds, mockEncrypt, mockLogger),
@@ -475,6 +477,7 @@ func TestLogout(t *testing.T) {
 
 				m.EXPECT().ClearNonceCookie(gomock.Any()).Times(1)
 				m.EXPECT().ClearStateCookie(gomock.Any()).Times(1)
+				m.EXPECT().ClearNextToCookie(gomock.Any()).Times(1)
 			},
 		},
 	} {
@@ -501,6 +504,7 @@ func TestLogout(t *testing.T) {
 
 			mockCookieManager := NewMockAuthCookieManagerInterface(ctrl)
 			api := NewAPI(
+				"",
 				mockOauth2Ctx,
 				mockHelper,
 				mockCookieManager,

pkg/authentication/interfaces.go

@@ -86,6 +86,12 @@ type AuthCookieManagerInterface interface {
 	GetRefreshTokenCookie(*http.Request) string
 	// ClearRefreshTokenCookie sets the expiration of the cookie to epoch
 	ClearRefreshTokenCookie(http.ResponseWriter)
+	// SetNextToCookie sets the encrypted nextTo relative url value cookie
+	SetNextToCookie(http.ResponseWriter, string)
+	// GetNextToCookie  returns the string value of the nextTo cookie if present, or empty string otherwise
+	GetNextToCookie(*http.Request) string
+	// ClearNextToCookie sets the expiration of the cookie to epoch
+	ClearNextToCookie(http.ResponseWriter)
 }
 
 type EncryptInterface interface {

pkg/ui/handlers.go

@@ -4,50 +4,33 @@
 package ui
 
 import (
-	"fmt"
 	"io/fs"
 	"net/http"
-	"net/url"
-	"path"
 	"strings"
 
+	"github.com/go-chi/chi/v5"
+
 	"github.com/canonical/identity-platform-admin-ui/internal/logging"
 	"github.com/canonical/identity-platform-admin-ui/internal/monitoring"
 	"github.com/canonical/identity-platform-admin-ui/internal/tracing"
-	"github.com/go-chi/chi/v5"
 )
 
 const UIPrefix = "/ui"
 
 type Config struct {
-	DistFS  fs.FS
-	BaseURL string
+	DistFS fs.FS
 }
 
 type API struct {
 	fileServer http.Handler
-	BaseURL    *url.URL
 
 	tracer  tracing.TracingInterface
 	monitor monitoring.MonitorInterface
 	logger  logging.LoggerInterface
 }
 
 func (a *API) RegisterEndpoints(mux *chi.Mux) {
-	// We force the trailing slash to make the UI routing easier
-	// The UI relies on relative routes to fetch it's assets and to call the backend api
-	// If we didn't force the trailing slash it would be harder to use relative paths:
-	//     "example.com/a"    + "./b"  -> "example.com/b"
-	//     "example.com/a/"   + "./b"  -> "example.com/a/b"
-	//     "example.com/a/b"  + "../c" -> "example.com/c"
-	//     "example.com/a/b/" + "../c" -> "example.com/a/c"
-	mux.Get(UIPrefix, func(w http.ResponseWriter, r *http.Request) {
-		url := UIPrefix
-		if a.BaseURL != nil {
-			url = path.Join(a.BaseURL.Path, url) + "/"
-		}
-		http.Redirect(w, r, url, http.StatusMovedPermanently)
-	})
+	mux.Get(UIPrefix, a.uiFiles)
 	mux.Get(UIPrefix+"/*", a.uiFiles)
 }
 
@@ -80,13 +63,6 @@ func NewAPI(config *Config, tracer tracing.TracingInterface, monitor monitoring.
 	a := new(API)
 
 	a.fileServer = http.FileServer(http.FS(config.DistFS))
-	if config.BaseURL != "" {
-		url, err := url.Parse(config.BaseURL)
-		if err != nil {
-			panic(fmt.Errorf("invalid Base URL provided: %s", err))
-		}
-		a.BaseURL = url
-	}
 
 	a.tracer = tracer
 	a.monitor = monitor

pkg/web/router.go

@@ -30,6 +30,7 @@ import (
 )
 
 type RouterConfig struct {
+	contextPath              string
 	payloadValidationEnabled bool
 	idp                      *idp.Config
 	schemas                  *schemas.Config
@@ -40,8 +41,9 @@ type RouterConfig struct {
 	olly                     O11yConfigInterface
 }
 
-func NewRouterConfig(payloadValidationEnabled bool, idp *idp.Config, schemas *schemas.Config, rules *rules.Config, ui *ui.Config, external ExternalClientsConfigInterface, oauth2 *authentication.Config, olly O11yConfigInterface) *RouterConfig {
+func NewRouterConfig(contextPath string, payloadValidationEnabled bool, idp *idp.Config, schemas *schemas.Config, rules *rules.Config, ui *ui.Config, external ExternalClientsConfigInterface, oauth2 *authentication.Config, olly O11yConfigInterface) *RouterConfig {
 	return &RouterConfig{
+		contextPath:              contextPath,
 		payloadValidationEnabled: payloadValidationEnabled,
 		idp:                      idp,
 		schemas:                  schemas,
@@ -188,6 +190,7 @@ func NewRouter(config *RouterConfig, wpool pool.WorkerPoolInterface) http.Handle
 	if oauth2Config.Enabled {
 
 		login := authentication.NewAPI(
+			config.contextPath,
 			oauth2Context,
 			authentication.NewOAuth2Helper(),
 			cookieManager,