feat: add pkce support to oidc server

frontend/src/lib/hooks/oidc.ts

@@ -1,40 +1,64 @@
-import { z } from "zod";
+export type OIDCValues = {
+  scope: string;
+  response_type: string;
+  client_id: string;
+  redirect_uri: string;
+  state: string;
+  nonce: string;
+  code_challenge: string;
+  code_challenge_method: string;
+};
 
-export const oidcParamsSchema = z.object({
-  scope: z.string().min(1),
-  response_type: z.string().min(1),
-  client_id: z.string().min(1),
-  redirect_uri: z.string().min(1),
-  state: z.string().optional(),
-  nonce: z.string().optional(),
-  code_challenge: z.string().optional(),
-  code_challenge_method: z.string().optional(),
-});
-
-export const useOIDCParams = (
-  params: URLSearchParams,
-): {
-  values: z.infer<typeof oidcParamsSchema>;
-  issues: string[];
-  isOidc: boolean;
+interface IuseOIDCParams {
+  values: OIDCValues;
   compiled: string;
-} => {
-  const obj = Object.fromEntries(params.entries());
-  const parsed = oidcParamsSchema.safeParse(obj);
+  isOidc: boolean;
+  missingParams: string[];
+}
 
-  if (parsed.success) {
-    return {
-      values: parsed.data,
-      issues: [],
-      isOidc: true,
-      compiled: new URLSearchParams(parsed.data).toString(),
-    };
+const optionalParams: string[] = [
+  "state",
+  "nonce",
+  "code_challenge",
+  "code_challenge_method",
+];
+
+export function useOIDCParams(params: URLSearchParams): IuseOIDCParams {
+  let compiled: string = "";
+  let isOidc = false;
+  const missingParams: string[] = [];
+
+  const values: OIDCValues = {
+    scope: params.get("scope") ?? "",
+    response_type: params.get("response_type") ?? "",
+    client_id: params.get("client_id") ?? "",
+    redirect_uri: params.get("redirect_uri") ?? "",
+    state: params.get("state") ?? "",
+    nonce: params.get("nonce") ?? "",
+    code_challenge: params.get("code_challenge") ?? "",
+    code_challenge_method: params.get("code_challenge_method") ?? "",
+  };
+
+  for (const key of Object.keys(values)) {
+    if (!values[key as keyof OIDCValues]) {
+      if (!optionalParams.includes(key)) {
+        missingParams.push(key);
+      }
+    }
+  }
+
+  if (missingParams.length === 0) {
+    isOidc = true;
+  }
+
+  if (isOidc) {
+    compiled = new URLSearchParams(values).toString();
   }
 
   return {
-    issues: parsed.error.issues.map((issue) => issue.path.toString()),
-    values: {} as z.infer<typeof oidcParamsSchema>,
-    isOidc: false,
-    compiled: "",
+    values,
+    compiled,
+    isOidc,
+    missingParams,
   };
-};
+}

frontend/src/pages/authorize-page.tsx

@@ -72,27 +72,36 @@ export const AuthorizePage = () => {
   const scopeMap = createScopeMap(t);
 
   const searchParams = new URLSearchParams(search);
-  const oidcParams = useOIDCParams(searchParams);
+  const {
+    values: props,
+    missingParams,
+    isOidc,
+    compiled: compiledOIDCParams,
+  } = useOIDCParams(searchParams);
+  const scopes = props.scope ? props.scope.split(" ").filter(Boolean) : [];
 
   const getClientInfo = useQuery({
-    queryKey: ["client", oidcParams.values.client_id],
+    queryKey: ["client", props.client_id],
     queryFn: async () => {
-      const res = await fetch(
-        `/api/oidc/clients/${encodeURIComponent(oidcParams.values.client_id)}`,
-      );
+      const res = await fetch(`/api/oidc/clients/${props.client_id}`);
       const data = await getOidcClientInfoSchema.parseAsync(await res.json());
       return data;
     },
-    enabled: oidcParams.isOidc,
+    enabled: isOidc,
   });
 
   const authorizeMutation = useMutation({
     mutationFn: () => {
       return axios.post("/api/oidc/authorize", {
-        ...oidcParams.values,
+        scope: props.scope,
+        response_type: props.response_type,
+        client_id: props.client_id,
+        redirect_uri: props.redirect_uri,
+        state: props.state,
+        nonce: props.nonce,
       });
     },
-    mutationKey: ["authorize", oidcParams.values.client_id],
+    mutationKey: ["authorize", props.client_id],
     onSuccess: (data) => {
       toast.info(t("authorizeSuccessTitle"), {
         description: t("authorizeSuccessSubtitle"),
@@ -106,17 +115,17 @@ export const AuthorizePage = () => {
     },
   });
 
-  if (oidcParams.issues.length > 0) {
+  if (missingParams.length > 0) {
     return (
       <Navigate
-        to={`/error?error=${encodeURIComponent(t("authorizeErrorMissingParams", { missingParams: oidcParams.issues.join(", ") }))}`}
+        to={`/error?error=${encodeURIComponent(t("authorizeErrorMissingParams", { missingParams: missingParams.join(", ") }))}`}
         replace
       />
     );
   }
 
   if (!isLoggedIn) {
-    return <Navigate to={`/login?${oidcParams.compiled}`} replace />;
+    return <Navigate to={`/login?${compiledOIDCParams}`} replace />;
   }
 
   if (getClientInfo.isLoading) {
@@ -143,9 +152,6 @@ export const AuthorizePage = () => {
     );
   }
 
-  const scopes =
-    oidcParams.values.scope.split(" ").filter((s) => s.trim() !== "") || [];
-
   return (
     <Card>
       <CardHeader className="mb-2">

frontend/src/pages/login-page.tsx

@@ -51,12 +51,15 @@ export const LoginPage = () => {
   const formId = useId();
 
   const searchParams = new URLSearchParams(search);
-  const redirectUri = searchParams.get("redirect_uri") || undefined;
-  const oidcParams = useOIDCParams(searchParams);
+  const {
+    values: props,
+    isOidc,
+    compiled: compiledOIDCParams,
+  } = useOIDCParams(searchParams);
 
   const [isOauthAutoRedirect, setIsOauthAutoRedirect] = useState(
     providers.find((provider) => provider.id === oauthAutoRedirect) !==
-      undefined && redirectUri !== undefined,
+      undefined && props.redirect_uri,
   );
 
   const oauthProviders = providers.filter(
@@ -73,18 +76,10 @@ export const LoginPage = () => {
     isPending: oauthIsPending,
     variables: oauthVariables,
   } = useMutation({
-    mutationFn: (provider: string) => {
-      const getParams = function (): string {
-        if (oidcParams.isOidc) {
-          return `?${oidcParams.compiled}`;
-        }
-        if (redirectUri) {
-          return `?redirect_uri=${encodeURIComponent(redirectUri)}`;
-        }
-        return "";
-      };
-      return axios.get(`/api/oauth/url/${provider}${getParams()}`);
-    },
+    mutationFn: (provider: string) =>
+      axios.get(
+        `/api/oauth/url/${provider}${props.redirect_uri ? `?redirect_uri=${encodeURIComponent(props.redirect_uri)}` : ""}`,
+      ),
     mutationKey: ["oauth"],
     onSuccess: (data) => {
       toast.info(t("loginOauthSuccessTitle"), {
@@ -114,12 +109,8 @@ export const LoginPage = () => {
     mutationKey: ["login"],
     onSuccess: (data) => {
       if (data.data.totpPending) {
-        if (oidcParams.isOidc) {
-          window.location.replace(`/totp?${oidcParams.compiled}`);
-          return;
-        }
         window.location.replace(
-          `/totp${redirectUri ? `?redirect_uri=${encodeURIComponent(redirectUri)}` : ""}`,
+          `/totp${props.redirect_uri ? `?redirect_uri=${encodeURIComponent(props.redirect_uri)}` : ""}`,
         );
         return;
       }
@@ -129,12 +120,12 @@ export const LoginPage = () => {
       });
 
       redirectTimer.current = window.setTimeout(() => {
-        if (oidcParams.isOidc) {
-          window.location.replace(`/authorize?${oidcParams.compiled}`);
+        if (isOidc) {
+          window.location.replace(`/authorize?${compiledOIDCParams}`);
           return;
         }
         window.location.replace(
-          `/continue${redirectUri ? `?redirect_uri=${encodeURIComponent(redirectUri)}` : ""}`,
+          `/continue${props.redirect_uri ? `?redirect_uri=${encodeURIComponent(props.redirect_uri)}` : ""}`,
         );
       }, 500);
     },
@@ -153,7 +144,7 @@ export const LoginPage = () => {
       !isLoggedIn &&
       isOauthAutoRedirect &&
       !hasAutoRedirectedRef.current &&
-      redirectUri !== undefined
+      props.redirect_uri
     ) {
       hasAutoRedirectedRef.current = true;
       oauthMutate(oauthAutoRedirect);
@@ -164,7 +155,7 @@ export const LoginPage = () => {
     hasAutoRedirectedRef,
     oauthAutoRedirect,
     isOauthAutoRedirect,
-    redirectUri,
+    props.redirect_uri,
   ]);
 
   useEffect(() => {
@@ -179,14 +170,14 @@ export const LoginPage = () => {
     };
   }, [redirectTimer, redirectButtonTimer]);
 
-  if (isLoggedIn && oidcParams.isOidc) {
-    return <Navigate to={`/authorize?${oidcParams.compiled}`} replace />;
+  if (isLoggedIn && isOidc) {
+    return <Navigate to={`/authorize?${compiledOIDCParams}`} replace />;
   }
 
-  if (isLoggedIn && redirectUri !== undefined) {
+  if (isLoggedIn && props.redirect_uri !== "") {
     return (
       <Navigate
-        to={`/continue${redirectUri ? `?redirect_uri=${encodeURIComponent(redirectUri)}` : ""}`}
+        to={`/continue${props.redirect_uri ? `?redirect_uri=${encodeURIComponent(props.redirect_uri)}` : ""}`}
         replace
       />
     );

frontend/src/pages/totp-page.tsx

@@ -27,8 +27,11 @@ export const TotpPage = () => {
   const redirectTimer = useRef<number | null>(null);
 
   const searchParams = new URLSearchParams(search);
-  const redirectUri = searchParams.get("redirect_uri") || undefined;
-  const oidcParams = useOIDCParams(searchParams);
+  const {
+    values: props,
+    isOidc,
+    compiled: compiledOIDCParams,
+  } = useOIDCParams(searchParams);
 
   const totpMutation = useMutation({
     mutationFn: (values: TotpSchema) => axios.post("/api/user/totp", values),
@@ -39,13 +42,13 @@ export const TotpPage = () => {
       });
 
       redirectTimer.current = window.setTimeout(() => {
-        if (oidcParams.isOidc) {
-          window.location.replace(`/authorize?${oidcParams.compiled}`);
+        if (isOidc) {
+          window.location.replace(`/authorize?${compiledOIDCParams}`);
           return;
         }
 
         window.location.replace(
-          `/continue${redirectUri ? `?redirect_uri=${encodeURIComponent(redirectUri)}` : ""}`,
+          `/continue${props.redirect_uri ? `?redirect_uri=${encodeURIComponent(props.redirect_uri)}` : ""}`,
         );
       }, 500);
     },

internal/assets/migrations/000007_oidc_pkce.down.sql

@@ -1 +1,2 @@
 ALTER TABLE "oidc_codes" DROP COLUMN "code_challenge";
+ALTER TABLE "oidc_codes" DROP COLUMN "code_challenge_method";

internal/assets/migrations/000007_oidc_pkce.up.sql

@@ -1 +1,2 @@
 ALTER TABLE "oidc_codes" ADD COLUMN "code_challenge" TEXT DEFAULT "";
+ALTER TABLE "oidc_codes" ADD COLUMN "code_challenge_method" TEXT DEFAULT "";

internal/controller/context_controller_test.go

@@ -10,12 +10,10 @@ import (
 	"github.com/steveiliop56/tinyauth/internal/config"
 	"github.com/steveiliop56/tinyauth/internal/controller"
 	"github.com/steveiliop56/tinyauth/internal/utils"
-	"github.com/steveiliop56/tinyauth/internal/utils/tlog"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestContextController(t *testing.T) {
-	tlog.NewTestLogger().Init()
 	controllerConfig := controller.ContextControllerConfig{
 		Providers: []controller.Provider{
 			{

internal/controller/health_controller_test.go

@@ -8,12 +8,10 @@ import (
 
 	"github.com/gin-gonic/gin"
 	"github.com/steveiliop56/tinyauth/internal/controller"
-	"github.com/steveiliop56/tinyauth/internal/utils/tlog"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestHealthController(t *testing.T) {
-	tlog.NewTestLogger().Init()
 	tests := []struct {
 		description string
 		path        string

internal/controller/oauth_controller.go

@@ -62,29 +62,7 @@ func (controller *OAuthController) oauthURLHandler(c *gin.Context) {
 		return
 	}
 
-	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)
+	sessionId, session, err := controller.auth.NewOAuthSession(req.Provider)
 
 	if err != nil {
 		tlog.App.Error().Err(err).Msg("Failed to create OAuth session")
@@ -107,6 +85,20 @@ 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,
@@ -137,24 +129,20 @@ func (controller *OAuthController) oauthCallbackHandler(c *gin.Context) {
 	}
 
 	c.SetCookie(controller.config.OAuthSessionCookieName, "", -1, "/", fmt.Sprintf(".%s", controller.config.CookieDomain), controller.config.SecureCookie, true)
-
-	oauthPendingSession, err := controller.auth.GetOAuthPendingSession(sessionIdCookie)
-
-	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
-	}
-
 	defer controller.auth.EndOAuthSession(sessionIdCookie)
 
 	state := c.Query("state")
-	if state != oauthPendingSession.State {
-		tlog.App.Warn().Err(err).Msg("CSRF token mismatch")
+	csrfCookie, err := c.Cookie(controller.config.CSRFCookieName)
+
+	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)
 		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)
+
 	code := c.Query("code")
 	_, err = controller.auth.GetOAuthToken(sessionIdCookie, code)
 
@@ -210,7 +198,7 @@ func (controller *OAuthController) oauthCallbackHandler(c *gin.Context) {
 		username = strings.Replace(user.Email, "@", "_", 1)
 	}
 
-	svc, err := controller.auth.GetOAuthService(sessionIdCookie)
+	service, err := controller.auth.GetOAuthService(sessionIdCookie)
 
 	if err != nil {
 		tlog.App.Error().Err(err).Msg("Failed to get OAuth service for session")
@@ -218,8 +206,8 @@ func (controller *OAuthController) oauthCallbackHandler(c *gin.Context) {
 		return
 	}
 
-	if svc.ID() != req.Provider {
-		tlog.App.Error().Msgf("OAuth service ID mismatch: expected %s, got %s", svc.ID(), req.Provider)
+	if service.ID() != req.Provider {
+		tlog.App.Error().Msgf("OAuth service ID mismatch: expected %s, got %s", service.ID(), req.Provider)
 		c.Redirect(http.StatusTemporaryRedirect, fmt.Sprintf("%s/error", controller.config.AppURL))
 		return
 	}
@@ -228,9 +216,9 @@ func (controller *OAuthController) oauthCallbackHandler(c *gin.Context) {
 		Username:    username,
 		Name:        name,
 		Email:       user.Email,
-		Provider:    svc.ID(),
+		Provider:    service.ID(),
 		OAuthGroups: utils.CoalesceToString(user.Groups),
-		OAuthName:   svc.Name(),
+		OAuthName:   service.Name(),
 		OAuthSub:    user.Sub,
 	}
 
@@ -246,39 +234,24 @@ func (controller *OAuthController) oauthCallbackHandler(c *gin.Context) {
 
 	tlog.AuditLoginSuccess(c, sessionCookie.Username, sessionCookie.Provider)
 
-	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()))
+	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)
 		return
 	}
 
-	if oauthPendingSession.CallbackParams.RedirectURI != "" {
-		queries, err := query.Values(config.RedirectQuery{
-			RedirectURI: oauthPendingSession.CallbackParams.RedirectURI,
-		})
+	queries, err := query.Values(config.RedirectQuery{
+		RedirectURI: 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))
-			return
-		}
-
-		c.Redirect(http.StatusTemporaryRedirect, fmt.Sprintf("%s/continue?%s", controller.config.AppURL, queries.Encode()))
+	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, controller.config.AppURL)
-}
-
-func (controller *OAuthController) isOidcRequest(params service.OAuthURLParams) bool {
-	return params.Scope != "" &&
-		params.ResponseType != "" &&
-		params.ClientID != "" &&
-		params.RedirectURI != ""
+	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()))
 }

internal/controller/oidc_controller.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/gin-gonic/gin"
 	"github.com/google/go-querystring/query"
-
 	"github.com/steveiliop56/tinyauth/internal/service"
 	"github.com/steveiliop56/tinyauth/internal/utils"
 	"github.com/steveiliop56/tinyauth/internal/utils/tlog"
@@ -71,7 +70,6 @@ func (controller *OIDCController) SetupRoutes() {
 	oidcGroup.POST("/authorize", controller.Authorize)
 	oidcGroup.POST("/token", controller.Token)
 	oidcGroup.GET("/userinfo", controller.Userinfo)
-	oidcGroup.POST("/userinfo", controller.Userinfo)
 }
 
 func (controller *OIDCController) GetClientInfo(c *gin.Context) {
@@ -311,7 +309,7 @@ func (controller *OIDCController) Token(c *gin.Context) {
 			return
 		}
 
-		ok := controller.oidc.ValidatePKCE(entry.CodeChallenge, req.CodeVerifier)
+		ok := controller.oidc.ValidatePKCE(entry.CodeChallenge, entry.CodeChallengeMethod, req.CodeVerifier)
 
 		if !ok {
 			tlog.App.Warn().Msg("PKCE validation failed")
@@ -377,48 +375,22 @@ func (controller *OIDCController) Userinfo(c *gin.Context) {
 		return
 	}
 
-	var token string
-
 	authorization := c.GetHeader("Authorization")
-	if authorization != "" {
-		tokenType, bearerToken, ok := strings.Cut(authorization, " ")
-		if !ok {
-			tlog.App.Warn().Msg("OIDC userinfo accessed with malformed authorization header")
-			c.JSON(401, gin.H{
-				"error": "invalid_request",
-			})
-			return
-		}
 
-		if strings.ToLower(tokenType) != "bearer" {
-			tlog.App.Warn().Msg("OIDC userinfo accessed with invalid token type")
-			c.JSON(401, gin.H{
-				"error": "invalid_request",
-			})
-			return
-		}
+	tokenType, token, ok := strings.Cut(authorization, " ")
 
-		token = bearerToken
-	} else if c.Request.Method == http.MethodPost {
-		if c.ContentType() != "application/x-www-form-urlencoded" {
-			tlog.App.Warn().Msg("OIDC userinfo POST accessed with invalid content type")
-			c.JSON(400, gin.H{
-				"error": "invalid_request",
-			})
-			return
-		}
-		token = c.PostForm("access_token")
-		if token == "" {
-			tlog.App.Warn().Msg("OIDC userinfo POST accessed without access_token in body")
-			c.JSON(401, gin.H{
-				"error": "invalid_request",
-			})
-			return
-		}
-	} else {
+	if !ok {
 		tlog.App.Warn().Msg("OIDC userinfo accessed without authorization header")
 		c.JSON(401, gin.H{
-			"error": "invalid_request",
+			"error": "invalid_grant",
+		})
+		return
+	}
+
+	if strings.ToLower(tokenType) != "bearer" {
+		tlog.App.Warn().Msg("OIDC userinfo accessed with invalid token type")
+		c.JSON(401, gin.H{
+			"error": "invalid_grant",
 		})
 		return
 	}

internal/controller/oidc_controller_test.go

@@ -1,8 +1,6 @@
 package controller_test
 
 import (
-	"crypto/sha256"
-	"encoding/base64"
 	"encoding/json"
 	"net/http/httptest"
 	"net/url"
@@ -17,13 +15,11 @@ import (
 	"github.com/steveiliop56/tinyauth/internal/controller"
 	"github.com/steveiliop56/tinyauth/internal/repository"
 	"github.com/steveiliop56/tinyauth/internal/service"
-	"github.com/steveiliop56/tinyauth/internal/utils/tlog"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestOIDCController(t *testing.T) {
-	tlog.NewTestLogger().Init()
 	tempDir := t.TempDir()
 
 	oidcServiceCfg := service.OIDCServiceConfig{
@@ -435,349 +431,6 @@ func TestOIDCController(t *testing.T) {
 				assert.False(t, ok, "Did not expect email claim in userinfo response")
 			},
 		},
-		{
-			description: "Ensure userinfo forbids access with no authorization header",
-			middlewares: []gin.HandlerFunc{},
-			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
-				req := httptest.NewRequest("GET", "/api/oidc/userinfo", nil)
-				router.ServeHTTP(recorder, req)
-				assert.Equal(t, 401, recorder.Code)
-
-				var res map[string]any
-				err := json.Unmarshal(recorder.Body.Bytes(), &res)
-				assert.NoError(t, err)
-				assert.Equal(t, "invalid_request", res["error"])
-			},
-		},
-		{
-			description: "Ensure userinfo forbids access with malformed authorization header",
-			middlewares: []gin.HandlerFunc{},
-			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
-				req := httptest.NewRequest("GET", "/api/oidc/userinfo", nil)
-				req.Header.Set("Authorization", "Bearer")
-				router.ServeHTTP(recorder, req)
-				assert.Equal(t, 401, recorder.Code)
-
-				var res map[string]any
-				err := json.Unmarshal(recorder.Body.Bytes(), &res)
-				assert.NoError(t, err)
-				assert.Equal(t, "invalid_request", res["error"])
-			},
-		},
-		{
-			description: "Ensure userinfo forbids access with invalid token type",
-			middlewares: []gin.HandlerFunc{},
-			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
-				req := httptest.NewRequest("GET", "/api/oidc/userinfo", nil)
-				req.Header.Set("Authorization", "Basic some-token")
-				router.ServeHTTP(recorder, req)
-				assert.Equal(t, 401, recorder.Code)
-
-				var res map[string]any
-				err := json.Unmarshal(recorder.Body.Bytes(), &res)
-				assert.NoError(t, err)
-				assert.Equal(t, "invalid_request", res["error"])
-			},
-		},
-		{
-			description: "Ensure userinfo forbids access with empty bearer token",
-			middlewares: []gin.HandlerFunc{},
-			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
-				req := httptest.NewRequest("GET", "/api/oidc/userinfo", nil)
-				req.Header.Set("Authorization", "Bearer ")
-				router.ServeHTTP(recorder, req)
-				assert.Equal(t, 401, recorder.Code)
-
-				var res map[string]any
-				err := json.Unmarshal(recorder.Body.Bytes(), &res)
-				assert.NoError(t, err)
-				assert.Equal(t, "invalid_grant", res["error"])
-			},
-		},
-		{
-			description: "Ensure userinfo POST rejects missing access token in body",
-			middlewares: []gin.HandlerFunc{},
-			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
-				req := httptest.NewRequest("POST", "/api/oidc/userinfo", strings.NewReader(""))
-				req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-				router.ServeHTTP(recorder, req)
-				assert.Equal(t, 401, recorder.Code)
-
-				var res map[string]any
-				err := json.Unmarshal(recorder.Body.Bytes(), &res)
-				assert.NoError(t, err)
-				assert.Equal(t, "invalid_request", res["error"])
-			},
-		},
-		{
-			description: "Ensure userinfo POST rejects wrong content type",
-			middlewares: []gin.HandlerFunc{},
-			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
-				req := httptest.NewRequest("POST", "/api/oidc/userinfo", strings.NewReader(`{"access_token":"some-token"}`))
-				req.Header.Set("Content-Type", "application/json")
-				router.ServeHTTP(recorder, req)
-				assert.Equal(t, 400, recorder.Code)
-
-				var res map[string]any
-				err := json.Unmarshal(recorder.Body.Bytes(), &res)
-				assert.NoError(t, err)
-				assert.Equal(t, "invalid_request", res["error"])
-			},
-		},
-		{
-			description: "Ensure userinfo accepts access token via POST body",
-			middlewares: []gin.HandlerFunc{
-				simpleCtx,
-			},
-			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
-				tokenTest, found := getTestByDescription("Ensure we can get a token with a valid request")
-				assert.True(t, found, "Token test not found")
-				tokenRecorder := httptest.NewRecorder()
-				tokenTest(t, router, tokenRecorder)
-
-				var tokenRes map[string]any
-				err := json.Unmarshal(tokenRecorder.Body.Bytes(), &tokenRes)
-				assert.NoError(t, err)
-
-				accessToken := tokenRes["access_token"].(string)
-				assert.NotEmpty(t, accessToken)
-
-				body := url.Values{}
-				body.Set("access_token", accessToken)
-				req := httptest.NewRequest("POST", "/api/oidc/userinfo", strings.NewReader(body.Encode()))
-				req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-				router.ServeHTTP(recorder, req)
-				assert.Equal(t, 200, recorder.Code)
-
-				var userInfoRes map[string]any
-				err = json.Unmarshal(recorder.Body.Bytes(), &userInfoRes)
-				assert.NoError(t, err)
-
-				_, ok := userInfoRes["sub"]
-				assert.True(t, ok, "Expected sub claim in userinfo response")
-			},
-		},
-		{
-			description: "Ensure plain PKCE succeeds",
-			middlewares: []gin.HandlerFunc{
-				simpleCtx,
-			},
-			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
-				reqBody := service.AuthorizeRequest{
-					Scope:         "openid",
-					ResponseType:  "code",
-					ClientID:      "some-client-id",
-					RedirectURI:   "https://test.example.com/callback",
-					State:         "some-state",
-					Nonce:         "some-nonce",
-					CodeChallenge: "some-challenge",
-					// Not setting a code challenge method should default to "plain"
-					CodeChallengeMethod: "",
-				}
-				reqBodyBytes, err := json.Marshal(reqBody)
-				assert.NoError(t, err)
-
-				req := httptest.NewRequest("POST", "/api/oidc/authorize", strings.NewReader(string(reqBodyBytes)))
-				req.Header.Set("Content-Type", "application/json")
-				router.ServeHTTP(recorder, req)
-				assert.Equal(t, 200, recorder.Code)
-
-				var res map[string]any
-				err = json.Unmarshal(recorder.Body.Bytes(), &res)
-				assert.NoError(t, err)
-
-				redirectURI := res["redirect_uri"].(string)
-				url, err := url.Parse(redirectURI)
-				assert.NoError(t, err)
-
-				queryParams := url.Query()
-				assert.Equal(t, queryParams.Get("state"), "some-state")
-
-				code := queryParams.Get("code")
-				assert.NotEmpty(t, code)
-
-				// Now exchange the code for a token
-				recorder = httptest.NewRecorder()
-				tokenReqBody := controller.TokenRequest{
-					GrantType:    "authorization_code",
-					Code:         code,
-					RedirectURI:  "https://test.example.com/callback",
-					CodeVerifier: "some-challenge",
-				}
-				reqBodyEncoded, err := query.Values(tokenReqBody)
-				assert.NoError(t, err)
-
-				req = httptest.NewRequest("POST", "/api/oidc/token", strings.NewReader(reqBodyEncoded.Encode()))
-				req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-				req.SetBasicAuth("some-client-id", "some-client-secret")
-				router.ServeHTTP(recorder, req)
-
-				assert.Equal(t, 200, recorder.Code)
-			},
-		},
-		{
-			description: "Ensure S256 PKCE succeeds",
-			middlewares: []gin.HandlerFunc{
-				simpleCtx,
-			},
-			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
-				hasher := sha256.New()
-				hasher.Write([]byte("some-challenge"))
-				codeChallenge := hasher.Sum(nil)
-				codeChallengeEncoded := base64.RawURLEncoding.EncodeToString(codeChallenge)
-				reqBody := service.AuthorizeRequest{
-					Scope:               "openid",
-					ResponseType:        "code",
-					ClientID:            "some-client-id",
-					RedirectURI:         "https://test.example.com/callback",
-					State:               "some-state",
-					Nonce:               "some-nonce",
-					CodeChallenge:       codeChallengeEncoded,
-					CodeChallengeMethod: "S256",
-				}
-				reqBodyBytes, err := json.Marshal(reqBody)
-				assert.NoError(t, err)
-
-				req := httptest.NewRequest("POST", "/api/oidc/authorize", strings.NewReader(string(reqBodyBytes)))
-				req.Header.Set("Content-Type", "application/json")
-				router.ServeHTTP(recorder, req)
-				assert.Equal(t, 200, recorder.Code)
-
-				var res map[string]any
-				err = json.Unmarshal(recorder.Body.Bytes(), &res)
-				assert.NoError(t, err)
-
-				redirectURI := res["redirect_uri"].(string)
-				url, err := url.Parse(redirectURI)
-				assert.NoError(t, err)
-
-				queryParams := url.Query()
-				assert.Equal(t, queryParams.Get("state"), "some-state")
-
-				code := queryParams.Get("code")
-				assert.NotEmpty(t, code)
-
-				// Now exchange the code for a token
-				recorder = httptest.NewRecorder()
-				tokenReqBody := controller.TokenRequest{
-					GrantType:    "authorization_code",
-					Code:         code,
-					RedirectURI:  "https://test.example.com/callback",
-					CodeVerifier: "some-challenge",
-				}
-				reqBodyEncoded, err := query.Values(tokenReqBody)
-				assert.NoError(t, err)
-
-				req = httptest.NewRequest("POST", "/api/oidc/token", strings.NewReader(reqBodyEncoded.Encode()))
-				req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-				req.SetBasicAuth("some-client-id", "some-client-secret")
-				router.ServeHTTP(recorder, req)
-
-				assert.Equal(t, 200, recorder.Code)
-			},
-		},
-		{
-			description: "Ensure request with invalid PKCE fails",
-			middlewares: []gin.HandlerFunc{
-				simpleCtx,
-			},
-			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
-				hasher := sha256.New()
-				hasher.Write([]byte("some-challenge"))
-				codeChallenge := hasher.Sum(nil)
-				codeChallengeEncoded := base64.RawURLEncoding.EncodeToString(codeChallenge)
-				reqBody := service.AuthorizeRequest{
-					Scope:               "openid",
-					ResponseType:        "code",
-					ClientID:            "some-client-id",
-					RedirectURI:         "https://test.example.com/callback",
-					State:               "some-state",
-					Nonce:               "some-nonce",
-					CodeChallenge:       codeChallengeEncoded,
-					CodeChallengeMethod: "S256",
-				}
-				reqBodyBytes, err := json.Marshal(reqBody)
-				assert.NoError(t, err)
-
-				req := httptest.NewRequest("POST", "/api/oidc/authorize", strings.NewReader(string(reqBodyBytes)))
-				req.Header.Set("Content-Type", "application/json")
-				router.ServeHTTP(recorder, req)
-				assert.Equal(t, 200, recorder.Code)
-
-				var res map[string]any
-				err = json.Unmarshal(recorder.Body.Bytes(), &res)
-				assert.NoError(t, err)
-
-				redirectURI := res["redirect_uri"].(string)
-				url, err := url.Parse(redirectURI)
-				assert.NoError(t, err)
-
-				queryParams := url.Query()
-				assert.Equal(t, queryParams.Get("state"), "some-state")
-
-				code := queryParams.Get("code")
-				assert.NotEmpty(t, code)
-
-				// Now exchange the code for a token
-				recorder = httptest.NewRecorder()
-				tokenReqBody := controller.TokenRequest{
-					GrantType:    "authorization_code",
-					Code:         code,
-					RedirectURI:  "https://test.example.com/callback",
-					CodeVerifier: "some-challenge-1",
-				}
-				reqBodyEncoded, err := query.Values(tokenReqBody)
-				assert.NoError(t, err)
-
-				req = httptest.NewRequest("POST", "/api/oidc/token", strings.NewReader(reqBodyEncoded.Encode()))
-				req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-				req.SetBasicAuth("some-client-id", "some-client-secret")
-				router.ServeHTTP(recorder, req)
-
-				assert.Equal(t, 400, recorder.Code)
-			},
-		},
-		{
-			description: "Ensure request with invalid challenge method fails",
-			middlewares: []gin.HandlerFunc{
-				simpleCtx,
-			},
-			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
-				hasher := sha256.New()
-				hasher.Write([]byte("some-challenge"))
-				codeChallenge := hasher.Sum(nil)
-				codeChallengeEncoded := base64.RawURLEncoding.EncodeToString(codeChallenge)
-				reqBody := service.AuthorizeRequest{
-					Scope:               "openid",
-					ResponseType:        "code",
-					ClientID:            "some-client-id",
-					RedirectURI:         "https://test.example.com/callback",
-					State:               "some-state",
-					Nonce:               "some-nonce",
-					CodeChallenge:       codeChallengeEncoded,
-					CodeChallengeMethod: "foo",
-				}
-				reqBodyBytes, err := json.Marshal(reqBody)
-				assert.NoError(t, err)
-
-				req := httptest.NewRequest("POST", "/api/oidc/authorize", strings.NewReader(string(reqBodyBytes)))
-				req.Header.Set("Content-Type", "application/json")
-				router.ServeHTTP(recorder, req)
-				assert.Equal(t, 200, recorder.Code)
-
-				var res map[string]any
-				err = json.Unmarshal(recorder.Body.Bytes(), &res)
-				assert.NoError(t, err)
-
-				redirectURI := res["redirect_uri"].(string)
-				url, err := url.Parse(redirectURI)
-				assert.NoError(t, err)
-
-				queryParams := url.Query()
-				error := queryParams.Get("error")
-				assert.NotEmpty(t, error)
-			},
-		},
 	}
 
 	app := bootstrap.NewBootstrapApp(config.Config{})

internal/controller/proxy_controller_test.go

@@ -17,7 +17,6 @@ import (
 )
 
 func TestProxyController(t *testing.T) {
-	tlog.NewTestLogger().Init()
 	tempDir := t.TempDir()
 
 	authServiceCfg := service.AuthServiceConfig{
@@ -391,6 +390,8 @@ func TestProxyController(t *testing.T) {
 		},
 	}
 
+	tlog.NewSimpleLogger().Init()
+
 	oauthBrokerCfgs := make(map[string]config.OAuthServiceConfig)
 
 	app := bootstrap.NewBootstrapApp(config.Config{})

internal/controller/resources_controller_test.go

@@ -8,13 +8,11 @@ import (
 
 	"github.com/gin-gonic/gin"
 	"github.com/steveiliop56/tinyauth/internal/controller"
-	"github.com/steveiliop56/tinyauth/internal/utils/tlog"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestResourcesController(t *testing.T) {
-	tlog.NewTestLogger().Init()
 	tempDir := t.TempDir()
 
 	resourcesControllerCfg := controller.ResourcesControllerConfig{

internal/controller/user_controller_test.go

@@ -22,7 +22,6 @@ import (
 )
 
 func TestUserController(t *testing.T) {
-	tlog.NewTestLogger().Init()
 	tempDir := t.TempDir()
 
 	authServiceCfg := service.AuthServiceConfig{
@@ -275,6 +274,8 @@ func TestUserController(t *testing.T) {
 		},
 	}
 
+	tlog.NewSimpleLogger().Init()
+
 	oauthBrokerCfgs := make(map[string]config.OAuthServiceConfig)
 
 	app := bootstrap.NewBootstrapApp(config.Config{})

internal/controller/well_known_controller_test.go

@@ -13,13 +13,11 @@ import (
 	"github.com/steveiliop56/tinyauth/internal/controller"
 	"github.com/steveiliop56/tinyauth/internal/repository"
 	"github.com/steveiliop56/tinyauth/internal/service"
-	"github.com/steveiliop56/tinyauth/internal/utils/tlog"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestWellKnownController(t *testing.T) {
-	tlog.NewTestLogger().Init()
 	tempDir := t.TempDir()
 
 	oidcServiceCfg := service.OIDCServiceConfig{

internal/middleware/context_middleware.go

@@ -24,7 +24,6 @@ var (
 		"GET /api/oidc/clients",
 		"POST /api/oidc/token",
 		"GET /api/oidc/userinfo",
-		"POST /api/oidc/userinfo",
 		"GET /resources",
 		"POST /api/user/login",
 		"GET /.well-known/openid-configuration",

internal/repository/models.go

@@ -5,14 +5,15 @@
 package repository
 
 type OidcCode struct {
-	Sub           string
-	CodeHash      string
-	Scope         string
-	RedirectURI   string
-	ClientID      string
-	ExpiresAt     int64
-	Nonce         string
-	CodeChallenge string
+	Sub                 string
+	CodeHash            string
+	Scope               string
+	RedirectURI         string
+	ClientID            string
+	ExpiresAt           int64
+	Nonce               string
+	CodeChallenge       string
+	CodeChallengeMethod string
 }
 
 type OidcToken struct {

internal/repository/oidc_queries.sql.go

@@ -18,22 +18,24 @@ INSERT INTO "oidc_codes" (
     "client_id",
     "expires_at",
     "nonce",
-    "code_challenge"
+    "code_challenge",
+    "code_challenge_method"
 ) VALUES (
-    ?, ?, ?, ?, ?, ?, ?, ?
+    ?, ?, ?, ?, ?, ?, ?, ?, ?
 )
-RETURNING sub, code_hash, scope, redirect_uri, client_id, expires_at, nonce, code_challenge
+RETURNING sub, code_hash, scope, redirect_uri, client_id, expires_at, nonce, code_challenge, code_challenge_method
 `
 
 type CreateOidcCodeParams struct {
-	Sub           string
-	CodeHash      string
-	Scope         string
-	RedirectURI   string
-	ClientID      string
-	ExpiresAt     int64
-	Nonce         string
-	CodeChallenge string
+	Sub                 string
+	CodeHash            string
+	Scope               string
+	RedirectURI         string
+	ClientID            string
+	ExpiresAt           int64
+	Nonce               string
+	CodeChallenge       string
+	CodeChallengeMethod string
 }
 
 func (q *Queries) CreateOidcCode(ctx context.Context, arg CreateOidcCodeParams) (OidcCode, error) {
@@ -46,6 +48,7 @@ func (q *Queries) CreateOidcCode(ctx context.Context, arg CreateOidcCodeParams)
 		arg.ExpiresAt,
 		arg.Nonce,
 		arg.CodeChallenge,
+		arg.CodeChallengeMethod,
 	)
 	var i OidcCode
 	err := row.Scan(
@@ -57,6 +60,7 @@ func (q *Queries) CreateOidcCode(ctx context.Context, arg CreateOidcCodeParams)
 		&i.ExpiresAt,
 		&i.Nonce,
 		&i.CodeChallenge,
+		&i.CodeChallengeMethod,
 	)
 	return i, err
 }
@@ -160,7 +164,7 @@ func (q *Queries) CreateOidcUserInfo(ctx context.Context, arg CreateOidcUserInfo
 const deleteExpiredOidcCodes = `-- name: DeleteExpiredOidcCodes :many
 DELETE FROM "oidc_codes"
 WHERE "expires_at" < ?
-RETURNING sub, code_hash, scope, redirect_uri, client_id, expires_at, nonce, code_challenge
+RETURNING sub, code_hash, scope, redirect_uri, client_id, expires_at, nonce, code_challenge, code_challenge_method
 `
 
 func (q *Queries) DeleteExpiredOidcCodes(ctx context.Context, expiresAt int64) ([]OidcCode, error) {
@@ -181,6 +185,7 @@ func (q *Queries) DeleteExpiredOidcCodes(ctx context.Context, expiresAt int64) (
 			&i.ExpiresAt,
 			&i.Nonce,
 			&i.CodeChallenge,
+			&i.CodeChallengeMethod,
 		); err != nil {
 			return nil, err
 		}
@@ -291,7 +296,7 @@ func (q *Queries) DeleteOidcUserInfo(ctx context.Context, sub string) error {
 const getOidcCode = `-- name: GetOidcCode :one
 DELETE FROM "oidc_codes"
 WHERE "code_hash" = ?
-RETURNING sub, code_hash, scope, redirect_uri, client_id, expires_at, nonce, code_challenge
+RETURNING sub, code_hash, scope, redirect_uri, client_id, expires_at, nonce, code_challenge, code_challenge_method
 `
 
 func (q *Queries) GetOidcCode(ctx context.Context, codeHash string) (OidcCode, error) {
@@ -306,6 +311,7 @@ func (q *Queries) GetOidcCode(ctx context.Context, codeHash string) (OidcCode, e
 		&i.ExpiresAt,
 		&i.Nonce,
 		&i.CodeChallenge,
+		&i.CodeChallengeMethod,
 	)
 	return i, err
 }
@@ -313,7 +319,7 @@ func (q *Queries) GetOidcCode(ctx context.Context, codeHash string) (OidcCode, e
 const getOidcCodeBySub = `-- name: GetOidcCodeBySub :one
 DELETE FROM "oidc_codes"
 WHERE "sub" = ?
-RETURNING sub, code_hash, scope, redirect_uri, client_id, expires_at, nonce, code_challenge
+RETURNING sub, code_hash, scope, redirect_uri, client_id, expires_at, nonce, code_challenge, code_challenge_method
 `
 
 func (q *Queries) GetOidcCodeBySub(ctx context.Context, sub string) (OidcCode, error) {
@@ -328,12 +334,13 @@ func (q *Queries) GetOidcCodeBySub(ctx context.Context, sub string) (OidcCode, e
 		&i.ExpiresAt,
 		&i.Nonce,
 		&i.CodeChallenge,
+		&i.CodeChallengeMethod,
 	)
 	return i, err
 }
 
 const getOidcCodeBySubUnsafe = `-- name: GetOidcCodeBySubUnsafe :one
-SELECT sub, code_hash, scope, redirect_uri, client_id, expires_at, nonce, code_challenge FROM "oidc_codes"
+SELECT sub, code_hash, scope, redirect_uri, client_id, expires_at, nonce, code_challenge, code_challenge_method FROM "oidc_codes"
 WHERE "sub" = ?
 `
 
@@ -349,12 +356,13 @@ func (q *Queries) GetOidcCodeBySubUnsafe(ctx context.Context, sub string) (OidcC
 		&i.ExpiresAt,
 		&i.Nonce,
 		&i.CodeChallenge,
+		&i.CodeChallengeMethod,
 	)
 	return i, err
 }
 
 const getOidcCodeUnsafe = `-- name: GetOidcCodeUnsafe :one
-SELECT sub, code_hash, scope, redirect_uri, client_id, expires_at, nonce, code_challenge FROM "oidc_codes"
+SELECT sub, code_hash, scope, redirect_uri, client_id, expires_at, nonce, code_challenge, code_challenge_method FROM "oidc_codes"
 WHERE "code_hash" = ?
 `
 
@@ -370,6 +378,7 @@ func (q *Queries) GetOidcCodeUnsafe(ctx context.Context, codeHash string) (OidcC
 		&i.ExpiresAt,
 		&i.Nonce,
 		&i.CodeChallenge,
+		&i.CodeChallengeMethod,
 	)
 	return i, err
 }

internal/service/auth_service.go

@@ -28,26 +28,12 @@ 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
-	CallbackParams OAuthURLParams
+	State     string
+	Verifier  string
+	Token     *oauth2.Token
+	Service   *OAuthServiceImpl
+	ExpiresAt time.Time
 }
 
 type LdapGroupsCache struct {
@@ -612,7 +598,7 @@ func (auth *AuthService) IsBypassedIP(acls config.AppIP, ip string) bool {
 	return false
 }
 
-func (auth *AuthService) NewOAuthSession(serviceName string, params OAuthURLParams) (string, OAuthPendingSession, error) {
+func (auth *AuthService) NewOAuthSession(serviceName string) (string, OAuthPendingSession, error) {
 	auth.ensureOAuthSessionLimit()
 
 	service, ok := auth.oauthBroker.GetService(serviceName)
@@ -631,11 +617,10 @@ func (auth *AuthService) NewOAuthSession(serviceName string, params OAuthURLPara
 	verifier := service.NewRandom()
 
 	session := OAuthPendingSession{
-		State:          state,
-		Verifier:       verifier,
-		Service:        &service,
-		ExpiresAt:      time.Now().Add(1 * time.Hour),
-		CallbackParams: params,
+		State:     state,
+		Verifier:  verifier,
+		Service:   &service,
+		ExpiresAt: time.Now().Add(1 * time.Hour),
 	}
 
 	auth.oauthMutex.Lock()
@@ -646,7 +631,7 @@ func (auth *AuthService) NewOAuthSession(serviceName string, params OAuthURLPara
 }
 
 func (auth *AuthService) GetOAuthURL(sessionId string) (string, error) {
-	session, err := auth.GetOAuthPendingSession(sessionId)
+	session, err := auth.getOAuthPendingSession(sessionId)
 
 	if err != nil {
 		return "", err
@@ -656,7 +641,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
@@ -676,7 +661,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
@@ -696,7 +681,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
@@ -730,7 +715,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()

internal/service/oidc_service.go

@@ -297,7 +297,7 @@ func (service *OIDCService) ValidateAuthorizeParams(req AuthorizeRequest) error
 
 	// PKCE code challenge method if set
 	if req.CodeChallenge != "" && req.CodeChallengeMethod != "" {
-		if req.CodeChallengeMethod != "S256" && req.CodeChallengeMethod != "plain" {
+		if req.CodeChallengeMethod != "S256" || req.CodeChallenge == "plain" {
 			return errors.New("invalid_request")
 		}
 	}
@@ -329,8 +329,10 @@ func (service *OIDCService) StoreCode(c *gin.Context, sub string, code string, r
 	if req.CodeChallenge != "" {
 		if req.CodeChallengeMethod == "S256" {
 			entry.CodeChallenge = req.CodeChallenge
+			entry.CodeChallengeMethod = "S256"
 		} else {
 			entry.CodeChallenge = service.hashAndEncodePKCE(req.CodeChallenge)
+			entry.CodeChallengeMethod = "plain"
 			tlog.App.Warn().Msg("Received plain PKCE code challenge, it's recommended to use S256 for better security")
 		}
 	}
@@ -749,15 +751,19 @@ func (service *OIDCService) GetJWK() ([]byte, error) {
 	return jwk.Public().MarshalJSON()
 }
 
-func (service *OIDCService) ValidatePKCE(codeChallenge string, codeVerifier string) bool {
+func (service *OIDCService) ValidatePKCE(codeChallenge string, codeChallengeMethod string, codeVerifier string) bool {
 	if codeChallenge == "" {
 		return true
 	}
-	return codeChallenge == service.hashAndEncodePKCE(codeVerifier)
+	if codeChallengeMethod == "plain" {
+		// Code challenge is hashed and encoded in the database for security reasons
+		return codeChallenge == service.hashAndEncodePKCE(codeVerifier)
+	}
+	return codeChallenge == codeVerifier
 }
 
 func (service *OIDCService) hashAndEncodePKCE(codeVerifier string) string {
 	hasher := sha256.New()
 	hasher.Write([]byte(codeVerifier))
-	return base64.RawURLEncoding.EncodeToString(hasher.Sum(nil))
+	return base64.URLEncoding.EncodeToString(hasher.Sum(nil))
 }

internal/utils/tlog/log_wrapper.go

@@ -55,17 +55,6 @@ func NewSimpleLogger() *Logger {
 	})
 }
 
-func NewTestLogger() *Logger {
-	return NewLogger(config.LogConfig{
-		Level: "trace",
-		Streams: config.LogStreams{
-			HTTP:  config.LogStreamConfig{Enabled: true},
-			App:   config.LogStreamConfig{Enabled: true},
-			Audit: config.LogStreamConfig{Enabled: true},
-		},
-	})
-}
-
 func (l *Logger) Init() {
 	Audit = l.Audit
 	HTTP = l.HTTP

sql/oidc_queries.sql

@@ -7,9 +7,10 @@ INSERT INTO "oidc_codes" (
     "client_id",
     "expires_at",
     "nonce",
-    "code_challenge"
+    "code_challenge",
+    "code_challenge_method"
 ) VALUES (
-    ?, ?, ?, ?, ?, ?, ?, ?
+    ?, ?, ?, ?, ?, ?, ?, ?, ?
 )
 RETURNING *;
 

sql/oidc_schemas.sql

@@ -6,7 +6,8 @@ CREATE TABLE IF NOT EXISTS "oidc_codes" (
     "client_id" TEXT NOT NULL,
     "expires_at" INTEGER NOT NULL,
     "nonce" TEXT DEFAULT "",
-    "code_challenge" TEXT DEFAULT ""
+    "code_challenge" TEXT DEFAULT "",
+    "code_challenge_method" TEXT DEFAULT ""
 );
 
 CREATE TABLE IF NOT EXISTS "oidc_tokens" (

sqlc.yml

@@ -28,3 +28,5 @@ sql:
             go_type: "string"
           - column: "oidc_codes.code_challenge"
             go_type: "string"
+          - column: "oidc_codes.code_challenge_method"
+            go_type: "string"