Skip to content

Commit 61efb37

Browse files
committed
Deny access to methods other than GET for readonly access tokens
1 parent ce59128 commit 61efb37

File tree

5 files changed

+135
-2
lines changed

5 files changed

+135
-2
lines changed

app/controllers/application_controller.rb

+4-2
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ def authenticate_using_access_tokens
4141
if request.headers["X-Api-Token"].present?
4242
token = request.headers["X-Api-Token"]
4343
@access_token = AccessToken.active.find_by_token_digest(Digest::SHA256.hexdigest(token))
44-
if @access_token.present?
44+
if @access_token.present? && AccessTokenPolicy.new(@access_token, request).request?
4545
@access_token.update!(last_accessed_at: Time.zone.now)
4646
true
4747
else
@@ -50,7 +50,9 @@ def authenticate_using_access_tokens
5050
else
5151
authenticate_with_http_token do |token|
5252
@access_token = AccessToken.active.find_by_token_digest(Digest::SHA256.hexdigest(token))
53-
@access_token.update!(last_accessed_at: Time.zone.now) if @access_token.present?
53+
return unless @access_token.present? && AccessTokenPolicy.new(@access_token, request).request?
54+
55+
@access_token.update!(last_accessed_at: Time.zone.now)
5456
end
5557
end
5658
end

app/policies/access_token_policy.rb

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
class AccessTokenPolicy
2+
attr_reader :access_token, :request
3+
4+
def initialize(access_token, request)
5+
@access_token = access_token
6+
@request = request
7+
end
8+
9+
def request?
10+
access_token.all_permissions? || request.get?
11+
end
12+
end

spec/integration/access_tokens_spec.rb

+47
Original file line numberDiff line numberDiff line change
@@ -47,5 +47,52 @@
4747

4848
expect(response).to have_http_status(:unauthorized)
4949
end
50+
51+
context "when a user has a readonly token" do
52+
let(:access) do
53+
AccessToken.new(owner: :test, permissions: :readonly)
54+
end
55+
56+
let(:headers) do
57+
{ Authorization: "Token #{token}" }
58+
end
59+
60+
it "allows access to the API for GET requests" do
61+
get(forms_path, headers:)
62+
63+
expect(response).to have_http_status(:ok)
64+
end
65+
66+
it "denies access to the API for POST requests" do
67+
post(forms_path, params: { form: { name: "test form" } }, headers:)
68+
69+
expect(response).to have_http_status(:unauthorized)
70+
expect(Form.last).to be nil
71+
end
72+
73+
it "denies access to the API for PUT requests" do
74+
form = create :form, id: 1, name: "test form"
75+
76+
put(form_path(1), params: { form: { name: "edited test form" } }, headers:)
77+
78+
expect(response).to have_http_status(:unauthorized)
79+
expect(form.name).to eq "test form"
80+
end
81+
82+
it "denies access to the API for PATCH requests" do
83+
form = create :form, id: 1, name: "test form"
84+
85+
patch(form_path(1), params: { form: { name: "edited test form" } }, headers:)
86+
87+
expect(response).to have_http_status(:unauthorized)
88+
expect(form.name).to eq "test form"
89+
end
90+
91+
it "does not allow creating other access tokens" do
92+
post(access_tokens_path, params: { owner: "test" }, headers:)
93+
94+
expect(response).to have_http_status(:unauthorized)
95+
end
96+
end
5097
end
5198
end
+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
require "rails_helper"
2+
3+
RSpec.describe AccessTokenPolicy do
4+
subject(:policy) { described_class.new(access, request) }
5+
6+
let(:access) do
7+
build :access_token
8+
end
9+
10+
let(:request) do
11+
ActionDispatch::Request.empty
12+
end
13+
14+
describe "#request?" do
15+
before do
16+
request.headers["REQUEST_METHOD"] = request_method
17+
end
18+
19+
context "when request method is GET" do
20+
let(:request_method) { "GET" }
21+
22+
it "grants access if access token has all permissions" do
23+
access.permissions = :all
24+
25+
expect(policy.request?).to be true
26+
end
27+
28+
it "grants access if access token has readonly permissions" do
29+
access.permissions = :readonly
30+
31+
expect(policy.request?).to be true
32+
end
33+
end
34+
35+
(ActionDispatch::Request::HTTP_METHODS - %w[GET]).each do |request_method_|
36+
context "when request method is #{request_method_}" do
37+
let(:request_method) { request_method_ }
38+
39+
it "grants access if access token has all permissions" do
40+
access.permissions = :all
41+
42+
expect(policy.request?).to be true
43+
end
44+
45+
it "denies access if access token has readonly permissions" do
46+
access.permissions = :readonly
47+
48+
expect(policy.request?).to be false
49+
end
50+
end
51+
end
52+
end
53+
end

spec/request/application_controller_spec.rb

+19
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,25 @@
156156
expect(json_body[:status]).to eq("unauthorised")
157157
end
158158
end
159+
160+
context "when token with readonly permissions is used for update method" do
161+
let(:access_token) { AccessToken.new(owner: "test-owner", permissions: :readonly) }
162+
163+
before do
164+
access_token
165+
token
166+
access_token.save!
167+
post forms_path, params: { form: { name: "Test form" } }, headers: req_headers
168+
end
169+
170+
it "returns 401" do
171+
expect(response.status).to eq(401)
172+
end
173+
174+
it "returns an error message" do
175+
expect(json_body[:status]).to eq("unauthorised")
176+
end
177+
end
159178
end
160179
end
161180
end

0 commit comments

Comments
 (0)