Skip to content

fix(oauth): strengthen PKCE validation and refresh token replay protection#37706

Merged
bircni merged 6 commits into
go-gitea:mainfrom
lunny:lunny/fix_oauth_redirect
May 16, 2026
Merged

fix(oauth): strengthen PKCE validation and refresh token replay protection#37706
bircni merged 6 commits into
go-gitea:mainfrom
lunny:lunny/fix_oauth_redirect

Conversation

@lunny
Copy link
Copy Markdown
Member

@lunny lunny commented May 14, 2026

This PR tightens several OAuth validation paths related to PKCE handling, redirect URI normalization, and refresh-token replay safety.

What it changes:

  • switch redirect URI comparison to ASCII-only normalization for exact-match checks, avoiding Unicode case-folding surprises
  • harden PKCE verification by:
    • allowing PKCE omission only when no challenge data was stored
    • rejecting exchanges with a missing verifier when PKCE was used
    • rejecting malformed challenge state where a challenge exists without a valid method
    • comparing derived challenges with constant-time string matching
  • make refresh-token invalidation counter updates conditional on the previously observed counter value, so stale refresh state cannot be accepted after the grant changes

Why:

These checks close gaps where:

  • redirect URI comparisons could rely on broader Unicode normalization than intended
  • malformed or incomplete PKCE state could be treated too permissively
  • concurrent or stale refresh-token use could advance the same grant more than once

@lunny lunny added type/bug backport/v1.26 This PR should be backported to Gitea 1.26 labels May 14, 2026
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 14, 2026
@bircni bircni requested a review from wxiaoguang May 15, 2026 07:25
@silverwind @claude
refactor(oauth): use x/oauth2 S256ChallengeFromVerifier and tighten t…
e3463df 
…ests

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
silverwind
silverwind approved these changes May 16, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@silverwind silverwind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cleaned up in e3463df

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels May 16, 2026
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 16, 2026
@bircni bircni enabled auto-merge (squash) May 16, 2026 14:17
@bircni bircni added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 16, 2026
@bircni bircni merged commit ae69aec into go-gitea:main May 16, 2026
21 checks passed
@GiteaBot GiteaBot added this to the 1.27.0 milestone May 16, 2026
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 16, 2026
@wxiaoguang wxiaoguang deleted the lunny/fix_oauth_redirect branch May 16, 2026 15:40
@GiteaBot
Copy link
Copy Markdown
Collaborator

GiteaBot commented May 16, 2026

I was unable to create a backport for 1.26. @lunny, please send one manually. 🍵

go run ./contrib/backport 37706
...  // fix git conflicts if any
go run ./contrib/backport --continue

@GiteaBot GiteaBot added the backport/manual No power to the bots! Create your backport yourself! label May 16, 2026
@lunny lunny mentioned this pull request May 17, 2026
Merged
@lunny lunny added the backport/done All backports for this PR have been created label May 17, 2026
bircni added a commit that referenced this pull request May 17, 2026
@lunny @silverwind
@claude @bircni
fix(oauth): strengthen PKCE validation and refresh token replay prote…
5038561 
…ction (#37706) (#37738)

Backport #37706 

This PR tightens several OAuth validation paths related to PKCE
handling, redirect URI normalization, and refresh-token replay safety.

What it changes:

- switch redirect URI comparison to ASCII-only normalization for
exact-match checks, avoiding Unicode case-folding surprises
- harden PKCE verification by:
  - allowing PKCE omission only when no challenge data was stored
  - rejecting exchanges with a missing verifier when PKCE was used
- rejecting malformed challenge state where a challenge exists without a
valid method
  - comparing derived challenges with constant-time string matching
- make refresh-token invalidation counter updates conditional on the
previously observed counter value, so stale refresh state cannot be
accepted after the grant changes

Why:

These checks close gaps where:
- redirect URI comparisons could rely on broader Unicode normalization
than intended
- malformed or incomplete PKCE state could be treated too permissively
- concurrent or stale refresh-token use could advance the same grant
more than once

Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: Claude (Opus 4.7) <noreply@anthropic.com>
Co-authored-by: Nicolas <bircni@icloud.com>
silverwind added a commit to silverwind/gitea that referenced this pull request May 18, 2026
@silverwind
Merge remote-tracking branch 'origin/main' into ci-cache-improvements
751acd4 
* origin/main: (39 commits)
  fix: Add missed token scope checking (go-gitea#37735)
  chore: Use giteabot instead of backporter (go-gitea#37422)
  fix: Allow direct commits for unprotected files with push restrictions (go-gitea#37657)
  chore: Conventional adjustments (go-gitea#37677)
  chore(db): introduce db.Session and db.EngineMigration interfaces (go-gitea#37746)
  fix(migrations): preserve unique constraints in v334 sync (go-gitea#37743)
  feat(web): also display PR counts in repo list (go-gitea#37739)
  feat: execute post run cleanup when workflow is cancelled (go-gitea#37275)
  fix(actions): wrong assumption that run id always >= job id (go-gitea#37737)
  fix(icon): use repo-forked icon to display forks count (go-gitea#37731)
  fix(oauth): strengthen PKCE validation and refresh token replay protection (go-gitea#37706)
  fix(web): enforce token scopes on raw, media, and attachment downloads (go-gitea#37698)
  feat: Add bypass allowlist for branch protection (go-gitea#36514)
  refactor(glob): use strings.Builder for regexp compilation (go-gitea#37730)
  feat(oauth): Support AWS Cognito OAuth2 provider (go-gitea#37607)
  feat: Add default PR branch update style setting (go-gitea#37410)
  refactor: move `workflowpattern` into `modules/actions` (go-gitea#37717)
  ci: add `zizmor` to `lint-actions` (go-gitea#37720)
  chore(doctor): remove four obsolete doctor check implementations (go-gitea#37728)
  chore(renovate): enable dockerfile manager (go-gitea#37719)
  ...

# Conflicts:
#	modules/globallock/locker_test.go
eleboucher pushed a commit to eleboucher/apoci that referenced this pull request May 20, 2026
@eleboucher
fix(deps): update module code.gitea.io/gitea (v1.26.1 → v1.26.2) (#47)
ce5a949 
This PR contains the following updates:

| Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) |
|---|---|---|---|
| [code.gitea.io/gitea](https://github.com/go-gitea/gitea) | `v1.26.1` → `v1.26.2` | ![age](https://developer.mend.io/api/mc/badges/age/go/code.gitea.io%2fgitea/v1.26.2?slim=true) | ![confidence](https://developer.mend.io/api/mc/badges/confidence/go/code.gitea.io%2fgitea/v1.26.1/v1.26.2?slim=true) |

---

### Release Notes

<details>
<summary>go-gitea/gitea (code.gitea.io/gitea)</summary>

### [`v1.26.2`](https://github.com/go-gitea/gitea/releases/tag/v1.26.2)

[Compare Source](go-gitea/gitea@v1.26.1...v1.26.2)

- SECURITY
  - fix(permissions): Fix reading permission ([#&#8203;37769](go-gitea/gitea#37769))
  - fix(actions): make artifact signature payloads unambiguous ([#&#8203;37707](go-gitea/gitea#37707))
  - fix: Unify public-only token filtering in API queries and repo access checks ([#&#8203;37118](go-gitea/gitea#37118))
  - fix: Add missed token scope checking ([#&#8203;37735](go-gitea/gitea#37735))
  - fix(oauth): bind token exchanges to the original client request ([#&#8203;37704](go-gitea/gitea#37704))
  - fix(oauth): strengthen PKCE validation and refresh token replay protection ([#&#8203;37706](go-gitea/gitea#37706))
  - fix(web): enforce token scopes on raw, media, and attachment downloads ([#&#8203;37698](go-gitea/gitea#37698))
  - fix(security): enforce wiki git writes and LFS token access at request time ([#&#8203;37695](go-gitea/gitea#37695))
  - feat(api): encrypt AWS creds ([#&#8203;37679](go-gitea/gitea#37679))
  - fix(deps): update dependency mermaid to v11.15.0 \[security], add e2e test
  - fix(packages): Add label for private and internal package and fix composor package source permission check ([#&#8203;37610](go-gitea/gitea#37610))
  - fix(git): Fix smart http request scope bug ([#&#8203;37583](go-gitea/gitea#37583))
  - Fix basic auth bug ([#&#8203;37503](go-gitea/gitea#37503))
  - Fix allow maintainer edit permission check ([#&#8203;37479](go-gitea/gitea#37479)) ([#&#8203;37484](go-gitea/gitea#37484))
  - Fix URL sanitization to handle schemeless credentials ([#&#8203;37440](go-gitea/gitea#37440)) ([#&#8203;37471](go-gitea/gitea#37471))
  - Fix attachment Content-Security-Policy ([#&#8203;37455](go-gitea/gitea#37455)) ([#&#8203;37464](go-gitea/gitea#37464))
  - chore(deps): bump go-git/go-git/v5 to 5.19.0 ([#&#8203;37608](go-gitea/gitea#37608))

- BUGFIXES
  - fix(pull): handle empty pull request files view to allow reviews ([#&#8203;37783](go-gitea/gitea#37783))
  - fix(markup): make RenderString never fail ([#&#8203;37779](go-gitea/gitea#37779))
  - fix: add natural sort to sortTreeViewNodes ([#&#8203;37772](go-gitea/gitea#37772))
  - fix: package creation unique conflict ([#&#8203;37774](go-gitea/gitea#37774))
  - fix!: add DEFAULT\_TITLE\_SOURCE setting for pull request title default behavior ([#&#8203;37465](go-gitea/gitea#37465))
  - fix: Allow direct commits for unprotected files with push restrictions ([#&#8203;37657](go-gitea/gitea#37657))
  - fix(actions): wrong assumption that run id always >= job id ([#&#8203;37737](go-gitea/gitea#37737))
  - fix(auth): set User-Agent on avatar fetch and sync avatar on link-account register ([#&#8203;37564](go-gitea/gitea#37564)) ([#&#8203;37588](go-gitea/gitea#37588))
  - fix(actions): deadlock between PrepareRunAndInsert and UpdateTaskByState ([#&#8203;37692](go-gitea/gitea#37692))
  - fix(repo): /generate must sync the branch table for the new repo ([#&#8203;37693](go-gitea/gitea#37693))
  - build: Fix snap build (1.26)
  - fix(actions): run TransferLogs on UpdateLog{Rows:\[], NoMore:true} ([#&#8203;37631](go-gitea/gitea#37631))
  - fix show correct mergebase
  - fix: make clone URL respect public URL detection setting ([#&#8203;37615](go-gitea/gitea#37615))
  - fix: "run as root" check ([#&#8203;37622](go-gitea/gitea#37622))
  - chore(deps): update dependency go to v1.26.3 ([#&#8203;37601](go-gitea/gitea#37601))
  - Compare dropdown fails when selecting branch with no common merge-base ([#&#8203;37470](go-gitea/gitea#37470))
  - fix: treat email addresses case-insensitively ([#&#8203;37600](go-gitea/gitea#37600))
  - fix(actions): fix blank lines after ::endgroup:: ([#&#8203;37597](go-gitea/gitea#37597))
  - fix(actions): report individual step status in workflow job API response ([#&#8203;37592](go-gitea/gitea#37592))
  - fix: Invalid UTF-8 commit messages in JSON API responses ([#&#8203;37542](go-gitea/gitea#37542))
  - fix: use consistent GetUser family functions ([#&#8203;37553](go-gitea/gitea#37553))
  - fix(api): return 409 message instead of empty JSON for wrong commit id ([#&#8203;37572](go-gitea/gitea#37572))
  - fix(actions): prevent panic when workflow contains null jobs ([#&#8203;37570](go-gitea/gitea#37570))
  - Make ServeSetHeaders default to download attachment if filename exists ([#&#8203;37552](go-gitea/gitea#37552)) ([#&#8203;37555](go-gitea/gitea#37555))
  - Fix(actions): validate workflow param to prevent 500 error ([#&#8203;37546](go-gitea/gitea#37546)) ([#&#8203;37554](go-gitea/gitea#37554))
  - Don't unblock run-level-concurrency-blocked runs in the resolver ([#&#8203;37461](go-gitea/gitea#37461)) ([#&#8203;37538](go-gitea/gitea#37538))
  - Fix(packages): use file names for generic web downloads ([#&#8203;37514](go-gitea/gitea#37514)) ([#&#8203;37520](go-gitea/gitea#37520))
  - Fix merge autodetect can't close other PRs but only the last one when multiple PRs are pushed at once ([#&#8203;37512](go-gitea/gitea#37512)) ([#&#8203;37516](go-gitea/gitea#37516))
  - Fix update branch protection order ([#&#8203;37508](go-gitea/gitea#37508)) ([#&#8203;37513](go-gitea/gitea#37513))
  - Fix mCaptcha broken after Vite migration ([#&#8203;37492](go-gitea/gitea#37492)) ([#&#8203;37509](go-gitea/gitea#37509))
  - Fix review submission from single-commit PR view ([#&#8203;37475](go-gitea/gitea#37475)) ([#&#8203;37485](go-gitea/gitea#37485))
  - Fix scheduled action panic with null event payload ([#&#8203;37459](go-gitea/gitea#37459)) ([#&#8203;37466](go-gitea/gitea#37466))
  - Make GetPossibleUserByID can handle deleted user ([#&#8203;37430](go-gitea/gitea#37430)) ([#&#8203;37431](go-gitea/gitea#37431))
  - Remove excessive quote from terraform instructions ([#&#8203;37424](go-gitea/gitea#37424)) ([#&#8203;37426](go-gitea/gitea#37426))
  - Fix color regressions, add `priority` color ([#&#8203;37417](go-gitea/gitea#37417)) ([#&#8203;37421](go-gitea/gitea#37421))

- MISC
  - Add CurrentURL template variable back ([#&#8203;37444](go-gitea/gitea#37444)) ([#&#8203;37449](go-gitea/gitea#37449))

Instances on **[Gitea Cloud](https://cloud.gitea.com)** will be automatically upgraded to this version during the specified maintenance window.

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMDEuMSIsInVwZGF0ZWRJblZlciI6IjQzLjEwMS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJ0eXBlL3BhdGNoIl19-->

Reviewed-on: https://git.erwanleboucher.dev/eleboucher/apoci/pulls/47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@silverwind silverwind silverwind approved these changes

@bircni bircni bircni approved these changes

@wxiaoguang wxiaoguang Awaiting requested review from wxiaoguang

Assignees

No one assigned

Labels

backport/done All backports for this PR have been created backport/manual No power to the bots! Create your backport yourself! backport/v1.26 This PR should be backported to Gitea 1.26 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug

Projects

None yet

Milestone

1.27.0

Development

Successfully merging this pull request may close these issues.

4 participants

@lunny @GiteaBot @silverwind @bircni