fix(auth): fold /api/auth routes into Roaster authorization ($request?user)#814
Open
joewiz wants to merge 3 commits into
Open
fix(auth): fold /api/auth routes into Roaster authorization ($request?user)#814joewiz wants to merge 3 commits into
joewiz wants to merge 3 commits into
Conversation
…ribute
GET /api/auth/whoami and POST /api/auth/session resolved the current user
by reading request:get-attribute("org.exist.login.user"). That attribute
is populated only by the persistent-login flow (login-form params or the
remember-me cookie), so a request authenticated with the HTTP Basic
header reported as "guest" even though it executed as the real user:
before: curl -u admin: .../api/auth/whoami -> user=guest, isAdmin=false
after: curl -u admin: .../api/auth/whoami -> user=admin, isAdmin=true
Replace the attribute-based auth:get-user() with auth:current-user(),
which reads the actual eXist subject via sm:id() (sm:effective preferred,
matching the cookie/token case). This works because controller.xq calls
login:set-user before forwarding to the Roaster handlers, so by handler
time the subject already reflects whatever authenticated the request --
Basic header, the shared org.exist.login cookie (Path=/exist), or a
freshly minted login session.
This puts eXide's notion of "who is logged in" on the same sm:id() basis
as Roaster's rutil:getDBUser() and existdb-openapi -- the first step of
converging the stock apps onto a single identity model.
Verified: auth_spec 16/16; identity correct under Basic, cookie, and
unauthenticated, including with a temporary non-empty-password user
(confirming the fix is not an artifact of the default empty admin
password) and correctly distinguishing dba from non-dba accounts.
Beachhead only -- deliberate follow-ups (each its own verified step):
removing the security:[] overrides on the two /api/auth routes so they
read $request?user via Roaster's middleware; retiring the legacy
controller /login branch; the same sm:id() fix in documentation-next.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…?user) Removes the `security: []` overrides on POST /api/auth/session and GET /api/auth/whoami so they participate in Roaster's standard-authorization middleware like every other route, and reads identity from `$request?user` (populated via rutil:getDBUser() -> sm:id()) instead of the app-local auth:current-user() helper from the previous commit. Net effect is identical identity resolution (both are sm:id()-based), but the auth routes are no longer special-cased out of the framework: one identity path for all routes, and the login POST is now a cookie-auth route that Roaster's csrf:enforce can protect once x-csrf is enabled (currently a no-op in both apps — neither declares x-csrf; tracked separately). No CSRF behaviour change here: with no x-csrf config, csrf:enforce is inert, so removing security:[] from the login POST is safe. cookieAuth declares its cookie name (org.exist.login), so use-cookie-auth resolves correctly on these routes. Verified: identity correct under Basic, cookie, unauthenticated, and fresh login POST; full eXide suite 263 passing / 0 failing / 2 skipped. Stacked on fix/auth-identity-from-request-user (PR eXist-db#813); rebase onto develop once that merges. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
line-o
reviewed
Jun 8, 2026
| declare function auth:login($request as map(*)) { | ||
| let $user := auth:get-user() | ||
| let $user := $request?user | ||
| let $name := ($user?name, "guest")[1] |
Member
There was a problem hiding this comment.
I believe this fallback is neither necessary nor helpful
Member
Author
There was a problem hiding this comment.
[This response was co-authored with Claude Code. -Joe]
Agreed — removed in 6d1602a (both auth:login and auth:whoami).
You're right it's dead code: with security: [] gone, these routes now run through standard-authorization, so $request?user is always populated — the guest account (name = "guest") when unauthenticated, as the module note says — and $user?name is never empty. The ("guest")[1] branch could only ever fire on a genuinely-empty $request?user, which would be a regression worth surfacing rather than silently reporting "guest". Folding the routes back into the middleware is exactly what makes the fallback unnecessary, so good to drop it. Thanks for catching it.
line-o
reviewed
Jun 8, 2026
| declare function auth:whoami($request as map(*)) { | ||
| let $user := auth:get-user() | ||
| let $user := $request?user | ||
| let $name := ($user?name, "guest")[1] |
$request?user is populated by Roaster's standard-authorization for every route (= the guest account, name "guest", when unauthenticated), as the module doc states — so $user?name is always present and the ($user?name, "guest")[1] fallback is dead code. Folding these routes back into the auth middleware (this PR's change) is exactly what makes the fallback unnecessary; removing it also avoids masking a genuinely empty $request?user behind a silent "guest". Per @line-o's review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
[This PR was co-authored with Claude Code. -Joe]
Stacked on #813 (
fix/auth-identity-from-request-user). Targetsdevelop; rebase once #813 merges. Review #813 first.Summary
Removes the
security: []overrides onPOST /api/auth/sessionandGET /api/auth/whoamiso they participate in Roaster'sstandard-authorizationmiddleware like every other route, and reads identity from$request?userinstead of the app-localauth:current-user()helper that #813 introduced.Why
#813 fixed the identity source (sm:id() not the persistent-login attribute) but left the two auth routes opted out of the Roaster auth middleware via
security: [], with a local helper doing thesm:id()read. That override is itself the last bit of auth-route special-casing. Removing it means one identity path for every route —$request?user, populated byrutil:getDBUser()→sm:id()— matching existdb-openapi and the rest of the stack.Net identity resolution is unchanged (both are
sm:id()-based); this is convergence/cleanup, plus it makes the login POST a genuine cookie-auth route that Roaster'scsrf:enforcecan protect once CSRF is enabled.Safety
x-csrf, so Roaster'scsrf:enforceis inert — removingsecurity: []from the login POST triggers nothing. (Enabling CSRF protection is tracked as a separate, deliberate hardening step.)cookieAuthdeclares its cookie name (org.exist.login), souse-cookie-authresolves correctly on these routes.controller.xq'slogin:set-user(form params) before the forward; the handler only reports.Verification
$request?user.)Follow-ups (separate, tracked)
x-csrf: {same-origin: true}on cookie-authed writes in both apps (real CSRF gap today —csrf:enforceis a no-op without it). Needs a cy.request-Origin check first.controller.xq/loginbranch in favour of/api/auth/session.