Repository navigation
Conversation
… session The Studio UI learned about a missing session from a 401 on user/current-user-information. GET /session answers 200 with authenticated true/false (and the pending two-factor step); the bundle opens it with a prepended PUBLIC_ACCESS rule. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Symfony does not let a bundle add to security.access_control (the node cannot be overwritten), so a compiler pass puts the PUBLIC_ACCESS rule for <url_prefix>/session in front of the project's rules. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…he API smoke test SessionStatusService holds the logic (user role from UserPermissions), the path rule ends in \z, the upgrade note says the IP/https rules do not apply to the route, and the Postman smoke test calls /session with and without a session through the real firewall. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The intentional override of application access restrictions needs maintainer security review, with real-firewall two-factor and custom-prefix verification still outstanding.
No open issues 🎉
What changed in this PR
Verdict: Request maintainer security review.
Adds a session-status endpoint so the Studio login page can check authentication without relying on a failing protected request.
Changes:
- Adds a non-cacheable
GET /sessionresponse with authentication and pending two-factor status. - Registers a compiler pass that grants public access ahead of application access rules.
- Adds unit tests, Postman smoke requests, and API/upgrade documentation.
Review assessment:
- Root cause and caller coverage: The dedicated probe separates session discovery from protected user information (
src/Authorization/Controller/SessionController.php:42–57). Removing the login-page failure still depends on the companion UI change, which is outside this diff. - Boundary: Session-state logic belongs to the service; the controller handles serialization and cache headers (
src/Authorization/Service/SessionStatusService.php:41–48,src/Authorization/Controller/SessionController.php:54–59). - Compatibility: The endpoint is additive, but its public-access rule intentionally overrides application restrictions for the matching path (
src/DependencyInjection/CompilerPass/SessionAccessPass.php:45–53). - Regression coverage: Unit tests cover status states, response caching, and access matching; smoke requests cover anonymous and authenticated requests (
tests/Unit/Authorization/Service/SessionStatusServiceTest.php:29–63,tests/Unit/Authorization/Controller/SessionControllerTest.php:30–43,tests/Unit/DependencyInjection/CompilerPass/SessionAccessPassTest.php:32–63,tests/Api/postman/collections/Smoke Test.json:13–24,349–360). Tests were not independently run here. - Documentation: Upgrade notes explain the route and restriction override (
doc/02_Installation_and_Configuration/05_Upgrade.md:6–13). - Remaining risks: The access-map override warrants maintainer security review. Real-firewall two-factor and custom-prefix behavior remain unverified according to the PR description.
| File | Description |
|---|---|
translations/studio_api_docs.en.yaml |
Adds session endpoint documentation strings. |
tests/Unit/DependencyInjection/CompilerPass/SessionAccessPassTest.php |
Tests public access, path boundaries, and custom prefixes. |
tests/Unit/Authorization/Service/SessionStatusServiceTest.php |
Tests anonymous, authenticated, and two-factor states. |
tests/Unit/Authorization/Controller/SessionControllerTest.php |
Tests HTTP 200 responses and cache prevention. |
tests/Api/postman/collections/Smoke Test.json |
Adds anonymous and authenticated session probes. |
src/PimcoreStudioBackendBundle.php |
Registers the session access compiler pass. |
src/DependencyInjection/CompilerPass/SessionAccessPass.php |
Prepends the public session access rule. |
src/Authorization/Service/SessionStatusServiceInterface.php |
Defines the session-status service contract. |
src/Authorization/Service/SessionStatusService.php |
Derives authentication and pending two-factor status. |
src/Authorization/Schema/SessionStatus.php |
Defines the session response schema. |
src/Authorization/Controller/SessionController.php |
Exposes the non-cacheable session endpoint. |
doc/02_Installation_and_Configuration/05_Upgrade.md |
Documents the endpoint and security implications. |
config/authorization.yaml |
Wires the session-status service interface. |
Review effort: Balanced
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.



What
Adds
GET /pimcore-studio/api/session({"authenticated": true|false}, plus the two-factor step of a login that waits for its code). It answers 200 for every request that passes the general rate limit (429 above it), reveals only the caller's own session state, is not cacheable, and is opened to requests without a session by a compiler pass (SessionAccessPass) that puts aPUBLIC_ACCESSrule for<url_prefix>/sessionin front of the project'saccess_controlrules. Projects do not have to change theirsecurity.yaml, also not with a customurl_prefix.user/current-user-informationis unchanged. The failed request disappears from the login page only together with the UI companion PR. The rule is added without host, IP or channel, so a project's IP or https restriction does not apply to this one route.Why
The Studio login page learned that nobody is logged in from a 401 on
user/current-user-information, which the browser shows as a failed request. The UI will ask the new route instead (companion PR in pimcore/studio-ui-bundle,improvement/19159-session-probe-ui; it merges after this one). A prependedsecurity.access_controlis not possible: Symfony refuses a second definition of that node, hence the compiler pass.Fixes pimcore/pimcore#19159
Verification
SessionControllerTest(2),SessionStatusServiceTest(4) andSessionAccessPassTest(3, incl./session%0Astays protected) fail on base 5d5446a (classes do not exist) and pass on 02f52af. Neighbouring Authorization and Security suites pass.GET /sessionwithout and with a session through the real firewall (not run here; theapi-testworkflow runs it on a schedule against a remote environment, not on pull requests). On dev-2026.x the route answers 200 without a session,/session/and/session/xstay protected/404, POST gives 405.pimcore-pimcore-19159red on base (401 on the login page,/session404), green on 02f52af + UI f1fed400e (no 401,/session200authenticated:falseandtrueafter a login); the login still reaches Studio.Not verified
url_prefixon a running system (unit test of the pass only).api-testworkflow, so no pull request check sendsGET /sessionthrough the real firewall.Not verified (rating of 05c92c0)