feat: preserve oidc params in oauth flow

e2e/README.md

@@ -1,28 +0,0 @@
-# E2E Framework
-
-[Project link](https://github.com/orgs/tinyauthapp/projects/1/views/1)
-
-This is designed as an E2E framework to be able to test for changes in common proxy and application apps that tinyauth users are likely to use.
-
-This is **not** designed to test functionality, it is a [Canary](https://en.wikipedia.org/wiki/Sentinel_species#Canaries). All functionailty testing is already done by Unit tests within the standard tinyauth PR / release workflows.
-
-## Design
-
-Primary testing is via Docker, although a minimal Kubernetes stack is also planned.
-
-Initially this is being created to test the proxy connection, and ability to login.
-
-Testing of endpoints and providers will be done via `traefik`.
-
-It requires at least two endpoints, one will be `whoami` as an easy "is this working", but it also later requires an OIDC test (TBD), and a nested HTTP Auth (TBD).
-
-It should test against all "known" Oauth providers (ie, the ones that are specifically mentioned in the documentation, including community supplied if possible).
-
-> [!NOTE]
-> This requires having both Google and Github logins for the built-in providers, so security for those on a public E2E setup must be taken into account.
-
-## Running
-
-Run the <./test.sh> script, this handles everything for all tests.
-
-TODO: Implement options to limit testing to specific proxies and auth services.

e2e/compose.base.yaml

@@ -1,10 +0,0 @@
-# This contains base apps without any proxy information
-
-services:
-  tinyauth:
-    image: ${TINYAUTH_IMAGE:-ghcr.io/steveiliop56/tinyauth}:${TINYAUTH_IMAGE_TAG:-v5}
-    environment:
-      TINYAUTH_ANALYTICS_ENABLED: "false"
-      TINYAUTH_APPURL: "https://tinyauth.${DOMAIN:-local}"
-    volumes:
-      - "./config:/data"

e2e/compose.traefik.yaml

@@ -1,44 +0,0 @@
-# This contains Traefik proxy versions
-# All apps must be prefixed by `traefik-`
-
-services:
-  traefik:
-    container_name: traefik
-    image: ${TRAEFIK_IMAGE:-traefik}:${TRAEFIK_IMAGE_TAG:-v3}
-    networks:
-      - e2e
-    environment:
-      TZ: "${TZ:-Europe/London}"
-      PUID: "${PUID:-1000}"
-      PGID: "${PGID:-1000}"
-      UMASK: "000"
-    command:
-      - "--entryPoints.web.address=:80"
-      - "--entryPoints.web.http.redirections.entryPoint.scheme=https"
-      - "--entrypoints.web.http.redirections.entryPoint.to=websecure"
-      - "--entryPoints.websecure.address=:443"
-      - "--providers.docker=true"
-      - "--providers.docker.endpoint=/var/run/docker.sock"
-    volumes:
-      - "/var/run/docker.sock:/var/run/docker.sock:ro"
-      - "./.ssl/key.pem:/run/secrets/key.pem:ro"
-      - "./.ssl/cert.pem:/run/secrets/cert.pem:ro"
-      - "./config:/etc/traefik"
-
-  traefik-tinyauth:
-    container_name: traefik-tinyauth
-    extends:
-      file: ../compose.base.yaml
-      service: tinyauth
-    networks:
-      e2e-external:
-      e2e:
-        aliases:
-          - "traefik-tinyauth.$DOMAIN"
-    labels:
-      - "traefik.enable=true"
-      - "traefik.http.routers.tinyauth.rule=Host(`tinyauth.$DOMAIN`)"
-      - "traefik.http.services.tinyauth.loadbalancer.server.port=3000"
-      - "traefik.http.middlewares.tinyauth.forwardauth.address=http://tinyauth:3000/api/auth/traefik"
-      - "traefik.http.middlewares.tinyauth.forwardauth.authResponseHeaders=X-Forwarded-User"
-      - "traefik.http.middlewares.tinyauth.forwardauth.maxResponseBodySize=32768"

e2e/compose.yaml

@@ -1,53 +0,0 @@
-name: tinyauth-e2e
-
-services:
-  traefik-tinyauth:
-    container_name: traefik-tinyauth
-    extends:
-      file: ../compose.base.yaml
-      service: tinyauth
-    networks:
-      e2e-external:
-      e2e:
-        aliases:
-          - "traefik-tinyauth.$DOMAIN"
-    labels:
-      - "traefik.enable=true"
-      - "traefik.http.routers.tinyauth.rule=Host(`tinyauth.$DOMAIN`)"
-      - "traefik.http.services.tinyauth.loadbalancer.server.port=3000"
-      - "traefik.http.middlewares.tinyauth.forwardauth.address=http://tinyauth:3000/api/auth/traefik"
-      - "traefik.http.middlewares.tinyauth.forwardauth.authResponseHeaders=X-Forwarded-User"
-      - "traefik.http.middlewares.tinyauth.forwardauth.maxResponseBodySize=32768"
-
-  traefik-tinyauth-google:
-    container_name: traefik-tinyauth-google
-    secrets:
-      - google_client_secret
-    environment:
-      TINYAUTH_OAUTH_PROVIDERS_GOOGLE_CLIENTID: "$GOOGLE_CLIENT_ID"
-      TINYAUTH_OAUTH_PROVIDERS_GOOGLE_CLIENTSECRETFILE: "/run/secrets/google_client_secret"
-      TINYAUTH_OAUTH_WHITELIST: "${WHITELIST:?Set the WHITELIST to your google email address!}"
-
-  whoami:
-    image: traefik/whoami:latest
-    networks:
-      e2e:
-        aliases:
-          - "whoami.$DOMAIN"
-    labels:
-      traefik.enable: true
-      traefik.http.routers.whoami.rule: Host(`whoami.$DOMAIN`)
-      traefik.http.routers.whoami.middlewares: tinyauth
-
-networks:
-  e2e-external:
-    name: "e2e-external"
-    driver: bridge
-    enable_ipv4: true
-    enable_ipv6: true
-  e2e:
-    name: "e2e"
-    driver: bridge
-    internal: true
-    enable_ipv4: true
-    enable_ipv6: false

e2e/test.sh

@@ -1,2 +0,0 @@
-#! /bin/bash
-

frontend/src/pages/login-page.tsx

@@ -76,10 +76,14 @@ export const LoginPage = () => {
     isPending: oauthIsPending,
     variables: oauthVariables,
   } = useMutation({
-    mutationFn: (provider: string) =>
-      axios.get(
-        `/api/oauth/url/${provider}${props.redirect_uri ? `?redirect_uri=${encodeURIComponent(props.redirect_uri)}` : ""}`,
-      ),
+    mutationFn: (provider: string) => {
+      const params = isOidc
+        ? `?${compiledOIDCParams}`
+        : props.redirect_uri
+          ? `?redirect_uri=${encodeURIComponent(props.redirect_uri)}`
+          : "";
+      return axios.get(`/api/oauth/url/${provider}${params}`);
+    },
     mutationKey: ["oauth"],
     onSuccess: (data) => {
       toast.info(t("loginOauthSuccessTitle"), {

internal/controller/oauth_controller.go

@@ -62,7 +62,29 @@ func (controller *OAuthController) oauthURLHandler(c *gin.Context) {
 		return
 	}
 
-	sessionId, session, err := controller.auth.NewOAuthSession(req.Provider)
+	var reqParams service.OAuthURLParams
+
+	err = c.BindQuery(&reqParams)
+
+	if err != nil {
+		tlog.App.Error().Err(err).Msg("Failed to bind query parameters")
+		c.JSON(400, gin.H{
+			"status":  400,
+			"message": "Bad Request",
+		})
+		return
+	}
+
+	if !controller.isOidcRequest(reqParams) {
+		isRedirectSafe := utils.IsRedirectSafe(reqParams.RedirectURI, controller.config.CookieDomain)
+
+		if !isRedirectSafe {
+			tlog.App.Warn().Str("redirect_uri", reqParams.RedirectURI).Msg("Unsafe redirect URI detected, ignoring")
+			reqParams.RedirectURI = ""
+		}
+	}
+
+	sessionId, _, err := controller.auth.NewOAuthSession(req.Provider, reqParams)
 
 	if err != nil {
 		tlog.App.Error().Err(err).Msg("Failed to create OAuth session")
@@ -85,20 +107,6 @@ func (controller *OAuthController) oauthURLHandler(c *gin.Context) {
 	}
 
 	c.SetCookie(controller.config.OAuthSessionCookieName, sessionId, int(time.Hour.Seconds()), "/", fmt.Sprintf(".%s", controller.config.CookieDomain), controller.config.SecureCookie, true)
-	c.SetCookie(controller.config.CSRFCookieName, session.State, int(time.Hour.Seconds()), "/", fmt.Sprintf(".%s", controller.config.CookieDomain), controller.config.SecureCookie, true)
-
-	redirectURI := c.Query("redirect_uri")
-	isRedirectSafe := utils.IsRedirectSafe(redirectURI, controller.config.CookieDomain)
-
-	if !isRedirectSafe {
-		tlog.App.Warn().Str("redirect_uri", redirectURI).Msg("Unsafe redirect URI detected, ignoring")
-		redirectURI = ""
-	}
-
-	if redirectURI != "" && isRedirectSafe {
-		tlog.App.Debug().Msg("Setting redirect URI cookie")
-		c.SetCookie(controller.config.RedirectCookieName, redirectURI, int(time.Hour.Seconds()), "/", fmt.Sprintf(".%s", controller.config.CookieDomain), controller.config.SecureCookie, true)
-	}
 
 	c.JSON(200, gin.H{
 		"status":  200,
@@ -129,19 +137,23 @@ func (controller *OAuthController) oauthCallbackHandler(c *gin.Context) {
 	}
 
 	c.SetCookie(controller.config.OAuthSessionCookieName, "", -1, "/", fmt.Sprintf(".%s", controller.config.CookieDomain), controller.config.SecureCookie, true)
-	defer controller.auth.EndOAuthSession(sessionIdCookie)
 
-	state := c.Query("state")
-	csrfCookie, err := c.Cookie(controller.config.CSRFCookieName)
+	oauthPendingSession, err := controller.auth.GetOAuthPendingSession(sessionIdCookie)
 
-	if err != nil || state != csrfCookie {
-		tlog.App.Warn().Err(err).Msg("CSRF token mismatch or cookie missing")
-		c.SetCookie(controller.config.CSRFCookieName, "", -1, "/", fmt.Sprintf(".%s", controller.config.CookieDomain), controller.config.SecureCookie, true)
+	if err != nil {
+		tlog.App.Error().Err(err).Msg("Failed to get OAuth pending session")
 		c.Redirect(http.StatusTemporaryRedirect, fmt.Sprintf("%s/error", controller.config.AppURL))
 		return
 	}
 
-	c.SetCookie(controller.config.CSRFCookieName, "", -1, "/", fmt.Sprintf(".%s", controller.config.CookieDomain), controller.config.SecureCookie, true)
+	defer controller.auth.EndOAuthSession(sessionIdCookie)
+
+	state := c.Query("state")
+	if state != oauthPendingSession.State {
+		tlog.App.Warn().Err(err).Msg("CSRF token mismatch")
+		c.Redirect(http.StatusTemporaryRedirect, fmt.Sprintf("%s/error", controller.config.AppURL))
+		return
+	}
 
 	code := c.Query("code")
 	_, err = controller.auth.GetOAuthToken(sessionIdCookie, code)
@@ -198,7 +210,7 @@ func (controller *OAuthController) oauthCallbackHandler(c *gin.Context) {
 		username = strings.Replace(user.Email, "@", "_", 1)
 	}
 
-	service, err := controller.auth.GetOAuthService(sessionIdCookie)
+	svc, err := controller.auth.GetOAuthService(sessionIdCookie)
 
 	if err != nil {
 		tlog.App.Error().Err(err).Msg("Failed to get OAuth service for session")
@@ -206,8 +218,8 @@ func (controller *OAuthController) oauthCallbackHandler(c *gin.Context) {
 		return
 	}
 
-	if service.ID() != req.Provider {
-		tlog.App.Error().Msgf("OAuth service ID mismatch: expected %s, got %s", service.ID(), req.Provider)
+	if svc.ID() != req.Provider {
+		tlog.App.Error().Msgf("OAuth service ID mismatch: expected %s, got %s", svc.ID(), req.Provider)
 		c.Redirect(http.StatusTemporaryRedirect, fmt.Sprintf("%s/error", controller.config.AppURL))
 		return
 	}
@@ -216,9 +228,9 @@ func (controller *OAuthController) oauthCallbackHandler(c *gin.Context) {
 		Username:    username,
 		Name:        name,
 		Email:       user.Email,
-		Provider:    service.ID(),
+		Provider:    svc.ID(),
 		OAuthGroups: utils.CoalesceToString(user.Groups),
-		OAuthName:   service.Name(),
+		OAuthName:   svc.Name(),
 		OAuthSub:    user.Sub,
 	}
 
@@ -234,24 +246,39 @@ func (controller *OAuthController) oauthCallbackHandler(c *gin.Context) {
 
 	tlog.AuditLoginSuccess(c, sessionCookie.Username, sessionCookie.Provider)
 
-	redirectURI, err := c.Cookie(controller.config.RedirectCookieName)
-
-	if err != nil || !utils.IsRedirectSafe(redirectURI, controller.config.CookieDomain) {
-		tlog.App.Debug().Msg("No redirect URI cookie found, redirecting to app root")
-		c.Redirect(http.StatusTemporaryRedirect, controller.config.AppURL)
+	if controller.isOidcRequest(oauthPendingSession.CallbackParams) {
+		tlog.App.Debug().Msg("OIDC request, redirecting to authorize page")
+		queries, err := query.Values(oauthPendingSession.CallbackParams)
+		if err != nil {
+			tlog.App.Error().Err(err).Msg("Failed to encode OIDC callback query")
+			c.Redirect(http.StatusTemporaryRedirect, fmt.Sprintf("%s/error", controller.config.AppURL))
+			return
+		}
+		c.Redirect(http.StatusTemporaryRedirect, fmt.Sprintf("%s/authorize?%s", controller.config.AppURL, queries.Encode()))
 		return
 	}
 
-	queries, err := query.Values(config.RedirectQuery{
-		RedirectURI: redirectURI,
-	})
+	if oauthPendingSession.CallbackParams.RedirectURI != "" {
+		queries, err := query.Values(config.RedirectQuery{
+			RedirectURI: oauthPendingSession.CallbackParams.RedirectURI,
+		})
 
-	if err != nil {
-		tlog.App.Error().Err(err).Msg("Failed to encode redirect URI query")
-		c.Redirect(http.StatusTemporaryRedirect, fmt.Sprintf("%s/error", controller.config.AppURL))
+		if err != nil {
+			tlog.App.Error().Err(err).Msg("Failed to encode redirect URI query")
+			c.Redirect(http.StatusTemporaryRedirect, fmt.Sprintf("%s/error", controller.config.AppURL))
+			return
+		}
+
+		c.Redirect(http.StatusTemporaryRedirect, fmt.Sprintf("%s/continue?%s", controller.config.AppURL, queries.Encode()))
 		return
 	}
 
-	c.SetCookie(controller.config.RedirectCookieName, "", -1, "/", fmt.Sprintf(".%s", controller.config.CookieDomain), controller.config.SecureCookie, true)
-	c.Redirect(http.StatusTemporaryRedirect, fmt.Sprintf("%s/continue?%s", controller.config.AppURL, queries.Encode()))
+	c.Redirect(http.StatusTemporaryRedirect, controller.config.AppURL)
+}
+
+func (controller *OAuthController) isOidcRequest(params service.OAuthURLParams) bool {
+	return params.Scope != "" &&
+		params.ResponseType != "" &&
+		params.ClientID != "" &&
+		params.RedirectURI != ""
 }

internal/service/auth_service.go

@@ -28,12 +28,26 @@ const MaxOAuthPendingSessions = 256
 const OAuthCleanupCount = 16
 const MaxLoginAttemptRecords = 256
 
+// slightly modified version of the AuthorizeRequest from the OIDC service to basically accept all
+// parameters and pass them to the authorize page if needed
+type OAuthURLParams struct {
+	Scope               string `form:"scope" url:"scope"`
+	ResponseType        string `form:"response_type" url:"response_type"`
+	ClientID            string `form:"client_id" url:"client_id"`
+	RedirectURI         string `form:"redirect_uri" url:"redirect_uri"`
+	State               string `form:"state" url:"state"`
+	Nonce               string `form:"nonce" url:"nonce"`
+	CodeChallenge       string `form:"code_challenge" url:"code_challenge"`
+	CodeChallengeMethod string `form:"code_challenge_method" url:"code_challenge_method"`
+}
+
 type OAuthPendingSession struct {
-	State     string
-	Verifier  string
-	Token     *oauth2.Token
-	Service   *OAuthServiceImpl
-	ExpiresAt time.Time
+	State          string
+	Verifier       string
+	Token          *oauth2.Token
+	Service        *OAuthServiceImpl
+	ExpiresAt      time.Time
+	CallbackParams OAuthURLParams
 }
 
 type LdapGroupsCache struct {
@@ -598,7 +612,7 @@ func (auth *AuthService) IsBypassedIP(acls config.AppIP, ip string) bool {
 	return false
 }
 
-func (auth *AuthService) NewOAuthSession(serviceName string) (string, OAuthPendingSession, error) {
+func (auth *AuthService) NewOAuthSession(serviceName string, params OAuthURLParams) (string, OAuthPendingSession, error) {
 	auth.ensureOAuthSessionLimit()
 
 	service, ok := auth.oauthBroker.GetService(serviceName)
@@ -617,10 +631,11 @@ func (auth *AuthService) NewOAuthSession(serviceName string) (string, OAuthPendi
 	verifier := service.NewRandom()
 
 	session := OAuthPendingSession{
-		State:     state,
-		Verifier:  verifier,
-		Service:   &service,
-		ExpiresAt: time.Now().Add(1 * time.Hour),
+		State:          state,
+		Verifier:       verifier,
+		Service:        &service,
+		ExpiresAt:      time.Now().Add(1 * time.Hour),
+		CallbackParams: params,
 	}
 
 	auth.oauthMutex.Lock()
@@ -631,7 +646,7 @@ func (auth *AuthService) NewOAuthSession(serviceName string) (string, OAuthPendi
 }
 
 func (auth *AuthService) GetOAuthURL(sessionId string) (string, error) {
-	session, err := auth.getOAuthPendingSession(sessionId)
+	session, err := auth.GetOAuthPendingSession(sessionId)
 
 	if err != nil {
 		return "", err
@@ -641,7 +656,7 @@ func (auth *AuthService) GetOAuthURL(sessionId string) (string, error) {
 }
 
 func (auth *AuthService) GetOAuthToken(sessionId string, code string) (*oauth2.Token, error) {
-	session, err := auth.getOAuthPendingSession(sessionId)
+	session, err := auth.GetOAuthPendingSession(sessionId)
 
 	if err != nil {
 		return nil, err
@@ -661,7 +676,7 @@ func (auth *AuthService) GetOAuthToken(sessionId string, code string) (*oauth2.T
 }
 
 func (auth *AuthService) GetOAuthUserinfo(sessionId string) (config.Claims, error) {
-	session, err := auth.getOAuthPendingSession(sessionId)
+	session, err := auth.GetOAuthPendingSession(sessionId)
 
 	if err != nil {
 		return config.Claims{}, err
@@ -681,7 +696,7 @@ func (auth *AuthService) GetOAuthUserinfo(sessionId string) (config.Claims, erro
 }
 
 func (auth *AuthService) GetOAuthService(sessionId string) (OAuthServiceImpl, error) {
-	session, err := auth.getOAuthPendingSession(sessionId)
+	session, err := auth.GetOAuthPendingSession(sessionId)
 
 	if err != nil {
 		return nil, err
@@ -715,7 +730,7 @@ func (auth *AuthService) CleanupOAuthSessionsRoutine() {
 	}
 }
 
-func (auth *AuthService) getOAuthPendingSession(sessionId string) (*OAuthPendingSession, error) {
+func (auth *AuthService) GetOAuthPendingSession(sessionId string) (*OAuthPendingSession, error) {
 	auth.ensureOAuthSessionLimit()
 
 	auth.oauthMutex.RLock()