Skip to content

Commit eb3fa24

Browse files
matmannaskyfallwastaken
matmanna
and
skyfallwastaken
authored
feat: make regular api tokens revocable (#1027)
* add new icon from bounty * feat: add hackatime normal token revocation * chore: make linter not hate me (its always whitespace) <3 * fix: combine both revocation apis into one (as requested by mahad) * chore: add HKA_REVOCATION_KEY to .env.example * feat: add hackatime normal token revocation * chore: make linter not hate me (its always whitespace) <3 * fix: combine both revocation apis into one (as requested by mahad) * chore: add HKA_REVOCATION_KEY to .env.example * feat: add hackatime normal token revocation * chore: make linter not hate me (its always whitespace) <3 * fix: combine both revocation apis into one (as requested by mahad) * chore: add HKA_REVOCATION_KEY to .env.example * feat: add hackatime normal token revocation * chore: make linter not hate me (its always whitespace) <3 * fix: combine both revocation apis into one (as requested by mahad) * fix: stuff greptile suggested * style: add final newline * docs: apply .env.example suggestion from @skyfallwastaken Co-authored-by: Mahad Kalam <55807755+skyfallwastaken@users.noreply.github.com> * refactor: move apikey rotation to user model * style: remove unnecessary comment * fix: tests passing and inappropriate response codes * refactor: fix response codes * refactor: move key info request back into separate function * fix: broken ci because of merge mistake :/ * refactor: remove unnecessary test line and switch to report_error * fix: returned name for admin & regular keys --------- Co-authored-by: Mahad Kalam <55807755+skyfallwastaken@users.noreply.github.com>
1 parent e43cdc5 commit eb3fa24

8 files changed

Lines changed: 237 additions & 27 deletions

File tree

‎.env.example‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,6 @@ S3_ACCESS_KEY_ID=your_s3_access_key_id_here
6060
S3_SECRET_ACCESS_KEY=your_s3_secret_access_key_here
6161
S3_BUCKET=your_s3_bucket_name_here
6262
S3_ENDPOINT=https://<ACCOUNT_ID>.r2.cloudflarestorage.com
63+
64+
# Key for Revoker (https://github.com/hackclub/revoker)
65+
HKA_REVOCATION_KEY=your_hka_revocation_key_here

‎app/controllers/api/internal/revocations_controller.rb‎

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,60 @@
11
module Api
22
module Internal
33
class RevocationsController < Api::Internal::ApplicationController
4+
REGULAR_KEY_REGEX = /\A[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\z/i
5+
ADMIN_KEY_REGEX = /\Ahka_[0-9a-f]{64}\z/
6+
47
def create
58
token = params[:token]
69

7-
return head 400 unless token.present?
10+
return render_error("Token is required") unless token.present?
811

9-
admin_api_key = AdminApiKey.active.find_by(token:)
12+
key, user, token_type, token_format = find_key_info(token)
13+
return render_error("Token doesn't match any supported type") unless token_format
14+
return render_error("Token is invalid or already revoked") unless key.present?
15+
original_key_name = key.name
16+
return render_error("Token is invalid or already revoked") unless revoke_key(key)
1017

11-
return render json: { success: false } unless admin_api_key.present?
18+
response_payload = {
19+
success: true,
20+
status: "complete",
21+
token_type: token_type,
22+
owner_email: user.email_addresses.first&.email
23+
}
24+
response_payload[:key_name] = original_key_name if token_format == :admin
1225

13-
admin_api_key.revoke!
26+
render json: response_payload.compact_blank, status: :created
27+
end
1428

15-
user = admin_api_key.user
29+
private
1630

17-
render json: {
18-
success: true,
19-
owner_email: user.email_addresses.first&.email,
20-
key_name: admin_api_key.name
21-
}.compact_blank
31+
def find_key_info(token)
32+
if token.match?(ADMIN_KEY_REGEX)
33+
key = AdminApiKey.active.find_by(token:)
34+
return [ key, key&.user, key&.name, :admin ]
35+
end
36+
37+
if token.match?(REGULAR_KEY_REGEX)
38+
key = ApiKey.find_by(token:)
39+
return [ key, key&.user, key&.name, :regular ]
40+
end
41+
42+
[ nil, nil, nil, nil ]
43+
end
44+
45+
def revoke_key(key)
46+
if key.is_a?(AdminApiKey)
47+
key.revoke!
48+
else
49+
key.user.rotate_single_api_key!(key)
50+
end
51+
rescue ActiveRecord::ActiveRecordError => e
52+
report_error(e)
53+
false
54+
end
55+
56+
def render_error(message)
57+
render json: { success: false, error: message }, status: :unprocessable_entity
2258
end
2359

2460
private def authenticate!

‎app/controllers/settings/access_controller.rb‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,10 @@ def update
1414
end
1515

1616
def rotate_api_key
17-
@user.api_keys.transaction do
18-
@user.api_keys.destroy_all
17+
new_api_key = @user.rotate_api_keys!
1918

20-
new_api_key = @user.api_keys.create!(name: "Hackatime key")
21-
22-
PosthogService.capture(@user, "api_key_rotated")
23-
render json: { token: new_api_key.token }, status: :ok
24-
end
19+
PosthogService.capture(@user, "api_key_rotated")
20+
render json: { token: new_api_key.token }, status: :ok
2521
rescue => e
2622
report_error(e, message: "error rotate #{e.class.name}")
2723
render json: { error: "cant rotate" }, status: :unprocessable_entity

‎app/models/user.rb‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,20 @@ def create_email_signin_token(continue_param: nil)
286286
sign_in_tokens.create!(auth_type: :email, continue_param: continue_param)
287287
end
288288

289+
def rotate_api_keys!
290+
api_keys.transaction do
291+
api_keys.destroy_all
292+
api_keys.create!(name: "Hackatime key")
293+
end
294+
end
295+
296+
def rotate_single_api_key!(api_key)
297+
raise ActiveRecord::RecordNotFound unless api_key.user_id == id
298+
299+
api_key.update!(token: SecureRandom.uuid_v4)
300+
api_key
301+
end
302+
289303
def find_valid_token(token)
290304
sign_in_tokens.valid.find_by(token: token)
291305
end

‎spec/requests/api/internal/internal_spec.rb‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,19 @@
1212
parameter name: :payload, in: :body, schema: {
1313
type: :object,
1414
properties: {
15-
token: { type: :string }
15+
token: { type: :string },
16+
submitter: { type: :string },
17+
comment: { type: :string }
1618
},
1719
required: [ 'token' ]
1820
}
1921

20-
response(200, 'successful') do
22+
response(201, 'created') do
2123
let(:Authorization) { "Bearer test_revocation_key" }
22-
let(:payload) { { token: 'some_token' } }
24+
let(:user) { User.create!(timezone: "UTC") }
25+
let!(:email_address) { user.email_addresses.create!(email: "internal@example.com", source: :signing_in) }
26+
let!(:api_key) { user.api_keys.create!(name: "Desktop") }
27+
let(:payload) { { token: api_key.token } }
2328

2429
before do
2530
ENV["HKA_REVOCATION_KEY"] = "test_revocation_key"
@@ -32,15 +37,25 @@
3237
schema type: :object,
3338
properties: {
3439
success: { type: :boolean },
40+
status: { type: :string },
41+
token_type: { type: :string },
3542
owner_email: { type: :string, nullable: true },
3643
key_name: { type: :string, nullable: true }
3744
}
38-
run_test!
45+
run_test! do |response|
46+
body = JSON.parse(response.body)
47+
48+
expect(body["success"]).to eq(true)
49+
expect(body["status"]).to eq("complete")
50+
expect(body["token_type"]).to eq("Hackatime API Key")
51+
expect(body["owner_email"]).to eq(email_address.email)
52+
expect(body["key_name"]).to eq(api_key.name)
53+
end
3954
end
4055

41-
response(400, 'bad request') do
56+
response(422, 'unprocessable entity') do
4257
let(:Authorization) { "Bearer test_revocation_key" }
43-
let(:payload) { { token: nil } }
58+
let(:payload) { { token: SecureRandom.uuid_v4 } }
4459

4560
before do
4661
ENV["HKA_REVOCATION_KEY"] = "test_revocation_key"
@@ -50,6 +65,12 @@
5065
ENV.delete("HKA_REVOCATION_KEY")
5166
end
5267

68+
schema type: :object,
69+
properties: {
70+
success: { type: :boolean },
71+
error: { type: :string }
72+
},
73+
required: [ 'success', 'error' ]
5374
run_test!
5475
end
5576
end

‎swagger/v1/swagger.yaml‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1905,23 +1905,39 @@ paths:
19051905
- InternalToken: []
19061906
parameters: []
19071907
responses:
1908-
'200':
1909-
description: successful
1908+
'201':
1909+
description: created
19101910
content:
19111911
application/json:
19121912
schema:
19131913
type: object
19141914
properties:
19151915
success:
19161916
type: boolean
1917+
status:
1918+
type: string
1919+
token_type:
1920+
type: string
19171921
owner_email:
19181922
type: string
19191923
nullable: true
19201924
key_name:
19211925
type: string
19221926
nullable: true
1923-
'400':
1924-
description: bad request
1927+
'422':
1928+
description: unprocessable entity
1929+
content:
1930+
application/json:
1931+
schema:
1932+
type: object
1933+
properties:
1934+
success:
1935+
type: boolean
1936+
error:
1937+
type: string
1938+
required:
1939+
- success
1940+
- error
19251941
requestBody:
19261942
content:
19271943
application/json:
@@ -1930,6 +1946,10 @@ paths:
19301946
properties:
19311947
token:
19321948
type: string
1949+
submitter:
1950+
type: string
1951+
comment:
1952+
type: string
19331953
required:
19341954
- token
19351955
"/api/summary":

‎test/controllers/api/internal/revocations_controller_test.rb‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
require "test_helper"
2+
3+
class Api::Internal::RevocationsControllerTest < ActionDispatch::IntegrationTest
4+
setup do
5+
@previous_revocation_key = ENV["HKA_REVOCATION_KEY"]
6+
ENV["HKA_REVOCATION_KEY"] = "test-revocation-key"
7+
end
8+
9+
teardown do
10+
ENV["HKA_REVOCATION_KEY"] = @previous_revocation_key
11+
end
12+
13+
test "revokes regular ApiKey by rolling token" do
14+
user = User.create!(timezone: "UTC")
15+
email_address = user.email_addresses.create!(email: "regular@example.com", source: :signing_in)
16+
original_token = SecureRandom.uuid_v4
17+
key = user.api_keys.create!(name: "Desktop", token: original_token)
18+
19+
post "/api/internal/revoke", params: { token: original_token }, headers: auth_headers, as: :json
20+
21+
assert_response :created
22+
assert_equal true, response.parsed_body["success"]
23+
assert_equal "complete", response.parsed_body["status"]
24+
assert_equal key.name, response.parsed_body["token_type"]
25+
assert_equal email_address.email, response.parsed_body["owner_email"]
26+
assert_not_includes response.parsed_body.keys, "key_name"
27+
28+
key.reload
29+
assert_not_equal original_token, key.token
30+
assert_nil ApiKey.find_by(token: original_token)
31+
32+
post "/api/internal/revoke", params: { token: original_token }, headers: auth_headers, as: :json
33+
34+
assert_response :unprocessable_entity
35+
assert_equal false, response.parsed_body["success"]
36+
assert_equal "Token is invalid or already revoked", response.parsed_body["error"]
37+
end
38+
39+
test "returns success false for valid regular UUID token that does not exist" do
40+
token = SecureRandom.uuid_v4
41+
42+
post "/api/internal/revoke", params: { token: token }, headers: auth_headers, as: :json
43+
44+
assert_response :unprocessable_entity
45+
assert_equal false, response.parsed_body["success"]
46+
assert_equal "Token is invalid or already revoked", response.parsed_body["error"]
47+
end
48+
49+
test "returns success false for token that matches neither regex" do
50+
post "/api/internal/revoke", params: { token: "not-a-valid-token" }, headers: auth_headers, as: :json
51+
52+
assert_response :unprocessable_entity
53+
assert_equal false, response.parsed_body["success"]
54+
assert_equal "Token doesn't match any supported type", response.parsed_body["error"]
55+
end
56+
57+
test "revokes admin key" do
58+
user = User.create!(timezone: "UTC")
59+
email_address = user.email_addresses.create!(email: "admin@example.com", source: :signing_in)
60+
admin_key = user.admin_api_keys.create!(name: "Infra", token: "hka_#{SecureRandom.hex(32)}")
61+
62+
post "/api/internal/revoke", params: { token: admin_key.token }, headers: auth_headers, as: :json
63+
64+
assert_response :created
65+
assert_equal true, response.parsed_body["success"]
66+
assert_equal "complete", response.parsed_body["status"]
67+
assert_equal "Infra", response.parsed_body["token_type"]
68+
69+
admin_key.reload
70+
assert_equal email_address.email, response.parsed_body["owner_email"]
71+
assert_equal "Infra", response.parsed_body["key_name"]
72+
assert_not_nil admin_key.revoked_at
73+
assert_includes admin_key.name, "_revoked_"
74+
end
75+
76+
test "returns error for already-revoked admin key" do
77+
user = User.create!(timezone: "UTC")
78+
original_token = "hka_#{SecureRandom.hex(32)}"
79+
admin_key = user.admin_api_keys.create!(name: "Infra", token: original_token)
80+
admin_key.revoke!
81+
82+
post "/api/internal/revoke", params: { token: original_token }, headers: auth_headers, as: :json
83+
84+
assert_response :unprocessable_entity
85+
assert_equal false, response.parsed_body["success"]
86+
assert_equal "Token is invalid or already revoked", response.parsed_body["error"]
87+
end
88+
89+
private
90+
91+
def auth_headers
92+
{
93+
"Authorization" => ActionController::HttpAuthentication::Token.encode_credentials(ENV.fetch("HKA_REVOCATION_KEY"))
94+
}
95+
end
96+
end

‎test/models/user_test.rb‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,30 @@ class UserTest < ActiveSupport::TestCase
3030
assert_equal "gruvbox_dark", metadata[:value]
3131
end
3232

33+
test "rotate_api_keys! replaces existing api key with a new one" do
34+
user = User.create!(timezone: "UTC", slack_uid: "U#{SecureRandom.hex(8)}")
35+
user.api_keys.create!(name: "Original key")
36+
original_token = user.api_keys.first.token
37+
38+
new_api_key = user.rotate_api_keys!
39+
40+
assert_equal user.id, new_api_key.user_id
41+
assert_equal "Hackatime key", new_api_key.name
42+
assert_nil ApiKey.find_by(token: original_token)
43+
end
44+
45+
test "rotate_api_keys! creates a key when none exists" do
46+
user = User.create!(timezone: "UTC", slack_uid: "U#{SecureRandom.hex(8)}")
47+
48+
assert_equal 0, user.api_keys.count
49+
50+
new_api_key = user.rotate_api_keys!
51+
52+
assert_equal user.id, new_api_key.user_id
53+
assert_equal "Hackatime key", new_api_key.name
54+
assert_equal [ new_api_key.id ], user.api_keys.reload.pluck(:id)
55+
end
56+
3357
test "flipper id uses the user id" do
3458
user = User.create!(timezone: "UTC")
3559

0 commit comments

Comments
 (0)