FEATURE: allow user api key revocation for read only keys

app/controllers/user_api_keys_controller.rb

@@ -90,7 +90,16 @@ class UserApiKeysController < ApplicationController
   end
 
   def revoke
-    find_key.update_columns(revoked_at: Time.zone.now)
+    revoke_key = find_key
+    if current_key = request.env['HTTP_USER_API_KEY']
+      request_key = UserApiKey.find_by(key: current_key)
+      if request_key && request_key.id != revoke_key.id && !request_key.write
+        raise Discourse::InvalidAccess
+      end
+    end
+
+    revoke_key.update_columns(revoked_at: Time.zone.now)
+
     render json: success_json
   end
 

lib/auth/default_current_user_provider.rb

@@ -176,10 +176,16 @@ class Auth::DefaultCurrentUserProvider
 
   protected
 
+  WHITELISTED_WRITE_PATHS ||= [/^\/message-bus\/.*\/poll/, /^\/user-api-key\/revoke$/]
   def lookup_user_api_user(user_api_key)
     if api_key = UserApiKey.where(key: user_api_key, revoked_at: nil).includes(:user).first
-      if !api_key.write && (@env["REQUEST_METHOD"] != "GET" && @env["PATH_INFO"] !~ /^\/message-bus\/.*\/poll/)
-        raise Discourse::InvalidAccess
+      unless api_key.write
+        if @env["REQUEST_METHOD"] != "GET"
+          path = @env["PATH_INFO"]
+          unless WHITELISTED_WRITE_PATHS.any?{|whitelisted| path =~ whitelisted}
+            raise Discourse::InvalidAccess
+          end
+        end
       end
 
       api_key.user

spec/controllers/user_api_keys_controller_spec.rb

@@ -94,6 +94,27 @@ TXT
 
     end
 
+    it "will not allow readonly api keys to revoke others" do
+      key1 = Fabricate(:readonly_user_api_key)
+      key2 = Fabricate(:readonly_user_api_key)
+
+      request.env['HTTP_USER_API_KEY'] = key1.key
+      post :revoke, id: key2.id
+
+      expect(response.status).to eq(403)
+    end
+
+    it "will allow readonly api keys to revoke self" do
+      key = Fabricate(:readonly_user_api_key)
+      request.env['HTTP_USER_API_KEY'] = key.key
+      post :revoke, id: key.id
+
+      expect(response.status).to eq(200)
+
+      key.reload
+      expect(key.revoked_at).not_to eq(nil)
+    end
+
     it "will not return p access if not yet configured" do
       SiteSetting.min_trust_level_for_user_api_key = 0
       SiteSetting.allowed_user_api_auth_redirects = args[:auth_redirect]

spec/fabricators/user_api_key_fabricator.rb

@@ -0,0 +1,9 @@
+Fabricator(:readonly_user_api_key, from: :user_api_key) do
+  user
+  read true
+  write false
+  push false
+  client_id { SecureRandom.hex }
+  key { SecureRandom.hex }
+  application_name 'some app'
+end