forked from goauthentik/authentik
-
Notifications
You must be signed in to change notification settings - Fork 0
fix(oauth2): honour RedirectURI matching_mode in CORS origin check [Buzzvil fork patch] #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,24 @@ | ||
| # Buzzvil patched authentik image (overlay on the official server image). | ||
| # | ||
| # Replaces two SAML provider files so that IdP-initiated SSO forwards the | ||
| # SP-supplied RelayState (AWS WorkSpaces appends a per-session state code via | ||
| # ?RelayState= to the init URL) instead of only using the static | ||
| # default_relay_state. Without this, the WorkSpaces native client loops back to | ||
| # sign-in because euc-sso cannot bind the auth code to the client session. | ||
| # Overlays Buzzvil fork patches by replacing Python files in place. Paths below | ||
| # match the official image layout (/authentik/...); patch source of truth is this | ||
| # repo's authentik/ tree. | ||
| # | ||
| # Patch source of truth: this repo's authentik/ tree (see PR #1). File paths below | ||
| # match the official image layout (/authentik/...). | ||
| # 1) SAML RelayState (PR #1): IdP-initiated SSO forwards the SP-supplied | ||
| # RelayState (AWS WorkSpaces appends a per-session state code via | ||
| # ?RelayState= to the init URL) instead of only using the static | ||
| # default_relay_state. Without this the WorkSpaces native client loops back | ||
| # to sign-in because euc-sso cannot bind the auth code to the client session. | ||
| # 2) OAuth2 CORS regex (PR #2): cors_allow honours each redirect URI's | ||
| # matching_mode so REGEX-mode redirect URIs are matched against the request | ||
| # Origin (backport of goauthentik/authentik#22179). utils.py carries the fix; | ||
| # the three view files must be overlaid together because their cors_allow() | ||
| # calls now pass RedirectURI objects to the patched signature. | ||
| FROM ghcr.io/goauthentik/server:2026.2.1 | ||
|
|
||
| COPY --chown=authentik:authentik authentik/providers/saml/processors/authn_request_parser.py /authentik/providers/saml/processors/authn_request_parser.py | ||
| COPY --chown=authentik:authentik authentik/providers/saml/views/sso.py /authentik/providers/saml/views/sso.py | ||
| COPY --chown=authentik:authentik authentik/providers/oauth2/utils.py /authentik/providers/oauth2/utils.py | ||
| COPY --chown=authentik:authentik authentik/providers/oauth2/views/token.py /authentik/providers/oauth2/views/token.py | ||
| COPY --chown=authentik:authentik authentik/providers/oauth2/views/userinfo.py /authentik/providers/oauth2/views/userinfo.py | ||
| COPY --chown=authentik:authentik authentik/providers/oauth2/views/provider.py /authentik/providers/oauth2/views/provider.py |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| """Test OAuth2 utility helpers""" | ||
|
|
||
| from django.http import HttpResponse | ||
| from django.test import RequestFactory, TestCase | ||
|
|
||
| from authentik.providers.oauth2.models import RedirectURI, RedirectURIMatchingMode | ||
| from authentik.providers.oauth2.utils import cors_allow | ||
|
|
||
|
|
||
| class TestCorsAllow(TestCase): | ||
| """Tests for ``cors_allow``: ensures regex-mode redirect URI entries are honoured | ||
| for CORS origin validation, not only for the authorize-step redirect_uri check.""" | ||
|
|
||
| def setUp(self): | ||
| self.factory = RequestFactory() | ||
|
|
||
| def _post(self, origin: str): | ||
| return self.factory.post("/", HTTP_ORIGIN=origin) | ||
|
|
||
| def test_strict_match(self): | ||
| """Strict origin entry matches scheme + host + port (regression).""" | ||
| request = self._post("http://local.invalid") | ||
| response = cors_allow( | ||
| request, | ||
| HttpResponse(), | ||
| RedirectURI(RedirectURIMatchingMode.STRICT, "http://local.invalid/callback"), | ||
| ) | ||
| self.assertEqual(response["Access-Control-Allow-Origin"], "http://local.invalid") | ||
| self.assertEqual(response["Access-Control-Allow-Credentials"], "true") | ||
|
|
||
| def test_strict_no_match(self): | ||
| """Strict entry on a different host does not allow the origin.""" | ||
| request = self._post("http://other.invalid") | ||
| response = cors_allow( | ||
| request, | ||
| HttpResponse(), | ||
| RedirectURI(RedirectURIMatchingMode.STRICT, "http://local.invalid/callback"), | ||
| ) | ||
| self.assertNotIn("Access-Control-Allow-Origin", response) | ||
|
|
||
| def test_regex_match(self): | ||
| """Regex entry matching the Origin sets the CORS header.""" | ||
| request = self._post("https://app-abc123.example.com") | ||
| response = cors_allow( | ||
| request, | ||
| HttpResponse(), | ||
| RedirectURI( | ||
| RedirectURIMatchingMode.REGEX, | ||
| r"https://app-\w+\.example\.com", | ||
| ), | ||
| ) | ||
| self.assertEqual(response["Access-Control-Allow-Origin"], "https://app-abc123.example.com") | ||
|
|
||
| def test_regex_no_match(self): | ||
| """Regex entry that does not match the Origin must not allow the request.""" | ||
| request = self._post("https://evil.example.com") | ||
| response = cors_allow( | ||
| request, | ||
| HttpResponse(), | ||
| RedirectURI( | ||
| RedirectURIMatchingMode.REGEX, | ||
| r"https://app-\w+\.example\.com", | ||
| ), | ||
| ) | ||
| self.assertNotIn("Access-Control-Allow-Origin", response) | ||
|
|
||
| def test_regex_with_optional_path_group(self): | ||
| """A regex written as ``host(/path)?`` should match the bare Origin (without path) | ||
| as well as the full redirect URI used by the authorize step.""" | ||
| regex = r"https://app-\w+\.example\.com(/callback)?" | ||
| # Origin (no path) | ||
| response = cors_allow( | ||
| self._post("https://app-foo.example.com"), | ||
| HttpResponse(), | ||
| RedirectURI(RedirectURIMatchingMode.REGEX, regex), | ||
| ) | ||
| self.assertEqual(response["Access-Control-Allow-Origin"], "https://app-foo.example.com") | ||
| # Origin equal to a wrong host: must not match. | ||
| response = cors_allow( | ||
| self._post("https://other.example.com"), | ||
| HttpResponse(), | ||
| RedirectURI(RedirectURIMatchingMode.REGEX, regex), | ||
| ) | ||
| self.assertNotIn("Access-Control-Allow-Origin", response) | ||
|
|
||
| def test_regex_full_redirect_uri_does_not_match_bare_origin(self): | ||
| """A regex written for the full redirect URI (including a mandatory path) will | ||
| not match a bare Origin header. This is the documented behaviour: users wanting | ||
| regex CORS support should write ``host(/path)?`` or add a separate origin entry.""" | ||
| request = self._post("https://app-abc.example.com") | ||
| response = cors_allow( | ||
| request, | ||
| HttpResponse(), | ||
| RedirectURI( | ||
| RedirectURIMatchingMode.REGEX, | ||
| r"https://app-\w+\.example\.com/callback", | ||
| ), | ||
| ) | ||
| self.assertNotIn("Access-Control-Allow-Origin", response) | ||
|
|
||
| def test_malformed_regex_is_skipped(self): | ||
| """Malformed regexes are logged and skipped; a later valid entry still matches.""" | ||
| request = self._post("https://good.example.com") | ||
| response = cors_allow( | ||
| request, | ||
| HttpResponse(), | ||
| RedirectURI(RedirectURIMatchingMode.REGEX, r"["), # invalid regex | ||
| RedirectURI(RedirectURIMatchingMode.REGEX, r"https://good\.example\.com"), | ||
| ) | ||
| self.assertEqual(response["Access-Control-Allow-Origin"], "https://good.example.com") | ||
|
|
||
| def test_malformed_regex_alone_does_not_500(self): | ||
| """A malformed regex with no other allowed entries must return the response | ||
| unchanged rather than raising.""" | ||
| request = self._post("https://anything.example.com") | ||
| response = cors_allow( | ||
| request, | ||
| HttpResponse(), | ||
| RedirectURI(RedirectURIMatchingMode.REGEX, r"["), | ||
| ) | ||
| self.assertNotIn("Access-Control-Allow-Origin", response) | ||
|
|
||
| def test_options_request_allowed_without_match(self): | ||
| """Pre-flight OPTIONS requests are allowed through without an entry match | ||
| (regression: existing behaviour).""" | ||
| request = self.factory.options( | ||
| "/", | ||
| HTTP_ORIGIN="https://anywhere.example.com", | ||
| HTTP_ACCESS_CONTROL_REQUEST_HEADERS="content-type", | ||
| ) | ||
| response = cors_allow(request, HttpResponse()) | ||
| self.assertEqual(response["Access-Control-Allow-Origin"], "https://anywhere.example.com") | ||
|
|
||
| def test_bare_string_treated_as_strict(self): | ||
| """Backwards compatibility: bare string entries are treated as STRICT.""" | ||
| request = self._post("http://local.invalid") | ||
| response = cors_allow(request, HttpResponse(), "http://local.invalid/callback") | ||
| self.assertEqual(response["Access-Control-Allow-Origin"], "http://local.invalid") | ||
|
|
||
| def test_mixed_strict_and_regex(self): | ||
| """A provider with one strict and one regex entry honours both.""" | ||
| entries = ( | ||
| RedirectURI(RedirectURIMatchingMode.STRICT, "http://local.invalid/callback"), | ||
| RedirectURI(RedirectURIMatchingMode.REGEX, r"https://app-\w+\.example\.com"), | ||
| ) | ||
| # Strict origin matches via the strict entry. | ||
| response = cors_allow(self._post("http://local.invalid"), HttpResponse(), *entries) | ||
| self.assertEqual(response["Access-Control-Allow-Origin"], "http://local.invalid") | ||
| # Regex origin matches via the regex entry. | ||
| response = cors_allow(self._post("https://app-xyz.example.com"), HttpResponse(), *entries) | ||
| self.assertEqual(response["Access-Control-Allow-Origin"], "https://app-xyz.example.com") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 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
🧰 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
Source: Linters/SAST tools