fix(oauth2): honour RedirectURI matching_mode in CORS origin check [Buzzvil fork patch]#2
Conversation
…uzzvil fork patch] The OAuth2 provider supports STRICT and REGEX matching_mode for redirect URI entries. views/authorize.py honours both when validating the redirect_uri query param, but cors_allow() ignored matching_mode entirely: callers flattened redirect_uris to bare url strings and cors_allow only did scheme/host/port equality via urlparse. Effect: a browser app configured with a REGEX redirect URI completes /authorize/ but its fetch() to /application/o/token/ (and /userinfo/, and the provider-info endpoint) is blocked because the response carries no Access-Control-Allow-Origin header. Worker logs show "CORS: Origin is not an allowed origin" with the regex verbatim in `allowed`, never matched. - cors_allow now accepts RedirectURI | str and branches on matching_mode. Strict entries keep the existing scheme+host+port comparison (regression-safe). Regex entries are matched against the request Origin via re.fullmatch, mirroring views/authorize.py. - Malformed regexes raise re.error, which is caught, logged, and skipped so one bad entry cannot 500 the request. - The three call sites (token.py, userinfo.py, provider.py) pass RedirectURI objects through instead of flattening to url strings. - Bare string entries are still accepted (treated as STRICT) for backwards compatibility. A CORS Origin is scheme+host[:port] with no path, so a regex written for the full redirect URI with a mandatory path will not match a bare Origin; write it as host(/path)? or add a separate origin-only entry. Backport of upstream goauthentik#22179 (closes goauthentik#3084) onto the Buzzvil 2026.2.1 fork. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WalkthroughOAuth2 CORS 처리가 문자열과 Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…ed image The patched-image workflow builds an overlay on ghcr.io/goauthentik/server by COPYing only the changed Python files (sparse-checkout + Dockerfile COPY). Until now it carried the two SAML RelayState files only, so a `-relaystate` tag build would NOT include the oauth2 CORS regex fix. - .buzzvil/Dockerfile: COPY the four oauth2 runtime files (utils.py + views/token.py, userinfo.py, provider.py). All four must ship together — the views pass RedirectURI objects into cors_allow's patched signature. The test file is not runtime code and is deliberately excluded. - workflow sparse-checkout: add the same four files so they exist in the build context. - generalise the SAML-only comments and release title now that the image bundles both fork patches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
authentik/providers/oauth2/utils.py (1)
81-91: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy liftREGEX redirect URI의 실행 비용을 제한하세요.
RedirectURIMatchingMode.REGEX는 저장 시 문법만 검사하고, 요청마다Origin에re.fullmatch()를 그대로 수행합니다. 백트래킹이 큰 패턴은 워커를 오래 점유할 수 있으니, 저장 단계에서 허용 문법/복잡도 제한을 두거나 안전한 정규식 엔진으로 바꾸세요. 잘못된 패턴은 지금처럼 로그 후 건너뛰면 됩니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@authentik/providers/oauth2/utils.py` around lines 81 - 91, Limit regex redirect URI execution cost by enforcing allowed syntax and complexity when REGEX entries are stored, or replace Python re.fullmatch in the REGEX matching path with a safe linear-time regex engine. Preserve the existing behavior for invalid patterns: log the parsing failure and skip the entry.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-patched-ecr.yaml:
- Around line 78-79: Update the workflow step containing the gh release --title
and --notes arguments to pass github.ref_name through the step’s env
configuration, alongside the existing Line 76 usage. Replace every direct ${{
github.ref_name }} interpolation in the shell command with a quoted shell
variable expansion, preserving the release title and notes content.
---
Nitpick comments:
In `@authentik/providers/oauth2/utils.py`:
- Around line 81-91: Limit regex redirect URI execution cost by enforcing
allowed syntax and complexity when REGEX entries are stored, or replace Python
re.fullmatch in the REGEX matching path with a safe linear-time regex engine.
Preserve the existing behavior for invalid patterns: log the parsing failure and
skip the entry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e392d749-67e5-4973-9b88-59918b6cce61
📒 Files selected for processing (7)
.buzzvil/Dockerfile.github/workflows/build-patched-ecr.yamlauthentik/providers/oauth2/tests/test_utils.pyauthentik/providers/oauth2/utils.pyauthentik/providers/oauth2/views/provider.pyauthentik/providers/oauth2/views/token.pyauthentik/providers/oauth2/views/userinfo.py
| --title "${{ github.ref_name }} — Buzzvil patched authentik (SAML RelayState + OAuth2 CORS regex)" \ | ||
| --notes "패치 이미지: \`ghcr.io/buzzvil/authentik:${{ github.ref_name }}\` (multi-arch amd64/arm64, public). 오버레이 패치 목록·빌드·제거 조건은 .buzzvil/Dockerfile 및 저장소 README 참고." \ |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
태그명을 셸 명령에 직접 보간하지 마세요.
Line 78-79의 ${{ github.ref_name }}는 셸 실행 전에 확장됩니다. $() 등을 포함한 태그가 생성 가능하면 명령 치환으로 실행될 수 있습니다. 기존 Line 76도 함께 env 변수로 옮긴 뒤 인용된 변수 확장만 사용하세요.
수정 예시
env:
GH_TOKEN: ${{ github.token }}
+ RELEASE_TAG: ${{ github.ref_name }}
run: |
- gh release create "${{ github.ref_name }}" \
+ gh release create "$RELEASE_TAG" \
--repo "${{ github.repository }}" \
- --title "${{ github.ref_name }} — Buzzvil patched authentik (SAML RelayState + OAuth2 CORS regex)" \
- --notes "패치 이미지: \`ghcr.io/buzzvil/authentik:${{ github.ref_name }}\` (multi-arch amd64/arm64, public). 오버레이 패치 목록·빌드·제거 조건은 .buzzvil/Dockerfile 및 저장소 README 참고." \
+ --title "$RELEASE_TAG — Buzzvil patched authentik (SAML RelayState + OAuth2 CORS regex)" \
+ --notes "패치 이미지: \`ghcr.io/buzzvil/authentik:${RELEASE_TAG}\` (multi-arch amd64/arm64, public). 오버레이 패치 목록·빌드·제거 조건은 .buzzvil/Dockerfile 및 저장소 README 참고." \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --title "${{ github.ref_name }} — Buzzvil patched authentik (SAML RelayState + OAuth2 CORS regex)" \ | |
| --notes "패치 이미지: \`ghcr.io/buzzvil/authentik:${{ github.ref_name }}\` (multi-arch amd64/arm64, public). 오버레이 패치 목록·빌드·제거 조건은 .buzzvil/Dockerfile 및 저장소 README 참고." \ | |
| env: | |
| GH_TOKEN: ${{ github.token }} | |
| RELEASE_TAG: ${{ github.ref_name }} | |
| run: | | |
| gh release create "$RELEASE_TAG" \ | |
| --repo "${{ github.repository }}" \ | |
| - --title "${{ github.ref_name }} — Buzzvil patched authentik (SAML RelayState + OAuth2 CORS regex)" \ | |
| --title "$RELEASE_TAG — Buzzvil patched authentik (SAML RelayState + OAuth2 CORS regex)" \ | |
| --notes "패치 이미지: \`ghcr.io/buzzvil/authentik:${RELEASE_TAG}\` (multi-arch amd64/arm64, public). 오버레이 패치 목록·빌드·제거 조건은 .buzzvil/Dockerfile 및 저장소 README 참고." \ |
🧰 Tools
🪛 zizmor (1.26.1)
[error] 78-78: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 79-79: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build-patched-ecr.yaml around lines 78 - 79, Update the
workflow step containing the gh release --title and --notes arguments to pass
github.ref_name through the step’s env configuration, alongside the existing
Line 76 usage. Replace every direct ${{ github.ref_name }} interpolation in the
shell command with a quoted shell variable expansion, preserving the release
title and notes content.
Source: Linters/SAST tools
요약
발견된 이슈전반적으로 코드는 견고하며 동작에 영향을 주는 버그(🔴/🟡)는 발견하지 못했습니다. 아래는 확인·검증한 사항과 사소한 개선 제안입니다.
의심 코드
종합 평가APPROVE
|
|
@C0deWave 이거 ai가 업스트림 레포에 pr 올라와있던 거 베껴왔다고 했습니다 |
|
아하 제가 이슈만 봤군요 |
2 participants
문제
Authentik OAuth2 provider의 redirect URI 항목은
matching_mode로 STRICT와 REGEX를 지원합니다.views/authorize.py는redirect_uri쿼리 파라미터 검증 시 두 모드를 모두 올바르게 처리하지만, CORS origin 검사(authentik/providers/oauth2/utils.py::cors_allow)는matching_mode를 완전히 무시했습니다.token.py,userinfo.py,provider.py)가[x.url for x in redirect_uris]로 URL 문자열만 뽑아 넘김 →matching_mode유실cors_allow는 각 항목을urlparse해서 scheme/host/port 동등 비교만 수행증상: REGEX 모드 redirect URI로 설정된 브라우저 앱(SPA)은
/authorize/는 통과하지만,/application/o/token/(및/userinfo/, provider-info 엔드포인트)로 보내는fetch()가 브라우저에서 차단됩니다. 응답에Access-Control-Allow-Origin헤더가 없기 때문입니다. 워커 로그에는 설정한 정규식이allowed에 그대로 찍힌 채CORS: Origin is not an allowed origin이 남고 매칭은 절대 되지 않습니다.변경
cors_allow가RedirectURI | str을 받아matching_mode로 분기Origin을re.fullmatch로 매칭 —views/authorize.py와 동일 시맨틱re.error를 잡아 로깅 후 skip → 항목 하나가 요청을 500으로 떨구지 않음RedirectURI객체를 그대로 전달matching_mode는 이미 각 항목에 저장됨Origin vs. path (운영 주의)
CORS
Origin은 scheme+host[:port]로 경로가 없습니다. 따라서 경로가 필수인 full redirect URI용 정규식은 bare Origin과 매칭되지 않습니다. REGEX CORS를 쓰려면 다음 중 하나로 항목을 작성하세요:테스트
authentik/providers/oauth2/tests/test_utils.py신규 추가 — strict 회귀, regex 매칭/미매칭, 선택적 경로 그룹, 필수 경로 미매칭(문서화된 동작), 잘못된 정규식 skip/500 방지, OPTIONS preflight, bare-string 하위호환, strict+regex 혼합 구성.출처 · 배포
base/2026.2.1포크에 백포트. upstream PR은 현재 open이며, 머지 시 다음 리베이스에서 이 포크 패치 제거 가능.2026.2.1-relaystate에서 빌드되므로, 이 PR 머지 후 그 태그를 base/2026.2.1 최신으로 갱신(재태깅)·재빌드해야 반영됩니다 — RelayState 패치(fix(saml): forward SP RelayState in IdP-initiated flow (AWS WorkSpaces) [Buzzvil fork patch] #1)와 동일 절차.🤖 Generated with Claude Code