Skip to content

Disambiguate 401 and 403 when accessing /projects#719

Merged
fspeirs merged 2 commits intomainfrom
disambiguate-401-403
Mar 10, 2026
Merged

Disambiguate 401 and 403 when accessing /projects#719
fspeirs merged 2 commits intomainfrom
disambiguate-401-403

Conversation

@fspeirs
Copy link
Copy Markdown
Contributor

@fspeirs fspeirs commented Mar 9, 2026

Previously, Editor-API would return a 403 error for any project access that failed, including accesses that should have received a 401 Unauthorized error.

This change checks the state of current_user to decide whether to return 401 or 403. This will then allow client applications like Experience CS to properly route the user to either log in and retry or present an error page with explanations as to why the 403 occurred.

Status

  • Related to RaspberryPiFoundation/experience-cs#1890

Points for consideration:

  • Security
    • It may appear as if this change now leaks the existence of a given project identifier to an unauthenticated user but they were being leaked anyway. Some APIs return 404 to unauthenticated users accessing specific record endpoints in order to prevent identifier enumeration, but editor-api never did this anyway.

What's changed?

  • Modification to api_controller.rb#denied to:
    • render status: :forbidden when the user is logged in but denied access
    • rebder status: :unauthorized when the user is NOT logged in and denied access.

Previously, both conditions would render :forbidden.

Steps to perform after deploying to production

Nothing required.

No changes are required in editor-ui either as 401 and 403 are already handled identically in loadProjectReducers.js in that codebase.

Copilot AI review requested due to automatic review settings March 9, 2026 13:49
@cla-bot cla-bot Bot added the cla-signed label Mar 9, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 9, 2026

Test coverage

90.01% line coverage reported by SimpleCov.
Run: https://github.com/RaspberryPiFoundation/editor-api/actions/runs/22895692571

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the API’s global CanCan::AccessDenied handling to return 401 Unauthorized for unauthenticated requests and 403 Forbidden for authenticated-but-denied requests, enabling clients to distinguish “needs login” vs “insufficient permission” flows (notably for /projects access).

Changes:

  • Adjust ApiController#denied to conditionally render 401 vs 403 based on current_user presence.
  • Update request specs to cover both authenticated and unauthenticated CanCan::AccessDenied scenarios and assert the new response bodies.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
app/controllers/api_controller.rb Changes CanCan::AccessDenied rescue behavior to return 401 when unauthenticated and 403 when authenticated.
spec/requests/api_controller_spec.rb Updates/extends request specs to validate the new 401/403 behavior and corresponding JSON error messages.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@fspeirs fspeirs marked this pull request as draft March 9, 2026 13:57
@fspeirs fspeirs marked this pull request as ready for review March 9, 2026 14:59
@zetter-rpf zetter-rpf requested a review from mwtrew March 9, 2026 16:03
fspeirs added 2 commits March 10, 2026 09:24
Previously, Editor-API would return a 403 error for any project access
that failed, including accesses that should have recieved a 401
Unauthorised error.

This change checks the state of `current_user` to decide whether to
return 401 or 403. This will then allow client applications like
Experience CS to properly route the user to either log in and retry or
present an error page with explanations as to why the 403 occurred.
This branch changes the response for not-logged-in users to
:unauthorized instead of :forbidden.
@fspeirs fspeirs force-pushed the disambiguate-401-403 branch from c557563 to 5377296 Compare March 10, 2026 09:24
@fspeirs fspeirs merged commit 1863b28 into main Mar 10, 2026
6 checks passed
@fspeirs fspeirs deleted the disambiguate-401-403 branch March 10, 2026 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants