feat: add pkce support to oidc server

Co-authored-by: GitHub <noreply@github.com>

README.md

@@ -58,7 +58,7 @@ Tinyauth is licensed under the GNU General Public License v3.0. TL;DR — You ma
 
 A big thank you to the following people for providing me with more coffee:
 
-<!-- sponsors --><a href="https://github.com/erwinkramer"><img src="https:&#x2F;&#x2F;github.com&#x2F;erwinkramer.png" width="64px" alt="User avatar: erwinkramer" /></a>&nbsp;&nbsp;<a href="https://github.com/nicotsx"><img src="https:&#x2F;&#x2F;github.com&#x2F;nicotsx.png" width="64px" alt="User avatar: nicotsx" /></a>&nbsp;&nbsp;<a href="https://github.com/SimpleHomelab"><img src="https:&#x2F;&#x2F;github.com&#x2F;SimpleHomelab.png" width="64px" alt="User avatar: SimpleHomelab" /></a>&nbsp;&nbsp;<a href="https://github.com/jmadden91"><img src="https:&#x2F;&#x2F;github.com&#x2F;jmadden91.png" width="64px" alt="User avatar: jmadden91" /></a>&nbsp;&nbsp;<a href="https://github.com/tribor"><img src="https:&#x2F;&#x2F;github.com&#x2F;tribor.png" width="64px" alt="User avatar: tribor" /></a>&nbsp;&nbsp;<a href="https://github.com/eliasbenb"><img src="https:&#x2F;&#x2F;github.com&#x2F;eliasbenb.png" width="64px" alt="User avatar: eliasbenb" /></a>&nbsp;&nbsp;<a href="https://github.com/afunworm"><img src="https:&#x2F;&#x2F;github.com&#x2F;afunworm.png" width="64px" alt="User avatar: afunworm" /></a>&nbsp;&nbsp;<a href="https://github.com/chip-well"><img src="https:&#x2F;&#x2F;github.com&#x2F;chip-well.png" width="64px" alt="User avatar: chip-well" /></a>&nbsp;&nbsp;<a href="https://github.com/Lancelot-Enguerrand"><img src="https:&#x2F;&#x2F;github.com&#x2F;Lancelot-Enguerrand.png" width="64px" alt="User avatar: Lancelot-Enguerrand" /></a>&nbsp;&nbsp;<a href="https://github.com/allgoewer"><img src="https:&#x2F;&#x2F;github.com&#x2F;allgoewer.png" width="64px" alt="User avatar: allgoewer" /></a>&nbsp;&nbsp;<a href="https://github.com/NEANC"><img src="https:&#x2F;&#x2F;github.com&#x2F;NEANC.png" width="64px" alt="User avatar: NEANC" /></a>&nbsp;&nbsp;<a href="https://github.com/ax-mad"><img src="https:&#x2F;&#x2F;github.com&#x2F;ax-mad.png" width="64px" alt="User avatar: ax-mad" /></a>&nbsp;&nbsp;<!-- sponsors -->
+<!-- sponsors --><a href="https://github.com/erwinkramer"><img src="https:&#x2F;&#x2F;github.com&#x2F;erwinkramer.png" width="64px" alt="User avatar: erwinkramer" /></a>&nbsp;&nbsp;<a href="https://github.com/nicotsx"><img src="https:&#x2F;&#x2F;github.com&#x2F;nicotsx.png" width="64px" alt="User avatar: nicotsx" /></a>&nbsp;&nbsp;<a href="https://github.com/SimpleHomelab"><img src="https:&#x2F;&#x2F;github.com&#x2F;SimpleHomelab.png" width="64px" alt="User avatar: SimpleHomelab" /></a>&nbsp;&nbsp;<a href="https://github.com/jmadden91"><img src="https:&#x2F;&#x2F;github.com&#x2F;jmadden91.png" width="64px" alt="User avatar: jmadden91" /></a>&nbsp;&nbsp;<a href="https://github.com/tribor"><img src="https:&#x2F;&#x2F;github.com&#x2F;tribor.png" width="64px" alt="User avatar: tribor" /></a>&nbsp;&nbsp;<a href="https://github.com/eliasbenb"><img src="https:&#x2F;&#x2F;github.com&#x2F;eliasbenb.png" width="64px" alt="User avatar: eliasbenb" /></a>&nbsp;&nbsp;<a href="https://github.com/afunworm"><img src="https:&#x2F;&#x2F;github.com&#x2F;afunworm.png" width="64px" alt="User avatar: afunworm" /></a>&nbsp;&nbsp;<a href="https://github.com/chip-well"><img src="https:&#x2F;&#x2F;github.com&#x2F;chip-well.png" width="64px" alt="User avatar: chip-well" /></a>&nbsp;&nbsp;<a href="https://github.com/Lancelot-Enguerrand"><img src="https:&#x2F;&#x2F;github.com&#x2F;Lancelot-Enguerrand.png" width="64px" alt="User avatar: Lancelot-Enguerrand" /></a>&nbsp;&nbsp;<a href="https://github.com/allgoewer"><img src="https:&#x2F;&#x2F;github.com&#x2F;allgoewer.png" width="64px" alt="User avatar: allgoewer" /></a>&nbsp;&nbsp;<a href="https://github.com/NEANC"><img src="https:&#x2F;&#x2F;github.com&#x2F;NEANC.png" width="64px" alt="User avatar: NEANC" /></a>&nbsp;&nbsp;<a href="https://github.com/ax-mad"><img src="https:&#x2F;&#x2F;github.com&#x2F;ax-mad.png" width="64px" alt="User avatar: ax-mad" /></a>&nbsp;&nbsp;<a href="https://github.com/stegratech"><img src="https:&#x2F;&#x2F;github.com&#x2F;stegratech.png" width="64px" alt="User avatar: stegratech" /></a>&nbsp;&nbsp;<!-- sponsors -->
 
 ## Acknowledgements
 

frontend/src/lib/hooks/oidc.ts

@@ -5,6 +5,8 @@ export type OIDCValues = {
   redirect_uri: string;
   state: string;
   nonce: string;
+  code_challenge: string;
+  code_challenge_method: string;
 };
 
 interface IuseOIDCParams {
@@ -14,7 +16,12 @@ interface IuseOIDCParams {
   missingParams: string[];
 }
 
-const optionalParams: string[] = ["state", "nonce"];
+const optionalParams: string[] = [
+  "state",
+  "nonce",
+  "code_challenge",
+  "code_challenge_method",
+];
 
 export function useOIDCParams(params: URLSearchParams): IuseOIDCParams {
   let compiled: string = "";
@@ -28,6 +35,8 @@ export function useOIDCParams(params: URLSearchParams): IuseOIDCParams {
     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)) {

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

@@ -0,0 +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

@@ -0,0 +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/oidc_controller.go

@@ -34,6 +34,7 @@ type TokenRequest struct {
 	RefreshToken string `form:"refresh_token" url:"refresh_token"`
 	ClientSecret string `form:"client_secret" url:"client_secret"`
 	ClientID     string `form:"client_id" url:"client_id"`
+	CodeVerifier string `form:"code_verifier" url:"code_verifier"`
 }
 
 type CallbackError struct {
@@ -308,6 +309,16 @@ func (controller *OIDCController) Token(c *gin.Context) {
 			return
 		}
 
+		ok := controller.oidc.ValidatePKCE(entry.CodeChallenge, entry.CodeChallengeMethod, req.CodeVerifier)
+
+		if !ok {
+			tlog.App.Warn().Msg("PKCE validation failed")
+			c.JSON(400, gin.H{
+				"error": "invalid_grant",
+			})
+			return
+		}
+
 		tokenRes, err := controller.oidc.GenerateAccessToken(c, client, entry)
 
 		if err != nil {

internal/controller/proxy_controller.go

@@ -323,11 +323,14 @@ func (controller *ProxyController) getHeader(c *gin.Context, header string) (str
 }
 
 func (controller *ProxyController) useBrowserResponse(proxyCtx ProxyContext) bool {
-	if !proxyCtx.IsBrowser {
+	// If it's nginx or envoy we need non-browser response
+	if proxyCtx.ProxyType == Nginx || proxyCtx.ProxyType == Envoy {
 		return false
 	}
 
-	if proxyCtx.ProxyType == Traefik {
+	// For other proxies (traefik or caddy) we can check
+	// the user agent to determine if it's a browser or not
+	if proxyCtx.IsBrowser {
 		return true
 	}
 

internal/controller/proxy_controller_test.go

@@ -190,6 +190,53 @@ func TestProxyController(t *testing.T) {
 				assert.Equal(t, 401, recorder.Code)
 			},
 		},
+		{
+			description: "Ensure forward auth with is browser false returns json",
+			middlewares: []gin.HandlerFunc{},
+			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
+				req := httptest.NewRequest("GET", "/api/auth/traefik", nil)
+				req.Header.Set("x-forwarded-host", "test.example.com")
+				req.Header.Set("x-forwarded-proto", "https")
+				req.Header.Set("x-forwarded-uri", "/")
+				router.ServeHTTP(recorder, req)
+
+				assert.Equal(t, 401, recorder.Code)
+				assert.Contains(t, recorder.Body.String(), `"status":401`)
+				assert.Contains(t, recorder.Body.String(), `"message":"Unauthorized"`)
+			},
+		},
+		{
+			description: "Ensure forward auth with caddy and browser user agent returns redirect",
+			middlewares: []gin.HandlerFunc{},
+			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
+				req := httptest.NewRequest("GET", "/api/auth/traefik", nil)
+				req.Header.Set("x-forwarded-host", "test.example.com")
+				req.Header.Set("x-forwarded-proto", "https")
+				req.Header.Set("x-forwarded-uri", "/")
+				req.Header.Set("user-agent", browserUserAgent)
+				router.ServeHTTP(recorder, req)
+
+				assert.Equal(t, 307, recorder.Code)
+				location := recorder.Header().Get("Location")
+				assert.Contains(t, location, "https://tinyauth.example.com/login?redirect_uri=")
+				assert.Contains(t, location, "https%3A%2F%2Ftest.example.com%2F")
+			},
+		},
+		{
+			description: "Ensure forward auth with caddy and non browser user agent returns json",
+			middlewares: []gin.HandlerFunc{},
+			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
+				req := httptest.NewRequest("GET", "/api/auth/traefik", nil)
+				req.Header.Set("x-forwarded-host", "test.example.com")
+				req.Header.Set("x-forwarded-proto", "https")
+				req.Header.Set("x-forwarded-uri", "/")
+				router.ServeHTTP(recorder, req)
+
+				assert.Equal(t, 401, recorder.Code)
+				assert.Contains(t, recorder.Body.String(), `"status":401`)
+				assert.Contains(t, recorder.Body.String(), `"message":"Unauthorized"`)
+			},
+		},
 		{
 			description: "Ensure normal authentication flow for forward auth",
 			middlewares: []gin.HandlerFunc{

internal/repository/models.go

@@ -5,13 +5,15 @@
 package repository
 
 type OidcCode struct {
-	Sub         string
-	CodeHash    string
-	Scope       string
-	RedirectURI string
-	ClientID    string
-	ExpiresAt   int64
-	Nonce       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

@@ -17,21 +17,25 @@ INSERT INTO "oidc_codes" (
     "redirect_uri",
     "client_id",
     "expires_at",
-    "nonce"
+    "nonce",
+    "code_challenge",
+    "code_challenge_method"
 ) VALUES (
-    ?, ?, ?, ?, ?, ?, ?
+    ?, ?, ?, ?, ?, ?, ?, ?, ?
 )
-RETURNING sub, code_hash, scope, redirect_uri, client_id, expires_at, nonce
+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
+	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) {
@@ -43,6 +47,8 @@ func (q *Queries) CreateOidcCode(ctx context.Context, arg CreateOidcCodeParams)
 		arg.ClientID,
 		arg.ExpiresAt,
 		arg.Nonce,
+		arg.CodeChallenge,
+		arg.CodeChallengeMethod,
 	)
 	var i OidcCode
 	err := row.Scan(
@@ -53,6 +59,8 @@ func (q *Queries) CreateOidcCode(ctx context.Context, arg CreateOidcCodeParams)
 		&i.ClientID,
 		&i.ExpiresAt,
 		&i.Nonce,
+		&i.CodeChallenge,
+		&i.CodeChallengeMethod,
 	)
 	return i, err
 }
@@ -156,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
+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) {
@@ -176,6 +184,8 @@ func (q *Queries) DeleteExpiredOidcCodes(ctx context.Context, expiresAt int64) (
 			&i.ClientID,
 			&i.ExpiresAt,
 			&i.Nonce,
+			&i.CodeChallenge,
+			&i.CodeChallengeMethod,
 		); err != nil {
 			return nil, err
 		}
@@ -286,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
+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) {
@@ -300,6 +310,8 @@ func (q *Queries) GetOidcCode(ctx context.Context, codeHash string) (OidcCode, e
 		&i.ClientID,
 		&i.ExpiresAt,
 		&i.Nonce,
+		&i.CodeChallenge,
+		&i.CodeChallengeMethod,
 	)
 	return i, err
 }
@@ -307,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
+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) {
@@ -321,12 +333,14 @@ func (q *Queries) GetOidcCodeBySub(ctx context.Context, sub string) (OidcCode, e
 		&i.ClientID,
 		&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 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" = ?
 `
 
@@ -341,12 +355,14 @@ func (q *Queries) GetOidcCodeBySubUnsafe(ctx context.Context, sub string) (OidcC
 		&i.ClientID,
 		&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 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" = ?
 `
 
@@ -361,6 +377,8 @@ func (q *Queries) GetOidcCodeUnsafe(ctx context.Context, codeHash string) (OidcC
 		&i.ClientID,
 		&i.ExpiresAt,
 		&i.Nonce,
+		&i.CodeChallenge,
+		&i.CodeChallengeMethod,
 	)
 	return i, err
 }

internal/service/oidc_service.go

@@ -75,12 +75,14 @@ type TokenResponse struct {
 }
 
 type AuthorizeRequest struct {
-	Scope        string `json:"scope" binding:"required"`
-	ResponseType string `json:"response_type" binding:"required"`
-	ClientID     string `json:"client_id" binding:"required"`
-	RedirectURI  string `json:"redirect_uri" binding:"required"`
-	State        string `json:"state"`
-	Nonce        string `json:"nonce"`
+	Scope               string `json:"scope" binding:"required"`
+	ResponseType        string `json:"response_type" binding:"required"`
+	ClientID            string `json:"client_id" binding:"required"`
+	RedirectURI         string `json:"redirect_uri" binding:"required"`
+	State               string `json:"state"`
+	Nonce               string `json:"nonce"`
+	CodeChallenge       string `json:"code_challenge"`
+	CodeChallengeMethod string `json:"code_challenge_method"`
 }
 
 type OIDCServiceConfig struct {
@@ -293,6 +295,13 @@ func (service *OIDCService) ValidateAuthorizeParams(req AuthorizeRequest) error
 		return errors.New("invalid_request_uri")
 	}
 
+	// PKCE code challenge method if set
+	if req.CodeChallenge != "" && req.CodeChallengeMethod != "" {
+		if req.CodeChallengeMethod != "S256" || req.CodeChallenge == "plain" {
+			return errors.New("invalid_request")
+		}
+	}
+
 	return nil
 }
 
@@ -306,8 +315,7 @@ func (service *OIDCService) StoreCode(c *gin.Context, sub string, code string, r
 	// Fixed 10 minutes
 	expiresAt := time.Now().Add(time.Minute * time.Duration(10)).Unix()
 
-	// Insert the code into the database
-	_, err := service.queries.CreateOidcCode(c, repository.CreateOidcCodeParams{
+	entry := repository.CreateOidcCodeParams{
 		Sub:      sub,
 		CodeHash: service.Hash(code),
 		// Here it's safe to split and trust the output since, we validated the scopes before
@@ -316,7 +324,21 @@ func (service *OIDCService) StoreCode(c *gin.Context, sub string, code string, r
 		ClientID:    req.ClientID,
 		ExpiresAt:   expiresAt,
 		Nonce:       req.Nonce,
-	})
+	}
+
+	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")
+		}
+	}
+
+	// Insert the code into the database
+	_, err := service.queries.CreateOidcCode(c, entry)
 
 	return err
 }
@@ -728,3 +750,20 @@ func (service *OIDCService) GetJWK() ([]byte, error) {
 
 	return jwk.Public().MarshalJSON()
 }
+
+func (service *OIDCService) ValidatePKCE(codeChallenge string, codeChallengeMethod string, codeVerifier string) bool {
+	if codeChallenge == "" {
+		return true
+	}
+	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.URLEncoding.EncodeToString(hasher.Sum(nil))
+}

sql/oidc_queries.sql

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

sql/oidc_schemas.sql

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

sqlc.yml

@@ -26,3 +26,7 @@ sql:
             go_type: "string"
           - column: "oidc_tokens.nonce"
             go_type: "string"
+          - column: "oidc_codes.code_challenge"
+            go_type: "string"
+          - column: "oidc_codes.code_challenge_method"
+            go_type: "string"