diff --git a/.env.example b/.env.example index 0192c6534..2ace1c762 100644 --- a/.env.example +++ b/.env.example @@ -60,3 +60,6 @@ S3_ACCESS_KEY_ID=your_s3_access_key_id_here S3_SECRET_ACCESS_KEY=your_s3_secret_access_key_here S3_BUCKET=your_s3_bucket_name_here S3_ENDPOINT=https://.r2.cloudflarestorage.com + +# Key for Revoker (https://github.com/hackclub/revoker) +HKA_REVOCATION_KEY=your_hka_revocation_key_here diff --git a/app/controllers/api/internal/revocations_controller.rb b/app/controllers/api/internal/revocations_controller.rb index c42f06e84..5598052f8 100644 --- a/app/controllers/api/internal/revocations_controller.rb +++ b/app/controllers/api/internal/revocations_controller.rb @@ -1,24 +1,60 @@ module Api module Internal class RevocationsController < Api::Internal::ApplicationController + 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 + ADMIN_KEY_REGEX = /\Ahka_[0-9a-f]{64}\z/ + def create token = params[:token] - return head 400 unless token.present? + return render_error("Token is required") unless token.present? - admin_api_key = AdminApiKey.active.find_by(token:) + key, user, token_type, token_format = find_key_info(token) + return render_error("Token doesn't match any supported type") unless token_format + return render_error("Token is invalid or already revoked") unless key.present? + original_key_name = key.name + return render_error("Token is invalid or already revoked") unless revoke_key(key) - return render json: { success: false } unless admin_api_key.present? + response_payload = { + success: true, + status: "complete", + token_type: token_type, + owner_email: user.email_addresses.first&.email + } + response_payload[:key_name] = original_key_name if token_format == :admin - admin_api_key.revoke! + render json: response_payload.compact_blank, status: :created + end - user = admin_api_key.user + private - render json: { - success: true, - owner_email: user.email_addresses.first&.email, - key_name: admin_api_key.name - }.compact_blank + def find_key_info(token) + if token.match?(ADMIN_KEY_REGEX) + key = AdminApiKey.active.find_by(token:) + return [ key, key&.user, key&.name, :admin ] + end + + if token.match?(REGULAR_KEY_REGEX) + key = ApiKey.find_by(token:) + return [ key, key&.user, key&.name, :regular ] + end + + [ nil, nil, nil, nil ] + end + + def revoke_key(key) + if key.is_a?(AdminApiKey) + key.revoke! + else + key.user.rotate_single_api_key!(key) + end + rescue ActiveRecord::ActiveRecordError => e + report_error(e) + false + end + + def render_error(message) + render json: { success: false, error: message }, status: :unprocessable_entity end private def authenticate! diff --git a/app/controllers/settings/access_controller.rb b/app/controllers/settings/access_controller.rb index 22c6ab419..070156e0b 100644 --- a/app/controllers/settings/access_controller.rb +++ b/app/controllers/settings/access_controller.rb @@ -14,14 +14,10 @@ def update end def rotate_api_key - @user.api_keys.transaction do - @user.api_keys.destroy_all + new_api_key = @user.rotate_api_keys! - new_api_key = @user.api_keys.create!(name: "Hackatime key") - - PosthogService.capture(@user, "api_key_rotated") - render json: { token: new_api_key.token }, status: :ok - end + PosthogService.capture(@user, "api_key_rotated") + render json: { token: new_api_key.token }, status: :ok rescue => e report_error(e, message: "error rotate #{e.class.name}") render json: { error: "cant rotate" }, status: :unprocessable_entity diff --git a/app/models/user.rb b/app/models/user.rb index b165c8dfd..314326558 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -286,6 +286,20 @@ def create_email_signin_token(continue_param: nil) sign_in_tokens.create!(auth_type: :email, continue_param: continue_param) end + def rotate_api_keys! + api_keys.transaction do + api_keys.destroy_all + api_keys.create!(name: "Hackatime key") + end + end + + def rotate_single_api_key!(api_key) + raise ActiveRecord::RecordNotFound unless api_key.user_id == id + + api_key.update!(token: SecureRandom.uuid_v4) + api_key + end + def find_valid_token(token) sign_in_tokens.valid.find_by(token: token) end diff --git a/spec/requests/api/internal/internal_spec.rb b/spec/requests/api/internal/internal_spec.rb index d432b4f5e..ff128142e 100644 --- a/spec/requests/api/internal/internal_spec.rb +++ b/spec/requests/api/internal/internal_spec.rb @@ -12,14 +12,19 @@ parameter name: :payload, in: :body, schema: { type: :object, properties: { - token: { type: :string } + token: { type: :string }, + submitter: { type: :string }, + comment: { type: :string } }, required: [ 'token' ] } - response(200, 'successful') do + response(201, 'created') do let(:Authorization) { "Bearer test_revocation_key" } - let(:payload) { { token: 'some_token' } } + let(:user) { User.create!(timezone: "UTC") } + let!(:email_address) { user.email_addresses.create!(email: "internal@example.com", source: :signing_in) } + let!(:api_key) { user.api_keys.create!(name: "Desktop") } + let(:payload) { { token: api_key.token } } before do ENV["HKA_REVOCATION_KEY"] = "test_revocation_key" @@ -32,15 +37,25 @@ schema type: :object, properties: { success: { type: :boolean }, + status: { type: :string }, + token_type: { type: :string }, owner_email: { type: :string, nullable: true }, key_name: { type: :string, nullable: true } } - run_test! + run_test! do |response| + body = JSON.parse(response.body) + + expect(body["success"]).to eq(true) + expect(body["status"]).to eq("complete") + expect(body["token_type"]).to eq("Hackatime API Key") + expect(body["owner_email"]).to eq(email_address.email) + expect(body["key_name"]).to eq(api_key.name) + end end - response(400, 'bad request') do + response(422, 'unprocessable entity') do let(:Authorization) { "Bearer test_revocation_key" } - let(:payload) { { token: nil } } + let(:payload) { { token: SecureRandom.uuid_v4 } } before do ENV["HKA_REVOCATION_KEY"] = "test_revocation_key" @@ -50,6 +65,12 @@ ENV.delete("HKA_REVOCATION_KEY") end + schema type: :object, + properties: { + success: { type: :boolean }, + error: { type: :string } + }, + required: [ 'success', 'error' ] run_test! end end diff --git a/swagger/v1/swagger.yaml b/swagger/v1/swagger.yaml index 3c986a2ea..984fe24d5 100644 --- a/swagger/v1/swagger.yaml +++ b/swagger/v1/swagger.yaml @@ -1905,8 +1905,8 @@ paths: - InternalToken: [] parameters: [] responses: - '200': - description: successful + '201': + description: created content: application/json: schema: @@ -1914,14 +1914,30 @@ paths: properties: success: type: boolean + status: + type: string + token_type: + type: string owner_email: type: string nullable: true key_name: type: string nullable: true - '400': - description: bad request + '422': + description: unprocessable entity + content: + application/json: + schema: + type: object + properties: + success: + type: boolean + error: + type: string + required: + - success + - error requestBody: content: application/json: @@ -1930,6 +1946,10 @@ paths: properties: token: type: string + submitter: + type: string + comment: + type: string required: - token "/api/summary": diff --git a/test/controllers/api/internal/revocations_controller_test.rb b/test/controllers/api/internal/revocations_controller_test.rb new file mode 100644 index 000000000..53df340cd --- /dev/null +++ b/test/controllers/api/internal/revocations_controller_test.rb @@ -0,0 +1,96 @@ +require "test_helper" + +class Api::Internal::RevocationsControllerTest < ActionDispatch::IntegrationTest + setup do + @previous_revocation_key = ENV["HKA_REVOCATION_KEY"] + ENV["HKA_REVOCATION_KEY"] = "test-revocation-key" + end + + teardown do + ENV["HKA_REVOCATION_KEY"] = @previous_revocation_key + end + + test "revokes regular ApiKey by rolling token" do + user = User.create!(timezone: "UTC") + email_address = user.email_addresses.create!(email: "regular@example.com", source: :signing_in) + original_token = SecureRandom.uuid_v4 + key = user.api_keys.create!(name: "Desktop", token: original_token) + + post "/api/internal/revoke", params: { token: original_token }, headers: auth_headers, as: :json + + assert_response :created + assert_equal true, response.parsed_body["success"] + assert_equal "complete", response.parsed_body["status"] + assert_equal key.name, response.parsed_body["token_type"] + assert_equal email_address.email, response.parsed_body["owner_email"] + assert_not_includes response.parsed_body.keys, "key_name" + + key.reload + assert_not_equal original_token, key.token + assert_nil ApiKey.find_by(token: original_token) + + post "/api/internal/revoke", params: { token: original_token }, headers: auth_headers, as: :json + + assert_response :unprocessable_entity + assert_equal false, response.parsed_body["success"] + assert_equal "Token is invalid or already revoked", response.parsed_body["error"] + end + + test "returns success false for valid regular UUID token that does not exist" do + token = SecureRandom.uuid_v4 + + post "/api/internal/revoke", params: { token: token }, headers: auth_headers, as: :json + + assert_response :unprocessable_entity + assert_equal false, response.parsed_body["success"] + assert_equal "Token is invalid or already revoked", response.parsed_body["error"] + end + + test "returns success false for token that matches neither regex" do + post "/api/internal/revoke", params: { token: "not-a-valid-token" }, headers: auth_headers, as: :json + + assert_response :unprocessable_entity + assert_equal false, response.parsed_body["success"] + assert_equal "Token doesn't match any supported type", response.parsed_body["error"] + end + + test "revokes admin key" do + user = User.create!(timezone: "UTC") + email_address = user.email_addresses.create!(email: "admin@example.com", source: :signing_in) + admin_key = user.admin_api_keys.create!(name: "Infra", token: "hka_#{SecureRandom.hex(32)}") + + post "/api/internal/revoke", params: { token: admin_key.token }, headers: auth_headers, as: :json + + assert_response :created + assert_equal true, response.parsed_body["success"] + assert_equal "complete", response.parsed_body["status"] + assert_equal "Infra", response.parsed_body["token_type"] + + admin_key.reload + assert_equal email_address.email, response.parsed_body["owner_email"] + assert_equal "Infra", response.parsed_body["key_name"] + assert_not_nil admin_key.revoked_at + assert_includes admin_key.name, "_revoked_" + end + + test "returns error for already-revoked admin key" do + user = User.create!(timezone: "UTC") + original_token = "hka_#{SecureRandom.hex(32)}" + admin_key = user.admin_api_keys.create!(name: "Infra", token: original_token) + admin_key.revoke! + + post "/api/internal/revoke", params: { token: original_token }, headers: auth_headers, as: :json + + assert_response :unprocessable_entity + assert_equal false, response.parsed_body["success"] + assert_equal "Token is invalid or already revoked", response.parsed_body["error"] + end + + private + + def auth_headers + { + "Authorization" => ActionController::HttpAuthentication::Token.encode_credentials(ENV.fetch("HKA_REVOCATION_KEY")) + } + end +end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 87160a09b..d0b1f037e 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -30,6 +30,30 @@ class UserTest < ActiveSupport::TestCase assert_equal "gruvbox_dark", metadata[:value] end + test "rotate_api_keys! replaces existing api key with a new one" do + user = User.create!(timezone: "UTC", slack_uid: "U#{SecureRandom.hex(8)}") + user.api_keys.create!(name: "Original key") + original_token = user.api_keys.first.token + + new_api_key = user.rotate_api_keys! + + assert_equal user.id, new_api_key.user_id + assert_equal "Hackatime key", new_api_key.name + assert_nil ApiKey.find_by(token: original_token) + end + + test "rotate_api_keys! creates a key when none exists" do + user = User.create!(timezone: "UTC", slack_uid: "U#{SecureRandom.hex(8)}") + + assert_equal 0, user.api_keys.count + + new_api_key = user.rotate_api_keys! + + assert_equal user.id, new_api_key.user_id + assert_equal "Hackatime key", new_api_key.name + assert_equal [ new_api_key.id ], user.api_keys.reload.pluck(:id) + end + test "flipper id uses the user id" do user = User.create!(timezone: "UTC")