Fix OAuth2 authorization code expiry and reuse handling (#36797)

models/auth/oauth2.go

@@ -14,6 +14,7 @@ import (
 	"net/url"
 	"slices"
 	"strings"
+	"time"
 
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/modules/container"
@@ -27,6 +28,11 @@ import (
 	"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")
+
 // OAuth2Application represents an OAuth2 client (RFC 6749)
 type OAuth2Application struct {
 	ID           int64 `xorm:"pk autoincr"`
@@ -386,6 +392,14 @@ func (code *OAuth2AuthorizationCode) TableName() string {
 	return "oauth2_authorization_code"
 }
 
+// IsExpired reports whether the authorization code is expired.
+func (code *OAuth2AuthorizationCode) IsExpired() bool {
+	if code.ValidUntil.IsZero() {
+		return true
+	}
+	return code.ValidUntil <= timeutil.TimeStampNow()
+}
+
 // GenerateRedirectURI generates a redirect URI for a successful authorization request. State will be used if not empty.
 func (code *OAuth2AuthorizationCode) GenerateRedirectURI(state string) (*url.URL, error) {
 	redirect, err := url.Parse(code.RedirectURI)
@@ -403,8 +417,14 @@ func (code *OAuth2AuthorizationCode) GenerateRedirectURI(state string) (*url.URL
 
 // Invalidate deletes the auth code from the database to invalidate this code
 func (code *OAuth2AuthorizationCode) Invalidate(ctx context.Context) error {
-	_, err := db.GetEngine(ctx).ID(code.ID).NoAutoCondition().Delete(code)
-	return err
+	affected, err := db.GetEngine(ctx).ID(code.ID).NoAutoCondition().Delete(code)
+	if err != nil {
+		return err
+	}
+	if affected == 0 {
+		return ErrOAuth2AuthorizationCodeInvalidated
+	}
+	return nil
 }
 
 // ValidateCodeChallenge validates the given verifier against the saved code challenge. This is part of the PKCE implementation.
@@ -472,13 +492,15 @@ func (grant *OAuth2Grant) GenerateNewAuthorizationCode(ctx context.Context, redi
 	// for code scanners to grab sensitive tokens.
 	codeSecret := "gta_" + base32Lower.EncodeToString(rBytes)
 
+	validUntil := time.Now().Add(oauth2AuthorizationCodeValidity)
 	code = &OAuth2AuthorizationCode{
 		Grant:               grant,
 		GrantID:             grant.ID,
 		RedirectURI:         redirectURI,
 		Code:                codeSecret,
 		CodeChallenge:       codeChallenge,
 		CodeChallengeMethod: codeChallengeMethod,
+		ValidUntil:          timeutil.TimeStamp(validUntil.Unix()),
 	}
 	if err := db.Insert(ctx, code); err != nil {
 		return nil, err

models/auth/oauth2_test.go

@@ -5,13 +5,45 @@ package auth_test
 
 import (
 	"testing"
+	"time"
 
 	auth_model "code.gitea.io/gitea/models/auth"
 	"code.gitea.io/gitea/models/unittest"
+	"code.gitea.io/gitea/modules/timeutil"
 
 	"github.com/stretchr/testify/assert"
 )
 
+func TestOAuth2AuthorizationCodeValidity(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	t.Run("GenerateSetsValidUntil", func(t *testing.T) {
+		grant := unittest.AssertExistsAndLoadBean(t, &auth_model.OAuth2Grant{ID: 1})
+		expectedValidUntil := timeutil.TimeStamp(time.Now().Unix() + 600)
+		code, err := grant.GenerateNewAuthorizationCode(t.Context(), "http://127.0.0.1/", "", "")
+		assert.NoError(t, err)
+		assert.Equal(t, expectedValidUntil, code.ValidUntil)
+		assert.False(t, code.IsExpired())
+		assert.NoError(t, code.Invalidate(t.Context()))
+	})
+
+	t.Run("Expired", func(t *testing.T) {
+		defer timeutil.MockSet(time.Unix(2, 0).UTC())()
+
+		code := &auth_model.OAuth2AuthorizationCode{ValidUntil: timeutil.TimeStamp(1)}
+		assert.True(t, code.IsExpired())
+	})
+
+	t.Run("InvalidateTwice", func(t *testing.T) {
+		code, err := auth_model.GetOAuth2AuthorizationByCode(t.Context(), "authcode")
+		assert.NoError(t, err)
+		if assert.NotNil(t, code) {
+			assert.NoError(t, code.Invalidate(t.Context()))
+			assert.ErrorIs(t, code.Invalidate(t.Context()), auth_model.ErrOAuth2AuthorizationCodeInvalidated)
+		}
+	})
+}
+
 func TestOAuth2Application_GenerateClientSecret(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	app := unittest.AssertExistsAndLoadBean(t, &auth_model.OAuth2Application{ID: 1})

routers/web/auth/oauth2_provider.go

@@ -4,6 +4,7 @@
 package auth
 
 import (
+	"errors"
 	"fmt"
 	"html"
 	"html/template"
@@ -613,6 +614,14 @@ func handleAuthorizationCode(ctx *context.Context, form forms.AccessTokenForm, s
 		})
 		return
 	}
+	if authorizationCode.IsExpired() {
+		_ = authorizationCode.Invalidate(ctx)
+		handleAccessTokenError(ctx, oauth2_provider.AccessTokenError{
+			ErrorCode:        oauth2_provider.AccessTokenErrorCodeInvalidGrant,
+			ErrorDescription: "authorization code expired",
+		})
+		return
+	}
 	// check if code verifier authorizes the client, PKCE support
 	if !authorizationCode.ValidateCodeChallenge(form.CodeVerifier) {
 		handleAccessTokenError(ctx, oauth2_provider.AccessTokenError{
@@ -631,9 +640,15 @@ func handleAuthorizationCode(ctx *context.Context, form forms.AccessTokenForm, s
 	}
 	// remove token from database to deny duplicate usage
 	if err := authorizationCode.Invalidate(ctx); err != nil {
+		errDescription := "cannot process your request"
+		errCode := oauth2_provider.AccessTokenErrorCodeInvalidRequest
+		if errors.Is(err, auth.ErrOAuth2AuthorizationCodeInvalidated) {
+			errDescription = "authorization code already used"
+			errCode = oauth2_provider.AccessTokenErrorCodeInvalidGrant
+		}
 		handleAccessTokenError(ctx, oauth2_provider.AccessTokenError{
-			ErrorCode:        oauth2_provider.AccessTokenErrorCodeInvalidRequest,
-			ErrorDescription: "cannot proceed your request",
+			ErrorCode:        errCode,
+			ErrorDescription: errDescription,
 		})
 		return
 	}