Ensuring POST api/v0/articles authorizes by policy (#16685)

As part of the [Authorization System Use Case
1:1](https://github.com/orgs/forem/projects/46) project, we are driving
towards the feature of: "Only admin's may post articles in this Forem."

This commit ensures that the API's "create an article" end-point
delivers on that feature.

Along the way, I've added reading notes and comments, to help us further
flex in the future (namely move all authorization checks into a policy
object).

Closes forem/forem#16488

app/controllers/api/v0/api_controller.rb

@@ -50,6 +50,16 @@ module Api
       # Checks if the user is authenticated, sets @user to nil otherwise
       #
       # @return [User, NilClass]
+      #
+      # @see {#pundit_user} for one way we use this method
+      # @see {#authenticate_with_api_key_or_current_user} for the logic of building the user.
+      #
+      # @note We could memoize the `@user ||=` but Rubocop wants to rename that to
+      #       `authenticate_with_api_key_or_current_user` which would be bad as descendant classes
+      #       have chosen to reference the `@user` instance variable.  Intsead [@jeremyf] is
+      #       favoring leaving this method as is to reduce impact, and having `#pundit_user` do the
+      #       memoization.
+      #
       def authenticate_with_api_key_or_current_user
         @user = authenticate_with_api_key || current_user
       end
@@ -67,6 +77,24 @@ module Api
 
       private
 
+      # @note By default pundit_user is an alias of "#current_user".  However, as "#current_user"
+      #       only tells part of the story, we need to roll our own.  That means checking if we have
+      #       `@user` (which is set in #authenticate_with_api_key_or_current_user) but if that's not
+      #       present, call the method.
+      #
+      # @return [User, NilClass]
+      #
+      # @note [@jeremyf] is choosing to reference the instance variable (e.g. `@user`) and if that's
+      #       nil to call the `authenticate_with_api_key_or_current_user`.  This way I'm not
+      #       altering the implementation details of the `authenticate_with_api_key_or_current_user`
+      #       function by introducing memoization.
+      #
+      # @see {#authenticate_with_api_key_or_current_user}
+      def pundit_user
+        # What's going on here?
+        @pundit_user ||= @user || authenticate_with_api_key_or_current_user
+      end
+
       def authenticate_with_api_key
         api_key = request.headers["api-key"]
         return unless api_key

app/controllers/api/v0/articles_controller.rb

@@ -1,14 +1,18 @@
 module Api
   module V0
+    # @note This controller partially authorizes with the ArticlePolicy, in an ideal world, it would
+    #       fully authorize.  However, that refactor would require significantly more work.
     class ArticlesController < ApiController
       before_action :authenticate!, only: %i[create update me]
 
-      before_action :validate_article_param_is_hash, only: %w[create update]
+      before_action :validate_article_param_is_hash, only: %i[create update]
 
       before_action :set_cache_control_headers, only: %i[index show show_by_slug]
 
       skip_before_action :verify_authenticity_token, only: %i[create update]
 
+      after_action :verify_authorized, only: %i[create]
+
       INDEX_ATTRIBUTES_FOR_SERIALIZATION = %i[
         id user_id organization_id collection_id
         title description main_image published_at crossposted_at social_image
@@ -58,6 +62,8 @@ module Api
       end
 
       def create
+        authorize(Article)
+
         @article = Articles::Creator.call(@user, article_params).decorate
 
         if @article.persisted?

spec/requests/api/v0/articles_spec.rb

@@ -558,32 +558,52 @@ RSpec.describe "Api::V0::Articles", type: :request do
   end
 
   describe "POST /api/articles" do
+    # As written, it's envisioned that the subject and these "let" statements create a valid
+    # authentication and authorization.
+    subject(:the_response) do
+      # This looks a bit funny, I want to issue the request but test the response.  The "post"
+      # method does not return a "response" object.
+      post api_articles_path, params: { article: params }.to_json, headers: headers
+      response
+    end
+
     let(:api_secret) { create(:api_secret) }
     let(:user) { api_secret.user }
+    let(:headers) { { "api-key" => api_secret.secret, "content-type" => "application/json" } }
+    let(:params) { {} }
 
-    context "when unauthorized" do
-      it "fails with no api key" do
-        post api_articles_path, headers: { "content-type" => "application/json" }
-        expect(response).to have_http_status(:unauthorized)
-      end
+    context "when user suspended" do
+      before { user.add_role(:suspended) }
 
-      it "fails with a suspended user" do
-        user.add_role(:suspended)
-        post api_articles_path, headers: { "api-key" => api_secret.secret, "content-type" => "application/json" }
-        expect(response).to have_http_status(:unauthorized)
-      end
+      it { is_expected.to have_http_status(:unauthorized) }
+    end
 
-      it "fails with the wrong api key" do
-        post api_articles_path, headers: { "api-key" => "foobar", "content-type" => "application/json" }
-        expect(response).to have_http_status(:unauthorized)
-      end
+    context "when no api key provided" do
+      let(:headers) { { "content-type" => "application/json" } }
 
-      it "fails with a failing secure compare" do
-        allow(ActiveSupport::SecurityUtils)
-          .to receive(:secure_compare).and_return(false)
-        post api_articles_path, headers: { "api-key" => api_secret.secret, "content-type" => "application/json" }
-        expect(response).to have_http_status(:unauthorized)
-      end
+      it { is_expected.to have_http_status(:unauthorized) }
+    end
+
+    context "when given invalid api key" do
+      let(:headers) { { "api-key" => "no you're never gonna get it", "content-type" => "application/json" } }
+
+      it { is_expected.to have_http_status(:unauthorized) }
+    end
+
+    context "when security comparision fails" do
+      before { allow(ActiveSupport::SecurityUtils).to receive(:secure_compare).and_return(false) }
+
+      it { is_expected.to have_http_status(:unauthorized) }
+    end
+
+    context "when only admins can post to site" do
+      # [@jeremyf] Part of me loaths the idea of writing this specific policy implementation.
+      #            Another option is to do some "allow_any_instance_of" antics.  For now, this is
+      #            the concession, but as we move through further policy changes, I'm uncertain if
+      #            we want our requests to bombard the nuances of policy.
+      before { allow(ArticlePolicy).to receive(:limit_post_creation_to_admins?).and_return(true) }
+
+      it { is_expected.to have_http_status(:unauthorized) }
     end
 
     describe "when authorized" do