refactor(oauth): use x/oauth2 S256ChallengeFromVerifier and tighten tests

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

Author: silverwind

models/auth/oauth2.go

@@ -5,10 +5,8 @@ package auth
 
 import (
 	"context"
-	"crypto/sha256"
 	"crypto/subtle"
 	"encoding/base32"
-	"encoding/base64"
 	"errors"
 	"fmt"
 	"net"
@@ -25,14 +23,18 @@ import (
 
 	uuid "github.com/google/uuid"
 	"golang.org/x/crypto/bcrypt"
+	"golang.org/x/oauth2"
 	"xorm.io/builder"
 	"xorm.io/xorm"
 )
 
 // Authorization codes should expire within 10 minutes per https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2
 const oauth2AuthorizationCodeValidity = 10 * time.Minute
 
-var ErrOAuth2AuthorizationCodeInvalidated = errors.New("oauth2 authorization code already invalidated")
+var (
+	ErrOAuth2AuthorizationCodeInvalidated = errors.New("oauth2 authorization code already invalidated")
+	ErrOAuth2GrantStaleCounter            = errors.New("oauth2 grant state changed during token refresh")
+)
 
 // OAuth2Application represents an OAuth2 client (RFC 6749)
 type OAuth2Application struct {
@@ -442,8 +444,7 @@ func (code *OAuth2AuthorizationCode) requiresCodeVerifier() bool {
 func deriveCodeChallenge(method, verifier string) (string, bool) {
 	switch method {
 	case "S256":
-		hash := sha256.Sum256([]byte(verifier))
-		return base64.RawURLEncoding.EncodeToString(hash[:]), true
+		return oauth2.S256ChallengeFromVerifier(verifier), true
 	case "plain":
 		return verifier, true
 	default:
@@ -537,7 +538,7 @@ func (grant *OAuth2Grant) IncreaseCounter(ctx context.Context) error {
 		return err
 	}
 	if affected == 0 {
-		return errors.New("grant state changed during token refresh")
+		return ErrOAuth2GrantStaleCounter
 	}
 	grant.Counter++
 	return nil

models/auth/oauth2_test.go

@@ -4,8 +4,6 @@
 package auth_test
 
 import (
-	"crypto/sha256"
-	"encoding/base64"
 	"testing"
 	"time"
 
@@ -15,6 +13,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/oauth2"
 )
 
 func TestOAuth2AuthorizationCode(t *testing.T) {
@@ -143,6 +142,12 @@ func TestOAuth2Application_ContainsRedirectURI_ASCIIOnlyNormalization(t *testing
 			redirectURI: "https://signİn.example.test/callback",
 			allowed:     false,
 		},
+		{
+			name:        "loopback-strips-port",
+			registered:  []string{"http://127.0.0.1/callback"},
+			redirectURI: "http://127.0.0.1:12345/callback",
+			allowed:     true,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -237,8 +242,7 @@ func TestOAuth2Grant_IncreaseCounterRejectsStaleCounter(t *testing.T) {
 
 	assert.NoError(t, grant.IncreaseCounter(t.Context()))
 	err := stale.IncreaseCounter(t.Context())
-	assert.Error(t, err)
-	assert.Contains(t, err.Error(), "grant state changed during token refresh")
+	assert.ErrorIs(t, err, auth_model.ErrOAuth2GrantStaleCounter)
 }
 
 func TestOAuth2Grant_ScopeContains(t *testing.T) {
@@ -285,112 +289,36 @@ func TestRevokeOAuth2Grant(t *testing.T) {
 //////////////////// Authorization Code
 
 func TestOAuth2AuthorizationCode_ValidateCodeChallenge(t *testing.T) {
-	s256Verifier := "phase4-s256-verifier"
-	s256Sum := sha256.Sum256([]byte(s256Verifier))
-	missingVerifierSeed := "phase4-verifier-without-body"
-	missingVerifierSum := sha256.Sum256([]byte(missingVerifierSeed))
+	s256Verifier := "s256-verifier"
+	s256Challenge := oauth2.S256ChallengeFromVerifier(s256Verifier)
+	missingVerifierChallenge := oauth2.S256ChallengeFromVerifier("verifier-not-supplied")
 
 	testCases := []struct {
-		name     string
-		code     *auth_model.OAuth2AuthorizationCode
-		verifier string
-		valid    bool
+		name      string
+		method    string
+		challenge string
+		verifier  string
+		valid     bool
 	}{
-		{
-			name: "plain-success",
-			code: &auth_model.OAuth2AuthorizationCode{
-				CodeChallengeMethod: "plain",
-				CodeChallenge:       "plain-phase4-secret",
-			},
-			verifier: "plain-phase4-secret",
-			valid:    true,
-		},
-		{
-			name: "plain-failure",
-			code: &auth_model.OAuth2AuthorizationCode{
-				CodeChallengeMethod: "plain",
-				CodeChallenge:       "plain-phase4-secret",
-			},
-			verifier: "ierwgjoergjio",
-			valid:    false,
-		},
-		{
-			name: "s256-success",
-			code: &auth_model.OAuth2AuthorizationCode{
-				CodeChallengeMethod: "S256",
-				CodeChallenge:       base64.RawURLEncoding.EncodeToString(s256Sum[:]),
-			},
-			verifier: s256Verifier,
-			valid:    true,
-		},
-		{
-			name: "s256-failure",
-			code: &auth_model.OAuth2AuthorizationCode{
-				CodeChallengeMethod: "S256",
-				CodeChallenge:       base64.RawURLEncoding.EncodeToString(s256Sum[:]),
-			},
-			verifier: "wiogjerogorewngoenrgoiuenorg",
-			valid:    false,
-		},
-		{
-			name: "unsupported-method",
-			code: &auth_model.OAuth2AuthorizationCode{
-				CodeChallengeMethod: "monkey",
-				CodeChallenge:       "foiwgjioriogeiogjerger",
-			},
-			verifier: "foiwgjioriogeiogjerger",
-			valid:    false,
-		},
-		{
-			name: "no-pkce-configured",
-			code: &auth_model.OAuth2AuthorizationCode{
-				CodeChallengeMethod: "",
-				CodeChallenge:       "",
-			},
-			verifier: "",
-			valid:    true,
-		},
-		{
-			name: "s256-missing-verifier",
-			code: &auth_model.OAuth2AuthorizationCode{
-				CodeChallengeMethod: "S256",
-				CodeChallenge:       base64.RawURLEncoding.EncodeToString(missingVerifierSum[:]),
-			},
-			verifier: "",
-			valid:    false,
-		},
-		{
-			name: "plain-missing-verifier",
-			code: &auth_model.OAuth2AuthorizationCode{
-				CodeChallengeMethod: "plain",
-				CodeChallenge:       "plain-phase4-secret",
-			},
-			verifier: "",
-			valid:    false,
-		},
-		{
-			name: "missing-method-with-challenge",
-			code: &auth_model.OAuth2AuthorizationCode{
-				CodeChallengeMethod: "",
-				CodeChallenge:       "foierjiogerogerg",
-			},
-			verifier: "",
-			valid:    false,
-		},
-		{
-			name: "missing-method-rejects-even-matching-verifier",
-			code: &auth_model.OAuth2AuthorizationCode{
-				CodeChallengeMethod: "",
-				CodeChallenge:       "foierjiogerogerg",
-			},
-			verifier: "foierjiogerogerg",
-			valid:    false,
-		},
+		{"plain-success", "plain", "plain-secret", "plain-secret", true},
+		{"plain-failure", "plain", "plain-secret", "ierwgjoergjio", false},
+		{"s256-success", "S256", s256Challenge, s256Verifier, true},
+		{"s256-failure", "S256", s256Challenge, "wiogjerogorewngoenrgoiuenorg", false},
+		{"unsupported-method", "monkey", "foiwgjioriogeiogjerger", "foiwgjioriogeiogjerger", false},
+		{"no-pkce-configured", "", "", "", true},
+		{"s256-missing-verifier", "S256", missingVerifierChallenge, "", false},
+		{"plain-missing-verifier", "plain", "plain-secret", "", false},
+		{"missing-method-with-challenge", "", "foierjiogerogerg", "", false},
+		{"missing-method-rejects-even-matching-verifier", "", "foierjiogerogerg", "foierjiogerogerg", false},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			assert.Equal(t, tc.valid, tc.code.ValidateCodeChallenge(tc.verifier))
+			code := &auth_model.OAuth2AuthorizationCode{
+				CodeChallengeMethod: tc.method,
+				CodeChallenge:       tc.challenge,
+			}
+			assert.Equal(t, tc.valid, code.ValidateCodeChallenge(tc.verifier))
 		})
 	}
 }