Skip to content

Commit 1863b28

Browse files
authored
Disambiguate 401 and 403 when accessing /projects (#719)
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](https://github.com/RaspberryPiFoundation/editor-ui/blob/eed2940b52f6e1d75e0f0c0bd8b2e36561ec2999/src/redux/reducers/loadProjectReducers.js#L40) in `loadProjectReducers.js` in that codebase.
1 parent e4b72f8 commit 1863b28

3 files changed

Lines changed: 38 additions & 12 deletions

File tree

app/controllers/api_controller.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,12 @@ def authorize_user
2323
render json: { error: 'Unauthorized' }, status: :unauthorized unless current_user
2424
end
2525

26-
def denied(exception)
27-
render_error_as_json(exception, :forbidden)
26+
def denied(_exception)
27+
if current_user
28+
render json: { error: 'Forbidden' }, status: :forbidden
29+
else
30+
render json: { error: 'Unauthorized' }, status: :unauthorized
31+
end
2832
end
2933

3034
def not_found(exception)

spec/requests/api_controller_spec.rb

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,18 +76,40 @@ def index
7676
test_controller.error = CanCan::AccessDenied.new('foo')
7777
end
7878

79-
it 'responds with 403 Forbidden status code' do
80-
get '/test'
79+
context 'when current_user is not set' do
80+
it 'responds with 401 Unauthorized status code' do
81+
get '/test'
82+
83+
expect(response).to have_http_status(:unauthorized)
84+
end
85+
86+
it 'responds with JSON including error message' do
87+
get '/test'
8188

82-
expect(response).to have_http_status(:forbidden)
89+
expect(response.parsed_body).to include(
90+
'error' => 'Unauthorized'
91+
)
92+
end
8393
end
8494

85-
it 'responds with JSON including exception class & message' do
86-
get '/test'
95+
context 'when current_user is set' do
96+
before do
97+
allow(User).to receive(:from_token).and_return(User.new)
98+
end
8799

88-
expect(response.parsed_body).to include(
89-
'error' => 'CanCan::AccessDenied: foo'
90-
)
100+
it 'responds with 403 Forbidden status code' do
101+
get '/test', headers: { 'Authorization' => 'secret' }
102+
103+
expect(response).to have_http_status(:forbidden)
104+
end
105+
106+
it 'responds with JSON including error message' do
107+
get '/test', headers: { 'Authorization' => 'secret' }
108+
109+
expect(response.parsed_body).to include(
110+
'error' => 'Forbidden'
111+
)
112+
end
91113
end
92114
end
93115

spec/requests/projects/show_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,10 +254,10 @@
254254
}.to_json
255255
end
256256

257-
it 'returns forbidden response' do
257+
it 'returns unauthorized response' do
258258
get("/api/projects/#{project.identifier}", headers:)
259259

260-
expect(response).to have_http_status(:forbidden)
260+
expect(response).to have_http_status(:unauthorized)
261261
end
262262

263263
it 'does not return the project json' do

0 commit comments

Comments
 (0)