Merge branch 'broadcast-api-auth' into 'master'

Allow users to read broadcast messages via API

See merge request gitlab-org/gitlab!23298

changelogs/unreleased/broadcast-api-auth.yml

+---
+title: Allow users to read broadcast messages via API
+merge_request: 23298
+author: Rajendra Kadam
+type: changed

doc/api/broadcast_messages.md

@@ -4,7 +4,7 @@
 
 Broadcast messages API operates on [broadcast messages](../user/admin_area/broadcast_messages.md).
 
-The broadcast message API is only accessible to administrators. All requests by:
+As of GitLab 12.8, GET requests do not require authentication. All other broadcast message API endpoints are accessible only to administrators. Non-GET requests by:
 
 - Guests will result in `401 Unauthorized`.
 - Regular users will result in `403 Forbidden`.
@@ -20,7 +20,7 @@ GET /broadcast_messages
 Example request:
 
 ```sh
-curl --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.example.com/api/v4/broadcast_messages
+curl https://gitlab.example.com/api/v4/broadcast_messages
 ```
 
 Example response:
@@ -57,7 +57,7 @@ Parameters:
 Example request:
 
 ```sh
-curl --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.example.com/api/v4/broadcast_messages/1
+curl https://gitlab.example.com/api/v4/broadcast_messages/1
 ```
 
 Example response:

lib/api/broadcast_messages.rb

@@ -4,9 +4,6 @@ module API
   class BroadcastMessages < Grape::API
     include PaginationParams
 
-    before { authenticate! }
-    before { authenticated_as_admin! }
-
     resource :broadcast_messages do
       helpers do
         def find_message
@@ -40,6 +37,8 @@ module API
         optional :target_path, type: String, desc: 'Target path'
       end
       post do
+        authenticated_as_admin!
+
         message = BroadcastMessage.create(declared_params(include_missing: false))
 
         if message.persisted?
@@ -76,6 +75,8 @@ module API
         optional :target_path, type: String, desc: 'Target path'
       end
       put ':id' do
+        authenticated_as_admin!
+
         message = find_message
 
         if message.update(declared_params(include_missing: false))
@@ -93,6 +94,8 @@ module API
         requires :id, type: Integer, desc: 'Broadcast message ID'
       end
       delete ':id' do
+        authenticated_as_admin!
+
         message = find_message
 
         destroy_conditionally!(message)

spec/requests/api/broadcast_messages_spec.rb

@@ -8,22 +8,10 @@ describe API::BroadcastMessages do
   set(:message) { create(:broadcast_message) }
 
   describe 'GET /broadcast_messages' do
-    it 'returns a 401 for anonymous users' do
-      get api('/broadcast_messages')
-
-      expect(response).to have_gitlab_http_status(401)
-    end
-
-    it 'returns a 403 for users' do
-      get api('/broadcast_messages', user)
-
-      expect(response).to have_gitlab_http_status(403)
-    end
-
-    it 'returns an Array of BroadcastMessages for admins' do
+    it 'returns an Array of BroadcastMessages' do
       create(:broadcast_message)
 
-      get api('/broadcast_messages', admin)
+      get api('/broadcast_messages')
 
       expect(response).to have_gitlab_http_status(200)
       expect(response).to include_pagination_headers
@@ -34,21 +22,9 @@ describe API::BroadcastMessages do
   end
 
   describe 'GET /broadcast_messages/:id' do
-    it 'returns a 401 for anonymous users' do
+    it 'returns the specified message' do
       get api("/broadcast_messages/#{message.id}")
 
-      expect(response).to have_gitlab_http_status(401)
-    end
-
-    it 'returns a 403 for users' do
-      get api("/broadcast_messages/#{message.id}", user)
-
-      expect(response).to have_gitlab_http_status(403)
-    end
-
-    it 'returns the specified message for admins' do
-      get api("/broadcast_messages/#{message.id}", admin)
-
       expect(response).to have_gitlab_http_status(200)
       expect(json_response['id']).to eq message.id
       expect(json_response.keys)