From 58220de197c5b7322416ad32a9932478717d83fc Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 1 Mar 2026 16:53:47 -0800 Subject: [PATCH 1/6] Fix oauth2 expired --- models/auth/oauth2.go | 25 +++++++++++++++++++-- models/auth/oauth2_test.go | 34 +++++++++++++++++++++++++++++ routers/web/auth/oauth2_provider.go | 19 ++++++++++++++-- 3 files changed, 74 insertions(+), 4 deletions(-) diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index 2f5aff0933523..5a2d9778ab512 100644 --- a/models/auth/oauth2.go +++ b/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,10 @@ import ( "xorm.io/xorm" ) +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 +391,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 +416,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,6 +491,7 @@ func (grant *OAuth2Grant) GenerateNewAuthorizationCode(ctx context.Context, redi // for code scanners to grab sensitive tokens. codeSecret := "gta_" + base32Lower.EncodeToString(rBytes) + validUntil := timeutil.TimeStampNow() + timeutil.TimeStamp(oauth2AuthorizationCodeValidity/time.Second) code = &OAuth2AuthorizationCode{ Grant: grant, GrantID: grant.ID, @@ -479,6 +499,7 @@ func (grant *OAuth2Grant) GenerateNewAuthorizationCode(ctx context.Context, redi Code: codeSecret, CodeChallenge: codeChallenge, CodeChallengeMethod: codeChallengeMethod, + ValidUntil: validUntil, } if err := db.Insert(ctx, code); err != nil { return nil, err diff --git a/models/auth/oauth2_test.go b/models/auth/oauth2_test.go index 97f750755a20d..71390fce144ae 100644 --- a/models/auth/oauth2_test.go +++ b/models/auth/oauth2_test.go @@ -5,13 +5,47 @@ 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) { + mockNow := time.Date(2024, 1, 1, 1, 2, 3, 0, time.UTC) + defer timeutil.MockSet(mockNow)() + + grant := unittest.AssertExistsAndLoadBean(t, &auth_model.OAuth2Grant{ID: 1}) + code, err := grant.GenerateNewAuthorizationCode(t.Context(), "http://127.0.0.1/", "", "") + assert.NoError(t, err) + assert.Equal(t, timeutil.TimeStamp(mockNow.Unix()+600), 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}) diff --git a/routers/web/auth/oauth2_provider.go b/routers/web/auth/oauth2_provider.go index e09469526e930..cc02970dea10a 100644 --- a/routers/web/auth/oauth2_provider.go +++ b/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 proceed 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 } From 374b233648c75b6b0689a754f24eba53514542b5 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 1 Mar 2026 17:06:57 -0800 Subject: [PATCH 2/6] Add comment --- models/auth/oauth2.go | 1 + 1 file changed, 1 insertion(+) diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index 5a2d9778ab512..b7c7707f1e059 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -28,6 +28,7 @@ import ( "xorm.io/xorm" ) +// the expired time should less than 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") From e6b6b02dd686774d07273101b5ff7aa82e8b87e8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 1 Mar 2026 23:51:34 -0800 Subject: [PATCH 3/6] Update routers/web/auth/oauth2_provider.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Lunny Xiao --- routers/web/auth/oauth2_provider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/auth/oauth2_provider.go b/routers/web/auth/oauth2_provider.go index cc02970dea10a..05931d8f59627 100644 --- a/routers/web/auth/oauth2_provider.go +++ b/routers/web/auth/oauth2_provider.go @@ -640,7 +640,7 @@ 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 proceed your request" + errDescription := "cannot process your request" errCode := oauth2_provider.AccessTokenErrorCodeInvalidRequest if errors.Is(err, auth.ErrOAuth2AuthorizationCodeInvalidated) { errDescription = "authorization code already used" From 05f69636d1aaa13127ab49756f26e073649690cc Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 1 Mar 2026 23:51:40 -0800 Subject: [PATCH 4/6] Update models/auth/oauth2.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Lunny Xiao --- models/auth/oauth2.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index b7c7707f1e059..771a7b16173e8 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -28,7 +28,7 @@ import ( "xorm.io/xorm" ) -// the expired time should less than 10 minutes per https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2 +// 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") From a54edef0453e0464e2e536ca7c89398a82e23ee6 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 1 Mar 2026 23:54:34 -0800 Subject: [PATCH 5/6] adjustment --- models/auth/oauth2.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index 771a7b16173e8..e2bb72b722a71 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -492,7 +492,7 @@ func (grant *OAuth2Grant) GenerateNewAuthorizationCode(ctx context.Context, redi // for code scanners to grab sensitive tokens. codeSecret := "gta_" + base32Lower.EncodeToString(rBytes) - validUntil := timeutil.TimeStampNow() + timeutil.TimeStamp(oauth2AuthorizationCodeValidity/time.Second) + validUntil := time.Now().Add(oauth2AuthorizationCodeValidity) code = &OAuth2AuthorizationCode{ Grant: grant, GrantID: grant.ID, @@ -500,7 +500,7 @@ func (grant *OAuth2Grant) GenerateNewAuthorizationCode(ctx context.Context, redi Code: codeSecret, CodeChallenge: codeChallenge, CodeChallengeMethod: codeChallengeMethod, - ValidUntil: validUntil, + ValidUntil: timeutil.TimeStamp(validUntil.Unix()), } if err := db.Insert(ctx, code); err != nil { return nil, err From b6ee1700aa51ef686ca35f25aec2506afc64162d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 2 Mar 2026 10:48:04 -0800 Subject: [PATCH 6/6] Fix test --- models/auth/oauth2_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/models/auth/oauth2_test.go b/models/auth/oauth2_test.go index 71390fce144ae..88ae065652c3a 100644 --- a/models/auth/oauth2_test.go +++ b/models/auth/oauth2_test.go @@ -18,13 +18,11 @@ func TestOAuth2AuthorizationCodeValidity(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) t.Run("GenerateSetsValidUntil", func(t *testing.T) { - mockNow := time.Date(2024, 1, 1, 1, 2, 3, 0, time.UTC) - defer timeutil.MockSet(mockNow)() - 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, timeutil.TimeStamp(mockNow.Unix()+600), code.ValidUntil) + assert.Equal(t, expectedValidUntil, code.ValidUntil) assert.False(t, code.IsExpired()) assert.NoError(t, code.Invalidate(t.Context())) })