-
Notifications
You must be signed in to change notification settings - Fork 1
fix(security,api): opaque prompt IDs and CardDAV single-decode #1206
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
Open
seonghobae
wants to merge
34
commits into
develop
Choose a base branch
from
goal/carddav-path-traversal-decode
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 26 commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
5673c53
fix(security,api): harden CardDAV paths and opaque prompt identifiers
seonghobae 6b8a5af
ci: revalidate security hardening on current head
seonghobae aa26b22
merge(develop): refresh organization isolation and CardDAV hardening
seonghobae ed3ed29
merge(develop): refresh security and opaque API hardening
seonghobae 0547162
Merge branch 'develop' into goal/carddav-path-traversal-decode
seonghobae 00205e7
Merge protected develop into security API fixes
seonghobae d57f161
Merge branch 'develop' into goal/carddav-path-traversal-decode
opencode-agent[bot] b1fd5dd
Merge protected develop into goal/carddav-path-traversal-decode
seonghobae ad7987a
Merge develop into goal/carddav-path-traversal-decode
seonghobae afe981b
test(carddav): reject ambiguous nested path encoding
seonghobae 0715733
test(carddav): reject invalid UTF-8 path octets
seonghobae 952f359
fix(carddav): enforce single-decode TXT path semantics
seonghobae d5e4286
docs(carddav): record TXT path decode boundary
seonghobae 4c8776e
refactor(carddav): leave document isolation to canonical security lane
seonghobae 808a54c
Merge branch 'develop' into goal/carddav-path-traversal-decode
opencode-agent[bot] d235e7c
test(auth): prove trusted OIDC admin sessions
seonghobae eefca3d
fix(auth): trust admin roles only from verified OIDC
seonghobae 59299a7
test(security): prove system admin source-policy boundary
seonghobae cc90e1e
fix(security): admit system admins inside source tenant boundary
seonghobae a9c9535
Merge branch 'develop' into goal/carddav-path-traversal-decode
seonghobae 7bdc601
Merge branch 'develop' into goal/carddav-path-traversal-decode
seonghobae 8b99f20
merge(develop): reconcile CardDAV single-decode onto current develop
cursoragent f15542c
Merge branch 'develop' into goal/carddav-path-traversal-decode
opencode-agent[bot] d7ae476
Merge branch 'develop' into goal/carddav-path-traversal-decode
opencode-agent[bot] cb55a7e
Merge branch 'develop' into goal/carddav-path-traversal-decode
seonghobae a280815
Merge branch 'develop' into goal/carddav-path-traversal-decode
seonghobae 4cae8cd
test(prompts): pin opaque response identifier schema
seonghobae 2533f3a
docs(prompts): record opaque response identifier contract
seonghobae 2389f0b
test(security): reject implicit orgless admin delegation
seonghobae 5eab848
fix(security): require concrete tenant for admin delegation
seonghobae aba2a03
test(carddav): remove duplicate asyncio marker
seonghobae 43467ed
test(carddav): preserve encoded reserved path identity
seonghobae 882c1dd
fix(carddav): preserve encoded reserved path characters
seonghobae 80a56be
docs(carddav): distinguish validation from reserved wire identity
seonghobae 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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| """Focused contracts for high-privilege OIDC session authority.""" | ||
|
|
||
| import time | ||
|
|
||
| import pytest | ||
| from fastapi import HTTPException | ||
|
|
||
| from api import auth as auth_module | ||
| from core.config import settings | ||
|
|
||
|
|
||
| ADMIN_ROLES = ( | ||
| "system_admin", | ||
| "platform_admin", | ||
| "tenant_admin", | ||
| "organization_admin", | ||
| ) | ||
|
|
||
|
|
||
| def _oidc_payload(role: str) -> dict[str, object]: | ||
| """Build a short-lived payload from the configured authoritative OIDC issuer.""" | ||
| return { | ||
| "iss": "https://login.example.test/realms/naruon", | ||
| "aud": "naruon-api", | ||
| "sub": "alice", | ||
| "role": role, | ||
| "org": "org-acme", | ||
| "groups": ["group-1"], | ||
| "workspace": "workspace-org-acme", | ||
| "exp": int(time.time()) + 300, | ||
| } | ||
|
|
||
|
|
||
| def _hmac_payload(role: str) -> dict[str, object]: | ||
| """Build compatibility-session metadata without granting membership authority.""" | ||
| return { | ||
| "ver": 1, | ||
| "iss": auth_module.SESSION_ISSUER, | ||
| "aud": auth_module.SESSION_AUDIENCE, | ||
| "sub": "alice", | ||
| "role": role, | ||
| "org": "org-acme", | ||
| "groups": ["group-1"], | ||
| "workspace": "workspace-org-acme", | ||
| "exp": int(time.time()) + 300, | ||
| } | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("admin_role", ADMIN_ROLES) | ||
| def test_trusted_oidc_session_can_supply_admin_role(monkeypatch, admin_role: str) -> None: | ||
| """A configured JWKS-backed IdP remains usable for authorized administrators.""" | ||
| previous_issuer_url = settings.OIDC_ISSUER_URL | ||
| previous_client_id = settings.OIDC_CLIENT_ID | ||
| settings.OIDC_ISSUER_URL = "https://login.example.test/realms/naruon" | ||
| settings.OIDC_CLIENT_ID = "naruon-api" | ||
| payload = _oidc_payload(admin_role) | ||
|
|
||
| monkeypatch.setattr(auth_module, "jwks_client", object()) | ||
| monkeypatch.setattr( | ||
| auth_module, | ||
| "_decode_cached_oidc_session_payload", | ||
| lambda _token: payload, | ||
| ) | ||
|
|
||
| try: | ||
| verified_payload, verifier = auth_module._verify_signed_session_token( | ||
| "trusted-idp-token" | ||
| ) | ||
| context = auth_module._auth_context_from_session_payload( | ||
| verified_payload, verifier | ||
| ) | ||
| finally: | ||
| settings.OIDC_ISSUER_URL = previous_issuer_url | ||
| settings.OIDC_CLIENT_ID = previous_client_id | ||
|
|
||
| assert verifier == "oidc" | ||
| assert context.role == admin_role | ||
| assert context.organization_id == "org-acme" | ||
| assert context.workspace_id == "workspace-org-acme" | ||
| assert context.session_verifier == "oidc" | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("admin_role", ADMIN_ROLES) | ||
| def test_hmac_compatibility_session_cannot_supply_admin_role(admin_role: str) -> None: | ||
| """HMAC compatibility credentials never become authoritative admin membership.""" | ||
| with pytest.raises(HTTPException) as exc: | ||
| auth_module._auth_context_from_session_payload( | ||
| _hmac_payload(admin_role), "hmac" | ||
| ) | ||
|
|
||
| assert exc.value.status_code == 401 | ||
| assert exc.value.detail == "Authentication required" |
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
44 changes: 44 additions & 0 deletions
44
backend/tests/test_carddav_encoded_path_canonicalization.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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| """Regression tests for canonical CardDAV TXT context-path execution.""" | ||
|
|
||
| import pytest | ||
|
|
||
| from services.carddav_discovery import _txt_context_path | ||
|
|
||
|
|
||
| def test_fully_encoded_leading_slash_is_canonicalized() -> None: | ||
| """Execute the same singly decoded representation that passed validation.""" | ||
| assert _txt_context_path(["path=%2Fsafe"]) == "/safe" | ||
|
|
||
|
|
||
| def test_percent_encoded_unicode_path_is_canonicalized() -> None: | ||
| """Preserve a safe Unicode path after one percent-decoding pass.""" | ||
| assert _txt_context_path(["path=/%EC%A3%BC%EC%86%8C%EB%A1%9D"]) == "/주소록" | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "txt_path", | ||
| [ | ||
| "/literal%252Fsegment", | ||
| "/%252e%252e%252fescape", | ||
| "/safe%2525control", | ||
| ], | ||
| ) | ||
| def test_nested_percent_encoding_is_rejected(txt_path: str) -> None: | ||
| """Reject values whose meaning would change under a second decode pass.""" | ||
| assert _txt_context_path([f"path={txt_path}"]) is None | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("txt_path", ["/safe%", "/safe%2", "/safe%2G"]) | ||
| def test_malformed_percent_triplets_are_rejected(txt_path: str) -> None: | ||
| """Reject malformed URI percent encodings instead of forwarding ambiguity.""" | ||
| assert _txt_context_path([f"path={txt_path}"]) is None | ||
|
|
||
|
|
||
| def test_invalid_utf8_percent_octet_is_rejected() -> None: | ||
| """Reject invalid UTF-8 rather than accepting a replacement-character path.""" | ||
| assert _txt_context_path(["path=/safe%FF"]) is None | ||
|
|
||
|
|
||
| def test_encoded_literal_percent_is_preserved_after_one_decode() -> None: | ||
| """Allow a single encoded percent when it does not form another triplet.""" | ||
| assert _txt_context_path(["path=/discount-100%25"]) == "/discount-100%" |
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,5 @@ | ||
| from services.carddav_discovery import _txt_context_path | ||
|
|
||
|
|
||
| def test_txt_context_path_rejects_unicode_c1_control(): | ||
| assert _txt_context_path(["path=/safe%C2%85header"]) is None |
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.
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.
Uh oh!
There was an error while loading. Please reload this page.