fix(oauth): RFC-compliant PKCE, redirect_uri validation, and security hardening

Makefile

@@ -42,7 +42,52 @@ trivy-scan:
 clean:
 	rm -rf build
 dev:
-	go run main.go --database-type=sqlite --database-url=test.db --jwt-type=HS256 --jwt-secret=test --admin-secret=admin --client-id=123456 --client-secret=secret
+	@PRIVATE_KEY=$$(printf '%s\n' \
+		"-----BEGIN RSA PRIVATE KEY-----" \
+		"MIIEowIBAAKCAQEA5dC50fVvQIDm66bBYW+qI+MypP9Pv9SMoHIz9cpcOj9sNhXI" \
+		"HTllAM5dhi/+HIaJdPugVQt1rlTJVSFR+ynSmwa89RPHs0o7CBytskGaaf2RJ6zD" \
+		"AY3TXKQQVAT3Qvb6ZOQh3+Hh8EOguqdE2iORo9s0KMk7tqS+/y4H3qrC6ngyt2QT" \
+		"6VqWbs92N/aO0p/FaL/Q7rGZ+9hTlu3L/T70r3nyeA636kM48XUSqcjDrs4/E+Vx" \
+		"XL2Y9Wo4kuaDmMPvMkdl6/wGOwAuIuHpmdfGh0hyLMdgpMqvFyEHuagCy+yFV6ES" \
+		"gVi2rOp1g28iISbjpMTkNikbCBuL/TeaSdmEPwIDAQABAoIBABDpeCXmI2Ps+HwN" \
+		"VKbNzQirZA3gsfw3xoovd/kVsA14nwcojtuZCStILyQRrRK+qH1A39l8/ecwhckW" \
+		"KiPLE0dloAstrkut4e6PZGLn/AE3xUeqVDFtlUkjKQZzKm+1oCiY4eX0B/335BXy" \
+		"+u34ocjxTS3Wh+aWyhgaSZROdF3vtcH0PHDroDd8oT/H0xN8fX/T1JozLN3jbxXz" \
+		"G4KznKNmx74SrV/y6/wmmQqIsghJBNRZGbg5bn/xcExkcRhWQ3Eka8yAlu31XBQV" \
+		"G5AtHEVO8lEi+a2PCA7t1xquw/8b48lc226aaN0pjaySNtyLB7EaKUxBDB2aGx3s" \
+		"nVIuOsUCgYEA5xWJ67BKy6MyHphhltTLvre/AGXKDhoV3IR3Hm1cu/iXCT/oOFje" \
+		"SYAAqqHXKEB+HZ4xKk7PnBzXkyLXqkCbEIBsv+GZpmboqZrPMfRgS6QcPWWMV9pZ" \
+		"f1h68hWpXfyY+yEhAvPKFFhjAt0f5uMiRaAUskZJTFiRJqxz+z0AdD0CgYEA/pgq" \
+		"Tk8WMKJBTic3m224FrR4qDok8tQp8FCerS7T5fFnSXZx64ucREn+Wm9ym5UTtbQz" \
+		"pne4diIsNIIlVFOjOV7jHjzBS5oly2N/2AT5+ST7O4GZfh/I9VKQWuIh+i3p3s9x" \
+		"7PSIlaql9ddV582gCMiL7/QDkHDFBVEz+vq31isCgYB2UoQFZ4ZU0OI34kSN67XL" \
+		"mOA2/ue/4sFw4W7w6ISERxxnAw8P0wk2z1EIDchSdvtchQSdqi8Ju4bycvPE3EHJ" \
+		"6EhG0+hN2QGm3nrbFEs+T/CZy2ZaEZaj6xVA4bCQTGe0ptj1XwkI89z2uWy9V23U" \
+		"Asy2H+EmM29XQxQ7/5c87QKBgQCUO+i1+5pB6tb3OCJKXxHGNoHiASiuMhXRFD+v" \
+		"OgqqYWnv/gTKTllH8YUlBqrGJ4B4VVmVXTOLpM30LKqrdJ8esj6uxlUNPc0vpNk0" \
+		"34DkLUISHZ1PMBaDr/TY1b1OuxjmYAZHHwG/ksJaZ2xfMPwy4QGJTpwcp2wvcl4/" \
+		"jWcoTQKBgALNq5XD/ufvZO2YQq9phF0EQza9zr45zSENF0cOsyVcEG4wfFv4Sg03" \
+		"JjTG57oYDCWeLrFCRQpysFi1pDUUCQ1Z/Kf9xKZ/OoE1mXGCKGilBGUijQasuO5Q" \
+		"GU+S3Xlk6TWCb2jTgc9UTjlp1FOgQSad4M6TW8vXGkSMODEj5g0S" \
+		"-----END RSA PRIVATE KEY-----") && \
+	PUBLIC_KEY=$$(printf '%s\n' \
+		"-----BEGIN RSA PUBLIC KEY-----" \
+		"MIIBCgKCAQEA5dC50fVvQIDm66bBYW+qI+MypP9Pv9SMoHIz9cpcOj9sNhXIHTll" \
+		"AM5dhi/+HIaJdPugVQt1rlTJVSFR+ynSmwa89RPHs0o7CBytskGaaf2RJ6zDAY3T" \
+		"XKQQVAT3Qvb6ZOQh3+Hh8EOguqdE2iORo9s0KMk7tqS+/y4H3qrC6ngyt2QT6VqW" \
+		"bs92N/aO0p/FaL/Q7rGZ+9hTlu3L/T70r3nyeA636kM48XUSqcjDrs4/E+VxXL2Y" \
+		"9Wo4kuaDmMPvMkdl6/wGOwAuIuHpmdfGh0hyLMdgpMqvFyEHuagCy+yFV6ESgVi2" \
+		"rOp1g28iISbjpMTkNikbCBuL/TeaSdmEPwIDAQAB" \
+		"-----END RSA PUBLIC KEY-----") && \
+	go run main.go \
+		--database-type=sqlite \
+		--database-url=test.db \
+		--jwt-type=RS256 \
+		--jwt-private-key="$$PRIVATE_KEY" \
+		--jwt-public-key="$$PUBLIC_KEY" \
+		--admin-secret=admin \
+		--client-id=kbyuFDidLLm280LIwVFiazOqjO3ty8KH \
+		--client-secret=60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa
 
 test:
 	go clean --testcache && TEST_DBS="sqlite" $(GO_TEST_ALL)

docs/oauth2-oidc-endpoints.md

@@ -69,8 +69,8 @@ Initiates the OAuth 2.0 authorization flow. Supports Authorization Code (with PK
 | `redirect_uri` | No | Where to redirect after auth (defaults to `/app`) |
 | `scope` | No | Space-separated scopes (default: `openid profile email`) |
 | `response_mode` | No | `query`, `fragment`, `form_post`, or `web_message` |
-| `code_challenge` | Required for `code` | PKCE S256 challenge: `BASE64URL(SHA256(code_verifier))` |
-| `code_challenge_method` | No | Only `S256` is supported (defaults to `S256`) |
+| `code_challenge` | Recommended | PKCE challenge. Required for public clients; confidential clients may use `client_secret` instead |
+| `code_challenge_method` | No | `S256` (default) or `plain` per RFC 7636 |
 | `nonce` | Recommended | Binds ID token to session; REQUIRED for implicit flows per OIDC |
 | `screen_hint` | No | Set to `signup` to show the signup page |
 
@@ -392,7 +392,7 @@ grant_type=authorization_code&code=AUTH_CODE&code_verifier=CODE_VERIFIER&client_
 | Standard | Status | Notes |
 |----------|--------|-------|
 | RFC 6749 (OAuth 2.0) | Implemented | Authorization Code + Refresh Token grants |
-| RFC 7636 (PKCE) | Implemented | S256 method required |
+| RFC 7636 (PKCE) | Implemented | S256 (default) and plain methods; optional for confidential clients |
 | RFC 7009 (Token Revocation) | Implemented | Returns 200 for invalid tokens |
 | RFC 6750 (Bearer Token) | Implemented | WWW-Authenticate on 401 |
 | OIDC Core 1.0 | Implemented | ID tokens, UserInfo, nonce |

internal/graphql/login.go

@@ -376,6 +376,8 @@ func (g *graphqlProvider) Login(ctx context.Context, params *model.LoginRequest)
 	code := ""
 	codeChallenge := ""
 	nonce := ""
+	oidcNonce := ""
+	authorizeRedirectURI := ""
 	if params.State != nil {
 		// Get state from store
 		authorizeState, _ := g.MemoryStoreProvider.GetState(refs.StringValue(params.State))
@@ -384,23 +386,30 @@ func (g *graphqlProvider) Login(ctx context.Context, params *model.LoginRequest)
 			if len(authorizeStateSplit) > 1 {
 				code = authorizeStateSplit[0]
 				codeChallenge = authorizeStateSplit[1]
+				if len(authorizeStateSplit) > 2 {
+					oidcNonce = authorizeStateSplit[2]
+				}
+				// RFC 6749 §4.1.3: redirect_uri from /authorize for validation at /oauth/token
+				if len(authorizeStateSplit) > 3 {
+					authorizeRedirectURI = authorizeStateSplit[3]
+				}
 			} else {
 				nonce = authorizeState
 			}
-			go g.MemoryStoreProvider.RemoveState(refs.StringValue(params.State))
+			g.MemoryStoreProvider.RemoveState(refs.StringValue(params.State))
 		}
 	}
 
 	if nonce == "" {
 		nonce = uuid.New().String()
 	}
 	hostname := parsers.GetHost(gc)
-	// gc, user, roles, scope, constants.AuthRecipeMethodBasicAuth, nonce, code
 	authToken, err := g.TokenProvider.CreateAuthToken(gc, &token.AuthTokenConfig{
 		User:        user,
 		Roles:       roles,
 		Scope:       scope,
 		Nonce:       nonce,
+		OIDCNonce:   oidcNonce,
 		Code:        code,
 		LoginMethod: constants.AuthRecipeMethodBasicAuth,
 		HostName:    hostname,
@@ -410,10 +419,9 @@ func (g *graphqlProvider) Login(ctx context.Context, params *model.LoginRequest)
 		return nil, err
 	}
 
-	// TODO add to other login options as well
 	// Code challenge could be optional if PKCE flow is not used
 	if code != "" {
-		if err := g.MemoryStoreProvider.SetState(code, codeChallenge+"@@"+authToken.FingerPrintHash); err != nil {
+		if err := g.MemoryStoreProvider.SetState(code, codeChallenge+"@@"+authToken.FingerPrintHash+"@@"+oidcNonce+"@@"+authorizeRedirectURI); err != nil {
 			log.Debug().Msg("Failed to set state")
 			return nil, err
 		}

internal/graphql/signup.go

@@ -287,6 +287,8 @@ func (g *graphqlProvider) SignUp(ctx context.Context, params *model.SignUpReques
 	code := ""
 	codeChallenge := ""
 	nonce := ""
+	oidcNonce := ""
+	authorizeRedirectURI := ""
 	if params.State != nil {
 		// Get state from store
 		authorizeState, _ := g.MemoryStoreProvider.GetState(refs.StringValue(params.State))
@@ -295,10 +297,16 @@ func (g *graphqlProvider) SignUp(ctx context.Context, params *model.SignUpReques
 			if len(authorizeStateSplit) > 1 {
 				code = authorizeStateSplit[0]
 				codeChallenge = authorizeStateSplit[1]
+				if len(authorizeStateSplit) > 2 {
+					oidcNonce = authorizeStateSplit[2]
+				}
+				if len(authorizeStateSplit) > 3 {
+					authorizeRedirectURI = authorizeStateSplit[3]
+				}
 			} else {
 				nonce = authorizeState
 			}
-			go g.MemoryStoreProvider.RemoveState(refs.StringValue(params.State))
+			g.MemoryStoreProvider.RemoveState(refs.StringValue(params.State))
 		}
 	}
 
@@ -310,6 +318,7 @@ func (g *graphqlProvider) SignUp(ctx context.Context, params *model.SignUpReques
 		Roles:       roles,
 		Scope:       scope,
 		Nonce:       nonce,
+		OIDCNonce:   oidcNonce,
 		Code:        code,
 		LoginMethod: constants.AuthRecipeMethodBasicAuth,
 		HostName:    hostname,
@@ -321,7 +330,7 @@ func (g *graphqlProvider) SignUp(ctx context.Context, params *model.SignUpReques
 
 	// Code challenge could be optional if PKCE flow is not used
 	if code != "" {
-		if err := g.MemoryStoreProvider.SetState(code, codeChallenge+"@@"+authToken.FingerPrintHash); err != nil {
+		if err := g.MemoryStoreProvider.SetState(code, codeChallenge+"@@"+authToken.FingerPrintHash+"@@"+oidcNonce+"@@"+authorizeRedirectURI); err != nil {
 			log.Debug().Err(err).Msg("SetState failed")
 			return nil, err
 		}

internal/graphql/verify_otp.go

@@ -156,6 +156,8 @@ func (g *graphqlProvider) VerifyOTP(ctx context.Context, params *model.VerifyOTP
 	code := ""
 	codeChallenge := ""
 	nonce := ""
+	oidcNonce := ""
+	authorizeRedirectURI := ""
 	if params.State != nil {
 		// Get state from store
 		authorizeState, _ := g.MemoryStoreProvider.GetState(refs.StringValue(params.State))
@@ -164,23 +166,29 @@ func (g *graphqlProvider) VerifyOTP(ctx context.Context, params *model.VerifyOTP
 			if len(authorizeStateSplit) > 1 {
 				code = authorizeStateSplit[0]
 				codeChallenge = authorizeStateSplit[1]
+				if len(authorizeStateSplit) > 2 {
+					oidcNonce = authorizeStateSplit[2]
+				}
+				if len(authorizeStateSplit) > 3 {
+					authorizeRedirectURI = authorizeStateSplit[3]
+				}
 			} else {
 				nonce = authorizeState
 			}
-			go g.MemoryStoreProvider.RemoveState(refs.StringValue(params.State))
+			g.MemoryStoreProvider.RemoveState(refs.StringValue(params.State))
 		}
 	}
 	if nonce == "" {
 		nonce = uuid.New().String()
 	}
 	hostname := parsers.GetHost(gc)
-	// user, roles, scope, loginMethod, nonce, code
 	authToken, err := g.TokenProvider.CreateAuthToken(gc, &token.AuthTokenConfig{
 		User:        user,
 		Roles:       roles,
 		Scope:       scope,
 		LoginMethod: loginMethod,
 		Nonce:       nonce,
+		OIDCNonce:   oidcNonce,
 		Code:        code,
 		HostName:    hostname,
 	})
@@ -191,7 +199,7 @@ func (g *graphqlProvider) VerifyOTP(ctx context.Context, params *model.VerifyOTP
 
 	// Code challenge could be optional if PKCE flow is not used
 	if code != "" {
-		if err := g.MemoryStoreProvider.SetState(code, codeChallenge+"@@"+authToken.FingerPrintHash); err != nil {
+		if err := g.MemoryStoreProvider.SetState(code, codeChallenge+"@@"+authToken.FingerPrintHash+"@@"+oidcNonce+"@@"+authorizeRedirectURI); err != nil {
 			log.Debug().Err(err).Msg("Failed to set state")
 			return nil, err
 		}