feat(security): per-endpoint read/write scope classification + full service map (#3102)

[Ben-Heerema]

fixes #3102

Important

Stacked on #3094 (claude/issue-3083-...). #3102 builds on OAuthScopes, which only exists on #3094's branch, so this PR is based on it and targets develop. Until #3094 merges, the diff below includes #3094's commits — review only the single commit feat(security): per-endpoint read/write scope classification + full service map. After #3094 merges to develop, this diff collapses to just the #3102 change.

Problem

#3094 introduced default-OFF OAuth scope enforcement piloted on schedule/tickler, and derived read vs. write purely from the HTTP method. That over-required .write for read-only POST endpoints (e.g. tickler/search, schedule/getAppointment) and left ~31 of ~33 services unmapped (NO_SCOPE_REQUIRED). This PR is the #3102 follow-up: complete the map and make read/write per-endpoint.

What changed (one file + its test)

• DOMAIN_BY_PATH_ROOT now maps every JAX-RS service root under /ws/services to a scope domain (~33 roots → ~28 domains). Related roots share a domain: rx+rxlookup→rx, reporting+reportbytemplate→report, eform+eforms→eform, demographics(+/merge)→demographic. The /initiate vocabulary (KNOWN_SCOPES) is derived from the map, so all domains' .read/.write are now accepted.
• Per-endpoint read/write classification (fixes the cubic P2 from feat(security): enforce OAuth 1.0a scopes behind a feature flag (#3083) #3094): safe methods are reads; non-safe methods are writes except
  • NONSAFE_READ_ROOTS — services whose entire non-safe surface is read-only (measurements, recordUX, rxlookup, patientDetailStatusService); and
  • READ_OP_SEGMENTS_BY_ROOT — read-only POSTs identified by a path-segment marker per root (tickler/search, schedule/getAppointment+appointmentHistory, notes/.../all+getIssueNote+…, demographics/search, consults/searchRequests+searchResponses, reporting/getReport+patientList+runReport, dxRegisty/findLikeIssue, persona/hasRights+isAllowedAccessToPatientRecord+preferences, providerService/.../search, rx/.../print).
• The classification was built by reading every service's @GET/@POST/@PUT/@DELETE methods; ambiguous operations default to .write (the security-safe direction). Resolution mechanism is unchanged — still keyed off the container-canonical getPathInfo(). Enforcement remains gated by oauth.scope.enforcement.enabled (default off), so the map can be reviewed/corrected before it is ever enabled.

Out of scope (the third part of #3102): flipping the default to ON and the rollout/migration plan — left for a separate decision.

Tests

OAuthScopesUnitTest extended with: full-map domain resolution for newly-covered services, shared-domain roots, read-only-POST overrides (tickler/search → tickler.read, schedule/getAppointment → schedule.read, notes/123/all → note.read, consults/searchRequests → consultation.read, reporting/.../getReport → report.read), wholly-read-only services (measurements/123 → measurement.read, recordUX/searchTemplates → recordux.read, patientDetailStatusService/validateHC → patientstatus.read), mutating-POST writes, unmapped-root → NO_SCOPE_REQUIRED, and the expanded /initiate vocabulary.

Local verification

• mvn test-compile — BUILD SUCCESS.
• OAuthScopesUnitTest + OAuthInterceptorScopeEnforcementUnitTest + OscarRequestTokenServiceScopeValidationUnitTest → 29 tests, 0 failures.

🤖 Generated with Claude Code

---

Summary by cubic

Adds per-endpoint read/write scope classification and a complete service-to-domain map for all /ws/services, enabling accurate OAuth 1.0a scope checks behind oauth.scope.enforcement.enabled (default off). Also validates scopes at /ws/oauth/initiate and safely handles tokens with no scopes.

• New Features

  • OAuthScopes: maps every service root to a domain and classifies read/write per endpoint. Safe methods → read; non-safe → write, with POST-only read overrides matched by positional templates (wildcards for params) to prevent param-value escalation. Write implies read; unmapped roots return NO_SCOPE_REQUIRED. Note: Disease Registry root is intentionally keyed as dxregisty to match its @Path.
  • OAuthInterceptor: resolves required scope from HttpServletRequest.getPathInfo() and enforces it when the flag is on; rejects out-of-scope calls with 403 insufficient_scope. Non-POST or blank/null methods default to write for fail-safe handling.
  • OscarRequestTokenService: trims/splits requested scopes and, when the flag is on, rejects empty or unknown scopes at /initiate with 400 invalid_scope; remains lenient when off.

• Bug Fixes

  • OscarOAuthDataProvider: treats null/blank persisted scopes as an empty grant to fail closed (403) instead of throwing a 500.

^{Written for commit 1841680. Summary will update on new commits.}

Summary by Sourcery

Expand OAuth scope resolution to cover all JAX-RS services under /ws/services with per-endpoint read/write classification and update tests accordingly.

New Features:

• Map all service roots under /ws/services to OAuth scope domains and expose the full set of readable/writable scopes for clients.
• Classify OAuth read vs. write requirements per endpoint, allowing known read-only POST operations to require .read instead of .write scopes.

Enhancements:

• Treat certain service roots as wholly read-only on non-safe methods and use explicit path templates to safely identify read-only POST operations.
• Derive the /initiate scope vocabulary from the complete domain map so clients can request scopes for all mapped domains.

Tests:

• Extend OAuthScopesUnitTest to cover the expanded service-to-domain map, per-endpoint read/write classification, and the updated known-scope vocabulary.

[Ben-Heerema]

@coderabbitai review
@cubic-dev-ai review

[Ben-Heerema]

@coderabbitai review
@cubic-dev-ai review

[Ben-Heerema]

@coderabbitai review
@cubic-dev-ai review

[Ben-Heerema]

Note for future self: 3094 should merge before this!

[Ben-Heerema]

@yingbull this is waiting for PR 3094, please ping me here when that is merged. :)

[cubic-dev-ai]

No issues found across 2 files

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

Architecture diagram

sequenceDiagram
    participant Client as OAuth Client App
    participant Initiate as InitiateEndpoint /ws/oauth/initiate
    participant Interceptor as OAuthInterceptor (CXF Filter)
    participant Scopes as OAuthScopes Resolver
    participant Service as JAX-RS Service /ws/services/{root}/...
    participant Provider as OscarOAuthDataProvider

    Note over Client,Provider: OAuth 1.0a Scope Enforcement Flow (flag-gated)

    Client->>Initiate: POST /ws/oauth/initiate (request scopes)
    Initiate->>Scopes: trim/split scope string, isKnownScope(scope)
    alt flag=oauth.scope.enforcement.enabled
        Scopes-->>Initiate: known scope?
        Initiate-->>Client: 400 invalid_scope if unknown/empty
    else flag disabled
        Initiate-->>Client: lenient, accept any scopes
    end
    Client->>Interceptor: Request to /ws/services/{root}/... (with HTTP method + access token)

    Note over Interceptor,Service: NEW: Full service map & per-endpoint read/write

    Interceptor->>Interceptor: Extract path via getPathInfo(), lower-case root
    Interceptor->>Scopes: requiredScope(httpMethod, path)

    alt Root NOT in DOMAIN_BY_PATH_ROOT
        Scopes-->>Interceptor: NO_SCOPE_REQUIRED (null)
        note over Interceptor,Service: No enforcement, pass through
        Interceptor->>Service: Forward request (unrestricted)
        Service-->>Client: Response
    else Root mapped to domain
        alt Safe method (GET/HEAD/OPTIONS)
            Scopes-->>Interceptor: {domain}.read
        else Non-safe method (POST/PUT/DELETE)
            alt POST + path matches READ_OP_TEMPLATES_BY_ROOT or root in NONSAFE_READ_ROOTS
                note over Scopes: CHANGED: Read-only POSTs now classified as .read
                Scopes-->>Interceptor: {domain}.read
            else POST + no template match, or PUT/DELETE, or null/blank method
                note over Scopes: Ambiguous defaults to .write (secure direction)
                Scopes-->>Interceptor: {domain}.write
            end
        end
        alt flag=oauth.scope.enforcement.enabled
            Interceptor->>Provider: Validate token has required scope
            alt Token satisfies scope (write implies read)
                Provider-->>Interceptor: authorized
                Interceptor->>Service: Forward request
                Service-->>Client: Response (200/2xx)
            else Scope insufficient or null/blank persisted scopes
                note over Provider: CHANGED: null scopes → empty grant (fail closed)
                Provider-->>Interceptor: 403 insufficient_scope
                Interceptor-->>Client: 403 Forbidden
            end
        else flag disabled
            Interceptor->>Service: Forward request (no enforcement)
            Service-->>Client: Response
        end
    end

Loading

_{Re-trigger cubic}

[sonarqubecloud]

Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[coderabbitai]

@Ben-Heerema Thanks for the update — I'll review the latest changes in #3103 now that the diff has collapsed to the OAuthScopes.java + OAuthScopesUnitTest.java payload from the develop merge. Good to know the earlier findings (TRACE as a safe method, the HashSet import, full CARLOS headers) came in via #3094, and I'll note the duplicate access-token lookup in OAuthInterceptor as a deferred follow-up rather than re-flagging it here.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cubic-dev-ai]

No issues found across 2 files

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

Architecture diagram

sequenceDiagram
    participant Client as OAuth Client App
    participant Initiate as InitiateEndpoint /ws/oauth/initiate
    participant Interceptor as OAuthInterceptor (CXF Filter)
    participant Scopes as OAuthScopes Resolver
    participant Service as JAX-RS Service /ws/services/{root}/...
    participant Provider as OscarOAuthDataProvider

    Note over Client,Provider: OAuth 1.0a Scope Enforcement Flow (flag-gated)

    Client->>Initiate: POST /ws/oauth/initiate (request scopes)
    Initiate->>Scopes: trim/split scope string, isKnownScope(scope)
    alt flag=oauth.scope.enforcement.enabled
        Scopes-->>Initiate: known scope?
        Initiate-->>Client: 400 invalid_scope if unknown/empty
    else flag disabled
        Initiate-->>Client: lenient, accept any scopes
    end
    Client->>Interceptor: Request to /ws/services/{root}/... (with HTTP method + access token)

    Note over Interceptor,Service: NEW: Full service map & per-endpoint read/write

    Interceptor->>Interceptor: Extract path via getPathInfo(), lower-case root
    Interceptor->>Scopes: requiredScope(httpMethod, path)

    alt Root NOT in DOMAIN_BY_PATH_ROOT
        Scopes-->>Interceptor: NO_SCOPE_REQUIRED (null)
        note over Interceptor,Service: No enforcement, pass through
        Interceptor->>Service: Forward request (unrestricted)
        Service-->>Client: Response
    else Root mapped to domain
        alt Safe method (GET/HEAD/OPTIONS)
            Scopes-->>Interceptor: {domain}.read
        else Non-safe method (POST/PUT/DELETE)
            alt POST + path matches READ_OP_TEMPLATES_BY_ROOT or root in NONSAFE_READ_ROOTS
                note over Scopes: CHANGED: Read-only POSTs now classified as .read
                Scopes-->>Interceptor: {domain}.read
            else POST + no template match, or PUT/DELETE, or null/blank method
                note over Scopes: Ambiguous defaults to .write (secure direction)
                Scopes-->>Interceptor: {domain}.write
            end
        end
        alt flag=oauth.scope.enforcement.enabled
            Interceptor->>Provider: Validate token has required scope
            alt Token satisfies scope (write implies read)
                Provider-->>Interceptor: authorized
                Interceptor->>Service: Forward request
                Service-->>Client: Response (200/2xx)
            else Scope insufficient or null/blank persisted scopes
                note over Provider: CHANGED: null scopes → empty grant (fail closed)
                Provider-->>Interceptor: 403 insufficient_scope
                Interceptor-->>Client: 403 Forbidden
            end
        else flag disabled
            Interceptor->>Service: Forward request (no enforcement)
            Service-->>Client: Response
        end
    end

Loading

_{Re-trigger cubic}

[sourcery-ai]

Reviewer's Guide

Extends OAuth scope handling to cover all /ws/services JAX-RS roots with a domain map and introduces per-endpoint read/write classification, including explicit handling of read-only POSTs, along with unit tests that validate the new mapping, classification logic, and scope vocabulary.

Sequence diagram for OAuth scope resolution with per-endpoint read/write classification

sequenceDiagram
    actor Client
    participant HttpServletRequest
    participant OAuthInterceptor
    participant OAuthScopes

    Client->>HttpServletRequest: POST /ws/services/tickler/search
    HttpServletRequest-->>OAuthInterceptor: getMethod() = POST
    HttpServletRequest-->>OAuthInterceptor: getPathInfo() = /services/tickler/search

    OAuthInterceptor->>OAuthScopes: requiredScope("POST", "/services/tickler/search")
    activate OAuthScopes
    OAuthScopes->>OAuthScopes: serviceSegments("/services/tickler/search")
    OAuthScopes-->>OAuthScopes: segments = ["tickler", "search"]
    OAuthScopes->>OAuthScopes: DOMAIN_BY_PATH_ROOT.get("tickler")
    OAuthScopes-->>OAuthScopes: domain = "tickler"
    OAuthScopes->>OAuthScopes: isSafeMethod("POST")
    OAuthScopes-->>OAuthScopes: false
    OAuthScopes->>OAuthScopes: isPostMethod("POST")
    OAuthScopes-->>OAuthScopes: true
    OAuthScopes->>OAuthScopes: isNonSafeRead("tickler", ["tickler","search"])
    OAuthScopes->>OAuthScopes: READ_OP_TEMPLATES_BY_ROOT.get("tickler")
    OAuthScopes-->>OAuthScopes: templates = [["search"]]
    OAuthScopes->>OAuthScopes: matchesTemplate(["search"], ["search"])
    OAuthScopes-->>OAuthScopes: true
    OAuthScopes-->>OAuthInterceptor: "tickler.read"
    deactivate OAuthScopes

    OAuthInterceptor-->>Client: 200 OK (if token has tickler.read or tickler.write)

Loading

File-Level Changes

Change	Details	Files
Introduce a full domain map for all /ws/services roots and derive the /initiate scope vocabulary from it.	Replace the pilot-only PILOT_DOMAIN_BY_PATH_ROOT with DOMAIN_BY_PATH_ROOT that maps all known JAX-RS service roots to scope domains, including shared domains for related roots and intentional misspelling for dxregisty to match its @path. Update buildKnownScopes to generate KNOWN_SCOPES from DOMAIN_BY_PATH_ROOT so every mapped domain exposes .read and .write at /initiate. Clarify Javadoc to describe the new full-map behavior and that unmapped roots resolve to NO_SCOPE_REQUIRED while enforcement remains gated by a configuration flag.	src/main/java/io/github/carlos_emr/carlos/webserv/oauth/OAuthScopes.java
Classify required read/write scope per endpoint by combining HTTP method safety with explicit read-only POST handling.	Extend requiredScope to compute the domain from all path segments under /services/, then decide read vs write per endpoint: safe methods → read; non-safe methods → write unless a POST qualifies for a read override. Add NONSAFE_READ_ROOTS for services whose non-safe surfaces are read-only, and READ_OP_TEMPLATES_BY_ROOT for explicit read-only POST operations keyed by root and path templates, using positional segment matching with "*" wildcards. Introduce helper methods (serviceSegments, isPostMethod, isNonSafeRead, matchesTemplate, seg) to parse and normalize segments, detect POST, test read-only POST cases, and enforce strict template matching that prevents path-parameter values from forcing read classification. Change the path parsing helper from pathRootUnderServices to serviceSegments, returning a lower-cased list of segments after /services/ instead of a single root string.	src/main/java/io/github/carlos_emr/carlos/webserv/oauth/OAuthScopes.java
Expand unit tests to cover the full service map, per-endpoint classification, and updated known-scope vocabulary.	Adjust the unmapped-root test to assert NO_SCOPE_REQUIRED for /services/oauth/info and an arbitrary unknown root instead of treating demographics as unpiloted. Add a nested FullMapClassification test suite that validates domain mapping for newly covered services, shared domains across related roots, read-only POST overrides, mutating POSTs remaining writes, wholly read-only services, and guard cases where path-parameter values look like operation names but must not downgrade writes to reads. Add tests confirming that non-POST and null/blank methods are always treated as writes even on read-only services, and that newly mapped domains are accepted by isKnownScope as part of the /initiate vocabulary.	src/test/java/io/github/carlos_emr/carlos/webserv/oauth/OAuthScopesUnitTest.java

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#3102	Replace the HTTP-method-only heuristic with an explicit per-endpoint read/write classification so that read-only POST endpoints (e.g. tickler/search, schedule/getAppointment) require .read instead of .write.	✅
#3102	Define and wire a complete endpoint→scope map for the remaining JAX-RS services under /ws/services/* (beyond schedule/tickler), and ensure the /initiate scope vocabulary reflects all mapped domains.	✅
#3102	Roll out scope enforcement beyond the initial pilot: expand enforcement based on the new map and decide/document the path to flipping oauth.scope.enforcement.enabled to default ON, including migration/communication for existing tokens.	❌	The PR explicitly limits itself to computing required scopes (per-endpoint classification and full map) and keeps enforcement gated by the existing feature flag with default OFF. It notes that flipping the default to ON and defining the rollout/migration plan are out of scope and left for a separate decision, and there are no code or documentation changes implementing that rollout.

Possibly linked issues

• [Security] OAuth scope enforcement: per-endpoint read/write classification + complete the endpoint→scope map (follow-up to #3083) #3102: PR delivers the per-endpoint read/write logic and full /ws/services scope map the issue specifies, excluding rollout.
• #claude assist security issue (OAuth 1.0a scopes not enforced): PR implements detailed endpoint→scope mapping and read/write classification needed to enforce OAuth 1.0a scopes described in issue.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've left some high level feedback:

• The read/write overrides in NONSAFE_READ_ROOTS and READ_OP_TEMPLATES_BY_ROOT are fully string-based; consider adding a one-time startup/assertion check that all these roots exist in DOMAIN_BY_PATH_ROOT so any future typo or service rename fails fast rather than silently disabling an override.
• requiredScope repeatedly normalizes the HTTP method (e.g., in isSafeMethod and isPostMethod); you could normalize/trim/lowercase once up front and pass around the normalized value to simplify the logic and avoid duplicate work.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The read/write overrides in `NONSAFE_READ_ROOTS` and `READ_OP_TEMPLATES_BY_ROOT` are fully string-based; consider adding a one-time startup/assertion check that all these roots exist in `DOMAIN_BY_PATH_ROOT` so any future typo or service rename fails fast rather than silently disabling an override.
- `requiredScope` repeatedly normalizes the HTTP method (e.g., in `isSafeMethod` and `isPostMethod`); you could normalize/trim/lowercase once up front and pass around the normalized value to simplify the logic and avoid duplicate work.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[Ben-Heerema]

@yingbull ready for review