Skip to content

Skip authenticated-only authorization checks for anonymous visitors #6256

Description

@bram-atmire

Summary

An anonymous request for an item page performs ~15 separate authz/authorizations/search/object feature checks, one HTTP request per feature. Roughly ten of them ask whether the visitor can perform an action that only an authenticated user could ever perform (edit, administer, create a version, manage groups, submit, ...). For a visitor who is not logged in, the answer to those is always "no," yet each still costs a round-trip and makes the backend re-resolve the visitor's groups and scan resource policies.

Because crawler and browser-impersonating scraper traffic is unauthenticated and dominates access load, this waste falls on exactly the requests that matter most for scaling.

This complements #3161 (obtain permissions in fewer REST requests). That issue proposes batching the checks; this one proposes not making the ones that can never succeed for an anonymous visitor. The two compose: fewer checks asked, and the remainder batched.

Evidence (measured)

Measured on a local DSpace 11 stack (Angular SSR + REST backend) with OpenTelemetry tracing, one anonymous item render:

Metric Before After skip
Backend REST calls (Contexts) 42 31
Authorization feature checks 15 5
Backend DB queries ~300 substantially fewer
Single-core SSR render throughput ~3.3 renders/s ~4.8 renders/s
Warm SSR render latency ~0.30s ~0.21s

Notes:

  • Only ~5 of the 42 calls serve the item's actual content; authorization is the single largest share of the backend's database load for the page.
  • The fan-out is the item page's data pattern, not a rendering-mode quirk: with SSR the Node server makes the calls; without SSR (or after hydration, navigating item to item) the browser makes the same calls. Both hit the backend.
  • The ~15 checks fire against the item and the Site object.

Proposed change (Angular, existing REST contract)

In AuthorizationDataService.isAuthorized(), when checking the current visitor for a feature that only makes sense once authenticated, answer false locally without a REST call whenever no one is logged in. Features an anonymous visitor can legitimately be granted are always checked normally.

Kept (still checked for anonymous): canDownload, canRequestACopy, canSendFeedback, epersonRegistration, epersonForgotPassword, canViewUsageStatistics.

Treated as authenticated-only (skipped for anonymous): the admin/manage set (administratorOf, isCollectionAdmin, isCommunityAdmin, canManageGroups, canManageGroup, canManagePolicies, canManageVersions, canManageMappings, canManageRelationships, canManageBitstreamBundles, loginOnBehalfOf, canRegisterDOI, canCreateVersion, canEditVersion, canDeleteVersion, canReplaceBitstreamAdmin), the login-required set (canClaimItem, canChangePassword, canSynchronizeWithORCID, canSubmit, canReplaceBitstreamSubmitter), the edit/write set (canEditItem, canEditMetadata, canDelete, canMove, canMakePrivate, withdrawItem, reinstateItem), and canSeeQA, canSubscribeDso, coarNotifyEnabled.

Two correctness points, both handled:

  • Safety: the frontend is not the security boundary. Skipping a check only avoids showing a control; the backend still enforces on any real action. The skipped features map to WRITE/ADMIN/login-only capabilities that are never granted to the ANONYMOUS group, so the skip returns the same "no" the server would.
  • No race for logged-in users: the check waits for isAuthenticationLoaded() before reading the auth state, so an authenticated admin is never transiently treated as anonymous while auth resolves.

Open design question for maintainers

The list above is a static classification of "features only an authenticated user can hold." That is true for every deployment I have seen, but the edit/write features check a policy the backend genuinely evaluates, so hardcoding the classification quietly steps outside the policy-driven model. Would you prefer:

  1. a static list, as drafted, or
  2. having each AuthorizationFeature declare whether it requires authentication (a property the UI reads), which is more principled but a larger change?

Related issues

Status

PR #6257 implements this: the skip, the isAuthenticationLoaded() fix, and unit tests (skip returns false with no request for anonymous, the check still runs for an authenticated user, kept features are never skipped, and nothing is decided before auth loads). The design question above is open for reviewer input on the PR.


Investigated and drafted with Claude Fable 5.1 (Anthropic), via Claude Code, on a local DSpace 11 stack with OpenTelemetry tracing. All figures are measured on that stack, not estimated.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

authorizationrelated to authorization, permissions or groupsbugimprovementperformance / cachingRelated to performance, caching or embedded objects

Type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions