diff --git a/app/controllers/api/v1/analytics_controller.rb b/app/controllers/api/v1/analytics_controller.rb index fc917b8fd..d168973df 100644 --- a/app/controllers/api/v1/analytics_controller.rb +++ b/app/controllers/api/v1/analytics_controller.rb @@ -8,7 +8,7 @@ module Api include Api::AnalyticsController - before_action :authenticate! + before_action :authenticate_with_api_key_or_current_user! before_action :authorize_user_organization before_action :load_owner before_action :validate_date_params, only: [:historical] diff --git a/app/controllers/api/v1/api_controller.rb b/app/controllers/api/v1/api_controller.rb index 6be086999..b94300de9 100644 --- a/app/controllers/api/v1/api_controller.rb +++ b/app/controllers/api/v1/api_controller.rb @@ -34,16 +34,54 @@ module Api render json: { error: "not found", status: 404 }, status: :not_found end + # @note This method is used in ApplicationController within the + # `verify_private_forem` method (read more in annotations there). + # It uses `authenticate_with_api_key_or_current_user!` under the + # hood to ensure the request is authenticated (on private forems in + # this case). We recommend API::V1 controllers rely on the methods + # below to check for either API key or API key + current_user. + # They're more verbose but they convey the auth method clearly. + def authenticate! + authenticate_with_api_key_or_current_user! + end + # @note This method is performing both authentication and authorization. The user suspended # should be something added to the corresponding pundit policy. - def authenticate! - @user = authenticate_with_api_key + def authenticate_with_api_key! + @user ||= authenticate_with_api_key return error_unauthorized unless @user return error_unauthorized if @user.suspended? true end + # @note This method is performing both authentication and authorization. The user suspended + # should be something added to the corresponding pundit policy. + def authenticate_with_api_key_or_current_user! + @user ||= authenticate_with_api_key_or_current_user + return error_unauthorized unless @user + return error_unauthorized if @user.suspended? + + true + end + + # Checks if the user is authenticated, sets @user to nil otherwise + # + # @return [User, NilClass] + # + # @see #pundit_user + # @see #authenticate_with_api_key_or_current_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 + def authorize_super_admin error_unauthorized unless @user.super_admin? end diff --git a/app/controllers/api/v1/articles_controller.rb b/app/controllers/api/v1/articles_controller.rb index 7426376bd..cb6f58606 100644 --- a/app/controllers/api/v1/articles_controller.rb +++ b/app/controllers/api/v1/articles_controller.rb @@ -5,7 +5,7 @@ module Api class ArticlesController < ApiController include Api::ArticlesController - before_action :authenticate! + before_action :authenticate_with_api_key!, only: %i[create update me unpublish] before_action :validate_article_param_is_hash, only: %i[create update] before_action :set_cache_control_headers, only: %i[index show show_by_slug] after_action :verify_authorized, only: %i[create] diff --git a/app/controllers/api/v1/comments_controller.rb b/app/controllers/api/v1/comments_controller.rb index 11eb60cfa..a773d80f5 100644 --- a/app/controllers/api/v1/comments_controller.rb +++ b/app/controllers/api/v1/comments_controller.rb @@ -3,7 +3,6 @@ module Api class CommentsController < ApiController include Api::CommentsController - before_action :authenticate! before_action :set_cache_control_headers, only: %i[index show] end end diff --git a/app/controllers/api/v1/followers_controller.rb b/app/controllers/api/v1/followers_controller.rb index 4985d5d68..49961ecd3 100644 --- a/app/controllers/api/v1/followers_controller.rb +++ b/app/controllers/api/v1/followers_controller.rb @@ -3,7 +3,7 @@ module Api class FollowersController < ApiController include Api::FollowersController - before_action :authenticate! + before_action :authenticate_with_api_key_or_current_user! before_action -> { limit_per_page(default: 80, max: 1000) } end end diff --git a/app/controllers/api/v1/follows_controller.rb b/app/controllers/api/v1/follows_controller.rb index 1187f020d..663504dc7 100644 --- a/app/controllers/api/v1/follows_controller.rb +++ b/app/controllers/api/v1/follows_controller.rb @@ -3,7 +3,7 @@ module Api class FollowsController < ApiController include Api::FollowsController - before_action :authenticate! + before_action :authenticate_with_api_key_or_current_user! end end end diff --git a/app/controllers/api/v1/health_checks_controller.rb b/app/controllers/api/v1/health_checks_controller.rb index 6885ac438..7a966b748 100644 --- a/app/controllers/api/v1/health_checks_controller.rb +++ b/app/controllers/api/v1/health_checks_controller.rb @@ -3,7 +3,7 @@ module Api class HealthChecksController < ApiController include Api::HealthChecksController - before_action :authenticate! + before_action :authenticate_with_token end end end diff --git a/app/controllers/api/v1/listings_controller.rb b/app/controllers/api/v1/listings_controller.rb index 57d362ace..044fca39a 100644 --- a/app/controllers/api/v1/listings_controller.rb +++ b/app/controllers/api/v1/listings_controller.rb @@ -5,7 +5,8 @@ module Api # actions `create` and `update` are defined in the module `ListingsToolkit`, # we thus silence Rubocop lexical scope filter cop: https://rails.rubystyle.guide/#lexically-scoped-action-filter - before_action :authenticate! + before_action :authenticate_with_api_key_or_current_user!, only: %i[create update] + before_action :authenticate_with_api_key_or_current_user, only: %i[show] before_action :set_cache_control_headers, only: %i[index show] before_action :set_and_authorize_listing, only: %i[update] end diff --git a/app/controllers/api/v1/organizations_controller.rb b/app/controllers/api/v1/organizations_controller.rb index 86893bdd2..bd998a71f 100644 --- a/app/controllers/api/v1/organizations_controller.rb +++ b/app/controllers/api/v1/organizations_controller.rb @@ -3,7 +3,6 @@ module Api class OrganizationsController < ApiController include Api::OrganizationsController - before_action :authenticate! before_action :find_organization, only: %i[users listings articles] end end diff --git a/app/controllers/api/v1/podcast_episodes_controller.rb b/app/controllers/api/v1/podcast_episodes_controller.rb index 344807e5a..0982c9736 100644 --- a/app/controllers/api/v1/podcast_episodes_controller.rb +++ b/app/controllers/api/v1/podcast_episodes_controller.rb @@ -3,7 +3,6 @@ module Api class PodcastEpisodesController < ApiController include Api::PodcastEpisodesController - before_action :authenticate! before_action :set_cache_control_headers, only: %i[index] end end diff --git a/app/controllers/api/v1/readinglist_controller.rb b/app/controllers/api/v1/readinglist_controller.rb index 190b7eed5..bee629e3d 100644 --- a/app/controllers/api/v1/readinglist_controller.rb +++ b/app/controllers/api/v1/readinglist_controller.rb @@ -3,7 +3,7 @@ module Api class ReadinglistController < ApiController include Api::ReadinglistController - before_action :authenticate! + before_action :authenticate_with_api_key! end end end diff --git a/app/controllers/api/v1/tags_controller.rb b/app/controllers/api/v1/tags_controller.rb index 55dd91a34..d017ea252 100644 --- a/app/controllers/api/v1/tags_controller.rb +++ b/app/controllers/api/v1/tags_controller.rb @@ -3,7 +3,6 @@ module Api class TagsController < ApiController include Api::TagsController - before_action :authenticate! before_action :set_cache_control_headers, only: %i[index] end end diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb index 18d1f5f3c..3ac23f096 100644 --- a/app/controllers/api/v1/users_controller.rb +++ b/app/controllers/api/v1/users_controller.rb @@ -3,7 +3,7 @@ module Api class UsersController < ApiController include Api::UsersController - before_action :authenticate! + before_action :authenticate_with_api_key!, only: %i[me suspend unpublish] end end end diff --git a/app/controllers/api/v1/videos_controller.rb b/app/controllers/api/v1/videos_controller.rb index 72341bf53..3e2400e91 100644 --- a/app/controllers/api/v1/videos_controller.rb +++ b/app/controllers/api/v1/videos_controller.rb @@ -3,7 +3,6 @@ module Api class VideosController < ApiController include Api::VideosController - before_action :authenticate! before_action :set_cache_control_headers, only: %i[index] end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3e8089fd6..e9d4fc149 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -67,6 +67,7 @@ class ApplicationController < ActionController::Base # @return [TrueClass] if the current requested action is for the API # @return [FalseClass] if the current requested action is not part of the API # @see Api::V0::ApiController + # @see Api::V1::ApiController # @see ApplicationController.api_action # @see ApplicationController#verify_private_forem diff --git a/spec/requests/api/v0/api_controller_spec.rb b/spec/requests/api/v0/api_controller_spec.rb index 3a99abd7a..a535f8546 100644 --- a/spec/requests/api/v0/api_controller_spec.rb +++ b/spec/requests/api/v0/api_controller_spec.rb @@ -29,9 +29,7 @@ RSpec.describe "Api::V0::ApiController", type: :request do end context "when API V1 is enabled" do - before do - allow(FeatureFlag).to receive(:enabled?).with(:api_v1).and_return(true) - end + before { allow(FeatureFlag).to receive(:enabled?).and_return(true) } context "when request header is v0 and does not include an api key" do let(:headers) { { Accept: "application/v0+json" } } diff --git a/spec/requests/api/v1/analytics_spec.rb b/spec/requests/api/v1/analytics_spec.rb index 4ca165aad..2242fb777 100644 --- a/spec/requests/api/v1/analytics_spec.rb +++ b/spec/requests/api/v1/analytics_spec.rb @@ -2,7 +2,7 @@ require "rails_helper" RSpec.describe "Api::V1::Analytics", type: :request do let(:api_secret) { create(:api_secret) } - let(:v1_headers) { { "Accept" => "application/vnd.forem.api-v1+json", "api-key" => api_secret.secret } } + let(:headers) { { "Accept" => "application/vnd.forem.api-v1+json", "api-key" => api_secret.secret } } before { allow(FeatureFlag).to receive(:enabled?).with(:api_v1).and_return(true) } @@ -19,7 +19,7 @@ RSpec.describe "Api::V1::Analytics", type: :request do end context "when the start parameter is not included" do - before { get "/api/analytics/historical", headers: v1_headers } + before { get "/api/analytics/historical", headers: headers } it "fails with an unprocessable entity HTTP error" do expect(response).to have_http_status(:unprocessable_entity) @@ -32,7 +32,7 @@ RSpec.describe "Api::V1::Analytics", type: :request do end context "when the start parameter has the incorrect format" do - before { get "/api/analytics/historical?start=2019/2/2", headers: v1_headers } + before { get "/api/analytics/historical?start=2019/2/2", headers: headers } it "fails with an unprocessable entity HTTP error" do expect(response).to have_http_status(:unprocessable_entity) diff --git a/spec/requests/api/v1/articles_spec.rb b/spec/requests/api/v1/articles_spec.rb index e9f2c6160..105e35a7e 100644 --- a/spec/requests/api/v1/articles_spec.rb +++ b/spec/requests/api/v1/articles_spec.rb @@ -7,13 +7,8 @@ RSpec.describe "Api::V1::Articles", type: :request do let(:new_article) { create(:article) } let(:api_secret) { create(:api_secret) } let(:listener) { :admin_api } - let(:v1_headers) do - { - "content-type" => "application/json", - "Accept" => "application/vnd.forem.api-v1+json", - "api-key" => api_secret.secret - } - end + let(:headers) { { "content-type" => "application/json", "Accept" => "application/vnd.forem.api-v1+json" } } + let(:auth_headers) { headers.merge({ "api-key" => api_secret.secret }) } before do stub_const("FlareTag::FLARE_TAG_IDS_HASH", { "discuss" => tag.id }) @@ -23,14 +18,9 @@ RSpec.describe "Api::V1::Articles", type: :request do describe "GET /api/articles" do before { article } - it "returns 401 when unauthenticated" do - get api_articles_path, headers: { "Accept" => "application/vnd.forem.api-v1+json" } - expect(response).to have_http_status(:unauthorized) - end - it "returns CORS headers" do origin = "http://example.com" - get api_articles_path, headers: v1_headers.merge({ origin: origin }) + get api_articles_path, headers: headers.merge({ origin: origin }) expect(response).to have_http_status(:ok) expect(response.headers["Access-Control-Allow-Origin"]).to eq(origin) @@ -41,7 +31,7 @@ RSpec.describe "Api::V1::Articles", type: :request do it "has correct keys in the response" do article.update_columns(organization_id: organization.id) - get api_articles_path, headers: v1_headers + get api_articles_path, headers: headers index_keys = %w[ type_of id title description cover_image readable_publish_date social_image @@ -54,44 +44,44 @@ RSpec.describe "Api::V1::Articles", type: :request do end it "returns correct tag list" do - get api_articles_path, headers: v1_headers + get api_articles_path, headers: headers expect(response.parsed_body.first["tag_list"]).to be_a_kind_of Array end it "returns correct tags" do - get api_articles_path, headers: v1_headers + get api_articles_path, headers: headers expect(response.parsed_body.first["tags"]).to be_a_kind_of String end context "without params" do it "returns json response" do - get api_articles_path, headers: v1_headers + get api_articles_path, headers: headers expect(response.media_type).to eq("application/json") end it "returns nothing if params state=all is not found" do - get api_articles_path(state: "all"), headers: v1_headers + get api_articles_path(state: "all"), headers: headers expect(response.parsed_body.size).to eq(0) end it "returns featured articles if no param is given" do article.update_column(:featured, true) - get api_articles_path, headers: v1_headers + get api_articles_path, headers: headers expect(response.parsed_body.size).to eq(1) end it "supports pagination" do create_list(:article, 2, featured: true) - get api_articles_path, params: { page: 1, per_page: 2 }, headers: v1_headers + get api_articles_path, params: { page: 1, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(2) - get api_articles_path, params: { page: 2, per_page: 2 }, headers: v1_headers + get api_articles_path, params: { page: 2, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(1) end it "returns flare tag in the response" do - get api_articles_path, headers: v1_headers + get api_articles_path, headers: headers response_article = response.parsed_body.first expect(response_article["flare_tag"]).to be_present expect(response_article["flare_tag"].keys).to eq(%w[name bg_color_hex text_color_hex]) @@ -99,7 +89,7 @@ RSpec.describe "Api::V1::Articles", type: :request do end it "sets the correct edge caching surrogate key" do - get api_articles_path, headers: v1_headers + get api_articles_path, headers: headers expected_key = ["articles", article.record_key].to_set expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key) @@ -109,32 +99,32 @@ RSpec.describe "Api::V1::Articles", type: :request do context "with username param" do it "returns user's articles for the given username" do create(:article, user: article.user) - get api_articles_path(username: article.user.username), headers: v1_headers + get api_articles_path(username: article.user.username), headers: headers expect(response.parsed_body.size).to eq(2) end it "returns nothing if given user is not found" do - get api_articles_path(username: "foobar"), headers: v1_headers + get api_articles_path(username: "foobar"), headers: headers expect(response.parsed_body.size).to eq(0) end it "returns org's articles if org's slug is given" do create(:article, user: article.user, organization: organization) - get api_articles_path(username: organization.slug), headers: v1_headers + get api_articles_path(username: organization.slug), headers: headers expect(response.parsed_body.size).to eq(1) end it "supports pagination" do create_list(:article, 2, user: article.user) - get api_articles_path(username: article.user.username), params: { page: 1, per_page: 2 }, headers: v1_headers + get api_articles_path(username: article.user.username), params: { page: 1, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(2) - get api_articles_path(username: article.user.username), params: { page: 2, per_page: 2 }, headers: v1_headers + get api_articles_path(username: article.user.username), params: { page: 2, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(1) end it "sets the correct edge caching surrogate key" do new_article = create(:article, user: article.user) - get api_articles_path(username: article.user.username), headers: v1_headers + get api_articles_path(username: article.user.username), headers: headers expected_key = ["articles", article.record_key, new_article.record_key].to_set expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key) @@ -143,12 +133,12 @@ RSpec.describe "Api::V1::Articles", type: :request do context "with tag param" do it "returns tag's articles" do - get api_articles_path(tag: article.tag_list.first), headers: v1_headers + get api_articles_path(tag: article.tag_list.first), headers: headers expect(response.parsed_body.size).to eq(1) end it "returns top tag articles if tag and top param is present" do - get api_articles_path(tag: article.tag_list.first, top: "7"), headers: v1_headers + get api_articles_path(tag: article.tag_list.first, top: "7"), headers: headers expect(response.parsed_body.size).to eq(1) end @@ -157,20 +147,20 @@ RSpec.describe "Api::V1::Articles", type: :request do tag = Tag.find_by(name: article.tag_list.first) tag.update(requires_approval: true) - get api_articles_path(tag: tag.name), headers: v1_headers + get api_articles_path(tag: tag.name), headers: headers expect(response.parsed_body.size).to eq(0) end it "supports pagination" do create_list(:article, 2, tags: "discuss") - get api_articles_path(tag: article.tag_list.first), params: { page: 1, per_page: 2 }, headers: v1_headers + get api_articles_path(tag: article.tag_list.first), params: { page: 1, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(2) - get api_articles_path(tag: article.tag_list.first), params: { page: 2, per_page: 2 }, headers: v1_headers + get api_articles_path(tag: article.tag_list.first), params: { page: 2, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(1) end it "sets the correct edge caching surrogate key" do - get api_articles_path(tag: article.tag_list.first), headers: v1_headers + get api_articles_path(tag: article.tag_list.first), headers: headers expected_key = ["articles", article.record_key].to_set expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key) @@ -180,7 +170,7 @@ RSpec.describe "Api::V1::Articles", type: :request do context "with tags param" do it "returns articles with any of the specified tags" do create(:article, published: true) - get api_articles_path(tags: "javascript, css, not-existing-tag"), headers: v1_headers + get api_articles_path(tags: "javascript, css, not-existing-tag"), headers: headers expect(response.parsed_body.size).to eq(1) end end @@ -188,11 +178,11 @@ RSpec.describe "Api::V1::Articles", type: :request do context "with tags_exclude param" do it "returns articles that do not contain any of excluded tag" do create(:article, published: true) - get api_articles_path(tags_exclude: "node, java"), headers: v1_headers + get api_articles_path(tags_exclude: "node, java"), headers: headers expect(response.parsed_body.size).to eq(2) create(:article, published: true, tags: "node") - get api_articles_path(tags_exclude: "node, java"), headers: v1_headers + get api_articles_path(tags_exclude: "node, java"), headers: headers expect(response.parsed_body.size).to eq(2) end end @@ -200,7 +190,7 @@ RSpec.describe "Api::V1::Articles", type: :request do context "with tags and tags_exclude params" do it "returns proper scope" do create(:article, published: true) - get api_articles_path(tags: "javascript, css", tags_exclude: "node, java"), headers: v1_headers + get api_articles_path(tags: "javascript, css", tags_exclude: "node, java"), headers: headers expect(response.parsed_body.size).to eq(1) end end @@ -208,7 +198,7 @@ RSpec.describe "Api::V1::Articles", type: :request do context "when tags and tags_exclude contain the same tag" do it "returns empty set" do create(:article, published: true, tags: "java") - get api_articles_path(tags: "java", tags_exclude: "java"), headers: v1_headers + get api_articles_path(tags: "java", tags_exclude: "java"), headers: headers expect(response.parsed_body.size).to eq(0) end end @@ -218,7 +208,7 @@ RSpec.describe "Api::V1::Articles", type: :request do # TODO: slight duplication, test should be removed old_article = create(:article) old_article.update_column(:published_at, 10.days.ago) - get api_articles_path(top: "7"), headers: v1_headers + get api_articles_path(top: "7"), headers: headers expect(response.parsed_body.size).to eq(1) end @@ -227,14 +217,14 @@ RSpec.describe "Api::V1::Articles", type: :request do old_articles.each do |old_article| old_article.update_column(:published_at, 10.days.ago) end - get api_articles_path(top: "11"), params: { page: 1, per_page: 2 }, headers: v1_headers + get api_articles_path(top: "11"), params: { page: 1, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(2) - get api_articles_path(top: "11"), params: { page: 2, per_page: 2 }, headers: v1_headers + get api_articles_path(top: "11"), params: { page: 2, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(1) end it "sets the correct edge caching surrogate key" do - get api_articles_path(top: "7"), headers: v1_headers + get api_articles_path(top: "7"), headers: headers expected_key = ["articles", article.record_key].to_set expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key) @@ -245,7 +235,7 @@ RSpec.describe "Api::V1::Articles", type: :request do it "returns a collection id" do collection = create(:collection, user: article.user) article.update_columns(collection_id: collection.id) - get api_articles_path(collection_id: collection.id), headers: v1_headers + get api_articles_path(collection_id: collection.id), headers: headers expect(response.parsed_body[0]["collection_id"]).to eq collection.id end @@ -256,16 +246,16 @@ RSpec.describe "Api::V1::Articles", type: :request do collection_articles.each do |collection_article| collection_article.update_columns(collection_id: collection.id) end - get api_articles_path(collection_id: collection.id), params: { page: 1, per_page: 2 }, headers: v1_headers + get api_articles_path(collection_id: collection.id), params: { page: 1, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(2) - get api_articles_path(collection_id: collection.id), params: { page: 2, per_page: 2 }, headers: v1_headers + get api_articles_path(collection_id: collection.id), params: { page: 2, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(1) end it "sets the correct edge caching surrogate key" do collection = create(:collection, user: article.user) article.update_columns(collection_id: collection.id) - get api_articles_path(collection_id: collection.id), headers: v1_headers + get api_articles_path(collection_id: collection.id), headers: headers expected_key = ["articles", article.record_key].to_set expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key) @@ -276,14 +266,14 @@ RSpec.describe "Api::V1::Articles", type: :request do it "returns fresh articles" do article.update_columns(public_reactions_count: 1, score: 1) - get api_articles_path(state: "fresh"), headers: v1_headers + get api_articles_path(state: "fresh"), headers: headers expect(response.parsed_body.size).to eq(1) end it "returns rising articles" do article.update_columns(public_reactions_count: 32, score: 1, featured_number: 2.days.ago.to_i) - get api_articles_path(state: "rising"), headers: v1_headers + get api_articles_path(state: "rising"), headers: headers expect(response.parsed_body.size).to eq(1) end @@ -291,14 +281,14 @@ RSpec.describe "Api::V1::Articles", type: :request do article.update_columns(published_at: 500.years.ago) new_article.update_columns(published_at: 1.minute.ago) - get latest_api_articles_path, headers: v1_headers + get latest_api_articles_path, headers: headers first_article_published_at = response.parsed_body.first["published_at"] last_article_published_at = response.parsed_body.last["published_at"] expect(first_article_published_at.to_date).to be > last_article_published_at.to_date end it "returns nothing if the state is unknown" do - get api_articles_path(state: "foobar"), headers: v1_headers + get api_articles_path(state: "foobar"), headers: headers expect(response.parsed_body).to be_empty end @@ -306,16 +296,16 @@ RSpec.describe "Api::V1::Articles", type: :request do it "supports pagination" do create_list(:article, 2, tags: "discuss", public_reactions_count: 1, score: 1) - get api_articles_path(state: "fresh"), params: { page: 1, per_page: 2 }, headers: v1_headers + get api_articles_path(state: "fresh"), params: { page: 1, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(2) - get api_articles_path(state: "fresh"), params: { page: 2, per_page: 2 }, headers: v1_headers + get api_articles_path(state: "fresh"), params: { page: 2, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(1) end it "sets the correct edge caching surrogate key" do article.update_columns(public_reactions_count: 1, score: 1) - get api_articles_path(state: "fresh"), headers: v1_headers + get api_articles_path(state: "fresh"), headers: headers expected_key = ["articles", article.record_key].to_set expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key) end @@ -325,21 +315,16 @@ RSpec.describe "Api::V1::Articles", type: :request do it "works if both the social image and the main image are missing" do article.update_columns(social_image: nil, main_image: nil) - get api_articles_path, headers: v1_headers + get api_articles_path, headers: headers expect(response).to have_http_status(:ok) end end end describe "GET /api/articles/:id" do - it "returns 401 when unauthenticated" do - get api_article_path(article.id), headers: { "Accept" => "application/vnd.forem.api-v1+json" } - expect(response).to have_http_status(:unauthorized) - end - it "returns CORS headers" do origin = "http://example.com" - get api_article_path(article.id), headers: v1_headers.merge({ origin: origin }) + get api_article_path(article.id), headers: headers.merge({ origin: origin }) expect(response).to have_http_status(:ok) expect(response.headers["Access-Control-Allow-Origin"]).to eq(origin) @@ -350,7 +335,7 @@ RSpec.describe "Api::V1::Articles", type: :request do it "has correct keys in the response" do article.update_columns(organization_id: organization.id) - get api_article_path(article.id), headers: v1_headers + get api_article_path(article.id), headers: headers show_keys = %w[ type_of id title description cover_image readable_publish_date social_image @@ -363,19 +348,19 @@ RSpec.describe "Api::V1::Articles", type: :request do end it "returns correct tag list" do - get api_article_path(article.id), headers: v1_headers + get api_article_path(article.id), headers: headers expect(response.parsed_body["tag_list"]).to be_a_kind_of String end it "returns correct tags" do - get api_article_path(article.id), headers: v1_headers + get api_article_path(article.id), headers: headers expect(response.parsed_body["tags"]).to be_a_kind_of Array end it "returns proper article" do - get api_article_path(article.id), headers: v1_headers + get api_article_path(article.id), headers: headers expect(response.parsed_body).to include( "title" => article.title, "body_markdown" => article.body_markdown, @@ -387,7 +372,7 @@ RSpec.describe "Api::V1::Articles", type: :request do article.update_columns( edited_at: 1.minute.from_now, crossposted_at: 2.minutes.ago, last_comment_at: 30.seconds.ago, ) - get api_article_path(article.id), headers: v1_headers + get api_article_path(article.id), headers: headers expect(response.parsed_body).to include( "created_at" => article.created_at.utc.iso8601, "edited_at" => article.edited_at.utc.iso8601, @@ -399,17 +384,17 @@ RSpec.describe "Api::V1::Articles", type: :request do it "fails with an unpublished article" do article.update_columns(published: false, published_at: nil) - get api_article_path(article.id), headers: v1_headers + get api_article_path(article.id), headers: headers expect(response).to have_http_status(:not_found) end it "fails with an unknown article ID" do - get api_article_path("9999"), headers: v1_headers + get api_article_path("9999"), headers: headers expect(response).to have_http_status(:not_found) end it "sets the correct edge caching surrogate key" do - get api_article_path(article), headers: v1_headers + get api_article_path(article), headers: headers expected_key = [article.record_key].to_set expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key) @@ -419,7 +404,7 @@ RSpec.describe "Api::V1::Articles", type: :request do describe "GET /api/articles/:username/:slug" do it "returns CORS headers" do origin = "http://example.com" - get "/api/articles/#{article.username}/#{article.slug}", headers: v1_headers.merge({ origin: origin }) + get "/api/articles/#{article.username}/#{article.slug}", headers: headers.merge({ origin: origin }) expect(response).to have_http_status(:ok) expect(response.headers["Access-Control-Allow-Origin"]).to eq(origin) expect(response.headers["Access-Control-Allow-Methods"]).to eq("HEAD, GET, OPTIONS") @@ -428,13 +413,13 @@ RSpec.describe "Api::V1::Articles", type: :request do end it "returns correct tags" do - get "/api/articles/#{article.username}/#{article.slug}", headers: v1_headers + get "/api/articles/#{article.username}/#{article.slug}", headers: headers expect(response.parsed_body["tags"]).to eq(article.tag_list) expect(response.parsed_body["tag_list"]).to eq(article.tags[0].name) end it "returns proper article" do - get "/api/articles/#{article.username}/#{article.slug}", headers: v1_headers + get "/api/articles/#{article.username}/#{article.slug}", headers: headers expect(response.parsed_body).to include( "title" => article.title, "body_markdown" => article.body_markdown, @@ -446,7 +431,7 @@ RSpec.describe "Api::V1::Articles", type: :request do article.update_columns( edited_at: 1.minute.from_now, crossposted_at: 2.minutes.ago, last_comment_at: 30.seconds.ago, ) - get "/api/articles/#{article.username}/#{article.slug}", headers: v1_headers + get "/api/articles/#{article.username}/#{article.slug}", headers: headers expect(response.parsed_body).to include( "created_at" => article.created_at.utc.iso8601, "edited_at" => article.edited_at.utc.iso8601, @@ -458,17 +443,17 @@ RSpec.describe "Api::V1::Articles", type: :request do it "fails with an unpublished article" do article.update_columns(published: false, published_at: nil) - get "/api/articles/#{article.username}/#{article.slug}", headers: v1_headers + get "/api/articles/#{article.username}/#{article.slug}", headers: headers expect(response).to have_http_status(:not_found) end it "fails with an unknown article path" do - get "/api/articles/chrisevans/#{article.slug}", headers: v1_headers + get "/api/articles/chrisevans/#{article.slug}", headers: headers expect(response).to have_http_status(:not_found) end it "sets the correct edge caching surrogate key" do - get "/api/articles/#{article.username}/#{article.slug}", headers: v1_headers + get "/api/articles/#{article.username}/#{article.slug}", headers: headers expected_key = [article.record_key].to_set expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key) @@ -487,13 +472,13 @@ RSpec.describe "Api::V1::Articles", type: :request do let(:user) { api_secret.user } it "returns proper response specification" do - get "/api/articles/me", headers: v1_headers + get "/api/articles/me", headers: auth_headers expect(response.media_type).to eq("application/json") expect(response).to have_http_status(:ok) end it "returns success when requesting published articles with public token" do - get "/api/articles/me/published", headers: v1_headers + get "/api/articles/me/published", headers: auth_headers expect(response.media_type).to eq("application/json") expect(response).to have_http_status(:ok) end @@ -501,14 +486,14 @@ RSpec.describe "Api::V1::Articles", type: :request do it "return only user's articles including markdown" do create(:article, user: user) create(:article) - get "/api/articles/me", headers: v1_headers + get "/api/articles/me", headers: auth_headers expect(response.parsed_body.length).to eq(1) expect(response.parsed_body[0]["body_markdown"]).not_to be_nil end it "supports pagination" do create_list(:article, 3, user: user) - get "/api/articles/me", headers: v1_headers, params: { page: 2, per_page: 2 } + get "/api/articles/me", headers: auth_headers, params: { page: 2, per_page: 2 } expect(response.parsed_body.length).to eq(1) end @@ -516,7 +501,7 @@ RSpec.describe "Api::V1::Articles", type: :request do article = create(:article, user: user) article.update_columns(organization_id: organization.id) - get "/api/articles/me", headers: v1_headers + get "/api/articles/me", headers: auth_headers keys = %w[ type_of id title description published published_at slug path url @@ -530,19 +515,19 @@ RSpec.describe "Api::V1::Articles", type: :request do it "only includes published articles by default" do create(:article, published: false, published_at: nil, user: user) - get "/api/articles/me", headers: v1_headers + get "/api/articles/me", headers: auth_headers expect(response.parsed_body.length).to eq(0) end it "only includes published articles when asking for published articles" do create(:article, published: false, published_at: nil, user: user) - get "/api/articles/me/published", headers: v1_headers + get "/api/articles/me/published", headers: auth_headers expect(response.parsed_body.length).to eq(0) end it "only includes unpublished articles when asking for unpublished articles" do create(:article, published: false, published_at: nil, user: user) - get "/api/articles/me/unpublished", headers: v1_headers + get "/api/articles/me/unpublished", headers: auth_headers expect(response.parsed_body.length).to eq(1) end @@ -552,7 +537,7 @@ RSpec.describe "Api::V1::Articles", type: :request do Timecop.travel(1.day.from_now) do newer = create(:article, published: false, published_at: nil, user: user) end - get "/api/articles/me/unpublished", headers: v1_headers + get "/api/articles/me/unpublished", headers: auth_headers expected_order = response.parsed_body.map { |resp| resp["id"] } expect(expected_order).to eq([newer.id, older.id]) end @@ -560,7 +545,7 @@ RSpec.describe "Api::V1::Articles", type: :request do it "puts unpublished articles at the top when asking for all articles" do create(:article, user: user) create(:article, published: false, published_at: nil, user: user) - get "/api/articles/me/all", headers: v1_headers + get "/api/articles/me/all", headers: auth_headers expected_order = response.parsed_body.map { |resp| resp["published"] } expect(expected_order).to eq([false, true]) end @@ -568,7 +553,7 @@ RSpec.describe "Api::V1::Articles", type: :request do it "correctly returns reading time in minutes" do create(:article, user: user) - get "/api/articles/me", headers: v1_headers + get "/api/articles/me", headers: auth_headers expect(response.parsed_body.first["reading_time_minutes"]).to eq(article.reading_time) end end @@ -909,35 +894,26 @@ RSpec.describe "Api::V1::Articles", type: :request do describe "when unauthorized" do it "fails with no api key" do - put path, headers: { "content-type" => "application/json", "Accept" => "application/vnd.forem.api-v1+json" } + put path, headers: headers expect(response).to have_http_status(:unauthorized) end it "fails with the wrong api key" do - put path, - headers: { "api-key" => "foobar", "content-type" => "application/json", - "Accept" => "application/vnd.forem.api-v1+json" } + put path, headers: headers.merge({ "api-key" => "foobar" }) expect(response).to have_http_status(:unauthorized) end it "fails with a failing secure compare" do allow(ActiveSupport::SecurityUtils) .to receive(:secure_compare).and_return(false) - put path, - headers: { "api-key" => api_secret.secret, "content-type" => "application/json", - "Accept" => "application/vnd.forem.api-v1+json" } + put path, headers: auth_headers expect(response).to have_http_status(:unauthorized) end end describe "when authorized" do - let(:put_headers) do - { "api-key" => api_secret.secret, "content-type" => "application/json", - "Accept" => "application/vnd.forem.api-v1+json" } - end - def put_article(**params) - put path, params: { article: params }.to_json, headers: put_headers + put path, params: { article: params }.to_json, headers: auth_headers end it "returns a 429 status code if the rate limit is reached" do @@ -956,7 +932,7 @@ RSpec.describe "Api::V1::Articles", type: :request do it "returns not found if the article does not belong to the user" do article = create(:article, user: create(:user)) params = { article: { title: "foobar" } }.to_json - put "/api/articles/#{article.id}", params: params, headers: put_headers + put "/api/articles/#{article.id}", params: params, headers: auth_headers expect(response).to have_http_status(:not_found) end @@ -964,7 +940,7 @@ RSpec.describe "Api::V1::Articles", type: :request do user.add_role(:super_admin) article = create(:article, user: create(:user)) params = { article: { title: "foobar" } }.to_json - put "/api/articles/#{article.id}", params: params, headers: put_headers + put "/api/articles/#{article.id}", params: params, headers: auth_headers expect(response).to have_http_status(:ok) end @@ -1083,7 +1059,7 @@ RSpec.describe "Api::V1::Articles", type: :request do body_markdown: "Yo ho ho", series: "a series" } } expect do - put "/api/articles/#{article.id}", params: params.to_json, headers: put_headers + put "/api/articles/#{article.id}", params: params.to_json, headers: auth_headers expect(response).to have_http_status(:ok) end.to change(Collection, :count).by(1) expect(article.reload.collection.user).to eq(article.user) @@ -1124,7 +1100,7 @@ RSpec.describe "Api::V1::Articles", type: :request do article = create(:article, user: create(:user)) params = { article: { title: Faker::Book.title, body_markdown: "Yo ho ho" } }.to_json - put "/api/articles/#{article.id}", params: params, headers: put_headers + put "/api/articles/#{article.id}", params: params, headers: auth_headers expect(response).to have_http_status(:ok) expect(article.reload.edited_at).to be_nil end @@ -1134,7 +1110,7 @@ RSpec.describe "Api::V1::Articles", type: :request do new_article = create(:article, user: create(:user)) params = { article: { title: Faker::Book.title } }.to_json expect do - put "/api/articles/#{new_article.id}", params: params, headers: put_headers + put "/api/articles/#{new_article.id}", params: params, headers: auth_headers article.reload end.not_to change(article, :edited_at) end @@ -1171,7 +1147,7 @@ RSpec.describe "Api::V1::Articles", type: :request do it "fails if params are not a Hash" do # Not using the nifty put_article helper method because it expects a Hash string_params = "this_string_is_definitely_not_a_hash" - put path, params: { article: string_params }.to_json, headers: put_headers + put path, params: { article: string_params }.to_json, headers: auth_headers expect(response).to have_http_status(:unprocessable_entity) expect(response.parsed_body["error"]).to be_present @@ -1194,19 +1170,17 @@ RSpec.describe "Api::V1::Articles", type: :request do context "when unauthorized" do it "fails with no api key" do - put path, headers: { "content-type" => "application/json", "Accept" => "application/vnd.forem.api-v1+json" } + put path, headers: headers expect(response).to have_http_status(:unauthorized) end it "fails with the wrong api key" do - put path, - headers: { "api-key" => "foobar", "content-type" => "application/json", - "Accept" => "application/vnd.forem.api-v1+json" } + put path, headers: headers.merge({ "api-key" => "foobar" }) expect(response).to have_http_status(:unauthorized) end it "fails without elevated_user?" do - put path, headers: v1_headers + put path, headers: auth_headers expect(response).to have_http_status(:unauthorized) end end @@ -1216,7 +1190,7 @@ RSpec.describe "Api::V1::Articles", type: :request do it "unpublishes an article" do expect(published_article.published).to be true - put path, headers: v1_headers + put path, headers: auth_headers expect(response).to have_http_status(:no_content) expect(published_article.reload.published).to be false end @@ -1227,13 +1201,13 @@ RSpec.describe "Api::V1::Articles", type: :request do it "unpublishes an article" do expect(published_article.published).to be true - put path, headers: v1_headers + put path, headers: auth_headers expect(response).to have_http_status(:no_content) expect(published_article.reload.published).to be false end it "creates an audit log of the action taken" do - put path, headers: v1_headers + put path, headers: auth_headers log = AuditLog.last expect(log.category).to eq(AuditLog::ADMIN_API_AUDIT_LOG_CATEGORY) diff --git a/spec/requests/api/v1/comments_spec.rb b/spec/requests/api/v1/comments_spec.rb index 6be42314f..f77faf32d 100644 --- a/spec/requests/api/v1/comments_spec.rb +++ b/spec/requests/api/v1/comments_spec.rb @@ -1,8 +1,7 @@ require "rails_helper" RSpec.describe "Api::V1::Comments", type: :request do - let(:api_secret) { create(:api_secret) } - let(:v1_headers) { { "api-key" => api_secret.secret, "Accept" => "application/vnd.forem.api-v1+json" } } + let(:headers) { { "Accept" => "application/vnd.forem.api-v1+json" } } let(:article) { create(:article) } let!(:root_comment) { create(:comment, commentable: article) } let!(:child_comment) do @@ -40,26 +39,21 @@ RSpec.describe "Api::V1::Comments", type: :request do before { allow(FeatureFlag).to receive(:enabled?).with(:api_v1).and_return(true) } describe "GET /api/comments" do - it "returns 401 when unauthenticated" do - get api_comments_path(a_id: article.id), headers: { "Accept" => "application/vnd.forem.api-v1+json" } - expect(response).to have_http_status(:unauthorized) - end - it "returns not found if wrong article id" do - get api_comments_path(a_id: "gobbledygook"), headers: v1_headers + get api_comments_path(a_id: "gobbledygook"), headers: headers expect(response).to have_http_status(:not_found) end it "returns comments for article" do - get api_comments_path(a_id: article.id), headers: v1_headers + get api_comments_path(a_id: article.id), headers: headers expect(response).to have_http_status(:ok) expect(response.parsed_body.size).to eq(1) end it "does not include children comments in the root list" do - get api_comments_path(a_id: article.id), headers: v1_headers + get api_comments_path(a_id: article.id), headers: headers expected_ids = article.comments.roots.map(&:id_code_generated) response_ids = response.parsed_body.map { |cm| cm["id_code"] } @@ -67,14 +61,14 @@ RSpec.describe "Api::V1::Comments", type: :request do end it "includes children comments in the children list" do - get api_comments_path(a_id: article.id), headers: v1_headers + get api_comments_path(a_id: article.id), headers: headers child_comment_json = find_child_comment(response) expect(child_comment_json["id_code"]).to eq(child_comment.id_code_generated) end it "includes grandchildren comments in the children-children list" do - get api_comments_path(a_id: article.id), headers: v1_headers + get api_comments_path(a_id: article.id), headers: headers root_comment_json = find_root_comment(response) grandchild_comment_json_id = root_comment_json.dig( @@ -84,7 +78,7 @@ RSpec.describe "Api::V1::Comments", type: :request do end it "includes great-grandchildren comments in the children-children-children list" do - get api_comments_path(a_id: article.id), headers: v1_headers + get api_comments_path(a_id: article.id), headers: headers root_comment_json = find_root_comment(response) great_grandchild_comment_json_id = root_comment_json.dig( @@ -96,7 +90,7 @@ RSpec.describe "Api::V1::Comments", type: :request do it "sets the correct edge caching surrogate key for all the comments" do sibling_root_comment = create(:comment, commentable: article) - get api_comments_path(a_id: article.id), headers: v1_headers + get api_comments_path(a_id: article.id), headers: headers expected_key = [ article.record_key, "comments", sibling_root_comment.record_key, @@ -107,7 +101,7 @@ RSpec.describe "Api::V1::Comments", type: :request do end it "returns date created" do - get api_comments_path(a_id: article.id), headers: v1_headers + get api_comments_path(a_id: article.id), headers: headers expect(find_root_comment(response)).to include( "created_at" => root_comment.created_at.utc.iso8601, ) @@ -119,25 +113,25 @@ RSpec.describe "Api::V1::Comments", type: :request do end it "appears in the thread" do - get api_comments_path(a_id: article.id), headers: v1_headers + get api_comments_path(a_id: article.id), headers: headers expect(find_child_comment(response)["id_code"]).to eq(child_comment.id_code_generated) end it "replaces the body_html" do - get api_comments_path(a_id: article.id), headers: v1_headers + get api_comments_path(a_id: article.id), headers: headers expect(find_child_comment(response)["body_html"]).to eq("
#{Comment.title_deleted}
") end it "does not render the user information" do - get api_comments_path(a_id: article.id), headers: v1_headers + get api_comments_path(a_id: article.id), headers: headers expect(find_child_comment(response)["user"]).to be_empty end it "still has children comments" do - get api_comments_path(a_id: article.id), headers: v1_headers + get api_comments_path(a_id: article.id), headers: headers expect(find_child_comment(response)["children"]).not_to be_empty end @@ -149,25 +143,25 @@ RSpec.describe "Api::V1::Comments", type: :request do end it "appears in the thread" do - get api_comments_path(a_id: article.id), headers: v1_headers + get api_comments_path(a_id: article.id), headers: headers expect(find_child_comment(response)["id_code"]).to eq(child_comment.id_code_generated) end it "replaces the body_html" do - get api_comments_path(a_id: article.id), headers: v1_headers + get api_comments_path(a_id: article.id), headers: headers expect(find_child_comment(response)["body_html"]).to eq("#{Comment.title_hidden}
") end it "does not render the user information" do - get api_comments_path(a_id: article.id), headers: v1_headers + get api_comments_path(a_id: article.id), headers: headers expect(find_child_comment(response)["user"]).to be_empty end it "still has children comments" do - get api_comments_path(a_id: article.id), headers: v1_headers + get api_comments_path(a_id: article.id), headers: headers expect(find_child_comment(response)["children"]).not_to be_empty end @@ -181,12 +175,12 @@ RSpec.describe "Api::V1::Comments", type: :request do before { comment } it "not found if bad podcast episode id" do - get api_comments_path(p_id: "asdfghjkl"), headers: v1_headers + get api_comments_path(p_id: "asdfghjkl"), headers: headers expect(response).to have_http_status(:not_found) end it "returns comment if good podcast episode id" do - get api_comments_path(p_id: podcast_episode.id), headers: v1_headers + get api_comments_path(p_id: podcast_episode.id), headers: headers expect(response).to have_http_status(:ok) expect(response.parsed_body.size).to eq(1) end @@ -194,32 +188,27 @@ RSpec.describe "Api::V1::Comments", type: :request do end describe "GET /api/comments/:id" do - it "returns 401 when unauthenticated" do - get api_comment_path(root_comment.id_code_generated), headers: { "Accept" => "application/vnd.forem.api-v1+json" } - expect(response).to have_http_status(:unauthorized) - end - it "returns not found if wrong comment id" do - get api_comment_path("foobar"), headers: v1_headers + get api_comment_path("foobar"), headers: headers expect(response).to have_http_status(:not_found) end it "returns the comment" do - get api_comment_path(root_comment.id_code_generated), headers: v1_headers + get api_comment_path(root_comment.id_code_generated), headers: headers expect(response).to have_http_status(:ok) expect(response.parsed_body["id_code"]).to eq(root_comment.id_code_generated) end it "includes children comments in the children list" do - get api_comment_path(root_comment.id_code_generated), headers: v1_headers + get api_comment_path(root_comment.id_code_generated), headers: headers expect(find_child_comment(response, :show)["id_code"]).to eq(child_comment.id_code_generated) end it "includes grandchildren comments in the children-children list" do - get api_comment_path(root_comment.id_code_generated), headers: v1_headers + get api_comment_path(root_comment.id_code_generated), headers: headers grandchild_comment_json_id = response.parsed_body.dig( "children", 0, "children", 0, "id_code" @@ -228,7 +217,7 @@ RSpec.describe "Api::V1::Comments", type: :request do end it "includes great-grandchildren comments in the children-children-children list" do - get api_comment_path(root_comment.id_code_generated), headers: v1_headers + get api_comment_path(root_comment.id_code_generated), headers: headers great_grandchild_comment_json_id = response.parsed_body.dig( "children", 0, "children", 0, "children", 0, "id_code" @@ -237,7 +226,7 @@ RSpec.describe "Api::V1::Comments", type: :request do end it "sets the correct edge caching surrogate key for all the comments" do - get api_comment_path(root_comment.id_code_generated), headers: v1_headers + get api_comment_path(root_comment.id_code_generated), headers: headers expected_key = [ "comments", root_comment.record_key, child_comment.record_key, @@ -252,25 +241,25 @@ RSpec.describe "Api::V1::Comments", type: :request do end it "appears in the thread" do - get api_comment_path(root_comment.id_code_generated), headers: v1_headers + get api_comment_path(root_comment.id_code_generated), headers: headers expect(find_child_comment(response, :show)["id_code"]).to eq(child_comment.id_code_generated) end it "replaces the body_html" do - get api_comment_path(root_comment.id_code_generated), headers: v1_headers + get api_comment_path(root_comment.id_code_generated), headers: headers expect(find_child_comment(response, :show)["body_html"]).to eq("[deleted]
") end it "does not render the user information" do - get api_comment_path(root_comment.id_code_generated), headers: v1_headers + get api_comment_path(root_comment.id_code_generated), headers: headers expect(find_child_comment(response, :show)["user"]).to be_empty end it "still has children comments" do - get api_comment_path(root_comment.id_code_generated), headers: v1_headers + get api_comment_path(root_comment.id_code_generated), headers: headers expect(find_child_comment(response, :show)["children"]).not_to be_empty end @@ -282,25 +271,25 @@ RSpec.describe "Api::V1::Comments", type: :request do end it "appears in the thread" do - get api_comment_path(root_comment.id_code_generated), headers: v1_headers + get api_comment_path(root_comment.id_code_generated), headers: headers expect(find_child_comment(response, :show)["id_code"]).to eq(child_comment.id_code_generated) end it "replaces the body_html" do - get api_comment_path(root_comment.id_code_generated), headers: v1_headers + get api_comment_path(root_comment.id_code_generated), headers: headers expect(find_child_comment(response, :show)["body_html"]).to eq("[hidden by post author]
") end it "does not render the user information" do - get api_comment_path(root_comment.id_code_generated), headers: v1_headers + get api_comment_path(root_comment.id_code_generated), headers: headers expect(find_child_comment(response, :show)["user"]).to be_empty end it "still has children comments" do - get api_comment_path(root_comment.id_code_generated), headers: v1_headers + get api_comment_path(root_comment.id_code_generated), headers: headers expect(find_child_comment(response, :show)["children"]).not_to be_empty end diff --git a/spec/requests/api/v1/docs/articles_spec.rb b/spec/requests/api/v1/docs/articles_spec.rb index 51914e077..6336910b7 100644 --- a/spec/requests/api/v1/docs/articles_spec.rb +++ b/spec/requests/api/v1/docs/articles_spec.rb @@ -25,6 +25,7 @@ RSpec.describe "Api::V1::Docs::Articles", type: :request do path "/api/articles" do get "Published articles" do + security [] tags "articles" description "This endpoint allows the client to retrieve a list of articles. @@ -93,18 +94,13 @@ belonging to the requested collection, ordered by ascending publication date.", example: 99 response "200", "A List of Articles" do - let(:"api-key") { api_secret.secret } + let(:"api-key") { nil } schema type: :array, items: { "$ref": "#/components/schemas/ArticleIndex" } add_examples run_test! end - - response "401", "unauthorized" do - let(:"api-key") { "invalid" } - run_test! - end end end end diff --git a/spec/requests/api/v1/feature_flags_spec.rb b/spec/requests/api/v1/feature_flags_spec.rb index 7542cad3f..b1e6f9bdf 100644 --- a/spec/requests/api/v1/feature_flags_spec.rb +++ b/spec/requests/api/v1/feature_flags_spec.rb @@ -3,8 +3,7 @@ require "rails_helper" RSpec.describe "Api::V1::FeatureFlagsController", type: :request do let(:flag) { "test_flag" } let(:params) { { flag: flag } } - let(:api_secret) { create(:api_secret) } - let(:v1_headers) { { "Accept" => "application/vnd.forem.api-v1+json" } } + let(:headers) { { "Accept" => "application/vnd.forem.api-v1+json" } } before { allow(FeatureFlag).to receive(:enabled?).with(:api_v1).and_return(true) } @@ -15,7 +14,7 @@ RSpec.describe "Api::V1::FeatureFlagsController", type: :request do # rubocop:enable Rails/Inquiry expect do - post api_feature_flags_path, params: params, headers: v1_headers + post api_feature_flags_path, params: params, headers: headers end.to raise_error(ActionController::RoutingError) end @@ -28,7 +27,7 @@ RSpec.describe "Api::V1::FeatureFlagsController", type: :request do FeatureFlag.disable(flag) expect do - post api_feature_flags_path, params: params, headers: v1_headers + post api_feature_flags_path, params: params, headers: headers end.to change { FeatureFlag.enabled?(flag) }.from(false).to(true) end @@ -36,7 +35,7 @@ RSpec.describe "Api::V1::FeatureFlagsController", type: :request do FeatureFlag.enable(flag) expect do - post api_feature_flags_path, params: params, headers: v1_headers + post api_feature_flags_path, params: params, headers: headers end.not_to change { FeatureFlag.enabled?(flag) }.from(true) end @@ -44,7 +43,7 @@ RSpec.describe "Api::V1::FeatureFlagsController", type: :request do FeatureFlag.enable(flag) expect do - delete api_feature_flags_path, params: params, headers: v1_headers + delete api_feature_flags_path, params: params, headers: headers end.to change { FeatureFlag.enabled?(flag) }.from(true).to(false) end @@ -52,14 +51,14 @@ RSpec.describe "Api::V1::FeatureFlagsController", type: :request do FeatureFlag.disable(flag) expect do - delete api_feature_flags_path, params: params, headers: v1_headers + delete api_feature_flags_path, params: params, headers: headers end.not_to change { FeatureFlag.enabled?(flag) }.from(false) end it "shows the current value of a feature flag" do FeatureFlag.enable(flag) - get api_feature_flags_path(flag: flag), headers: v1_headers + get api_feature_flags_path(flag: flag), headers: headers parsed_response = JSON.parse(response.body) expect(parsed_response[flag]).to be true diff --git a/spec/requests/api/v1/followers_spec.rb b/spec/requests/api/v1/followers_spec.rb index 11faa1a30..e5255049b 100644 --- a/spec/requests/api/v1/followers_spec.rb +++ b/spec/requests/api/v1/followers_spec.rb @@ -1,11 +1,9 @@ require "rails_helper" RSpec.describe "Api::V1::FollowersController", type: :request do - # TODO: Find resolution for current_user authentication - - let(:api_secret) { create(:api_secret) } - let(:user) { api_secret.user } - let(:v1_headers) { { "api-key" => api_secret.secret, "Accept" => "application/vnd.forem.api-v1+json" } } + let(:user) { create(:user) } + let(:api_secret) { create(:api_secret, user: user) } + let(:headers) { { "api-key" => api_secret.secret, "Accept" => "application/vnd.forem.api-v1+json" } } let(:follower) { create(:user) } let(:follower2) { create(:user) } @@ -20,15 +18,24 @@ RSpec.describe "Api::V1::FollowersController", type: :request do context "when user is unauthorized" do it "returns unauthorized" do - get api_followers_users_path, headers: { "Accept" => "application/vnd.forem.api-v1+json" } + get api_followers_users_path expect(response).to have_http_status(:unauthorized) end end + context "when the user is authorized as current_user" do + it "returns ok" do + sign_in user + + get api_followers_users_path + expect(response).to have_http_status(:ok) + end + end + context "when user is authorized with api key" do it "returns user's followers list with the correct format" do - get api_followers_users_path, headers: v1_headers + get api_followers_users_path, headers: headers expect(response).to have_http_status(:ok) response_follower = response.parsed_body.first @@ -44,20 +51,20 @@ RSpec.describe "Api::V1::FollowersController", type: :request do it "supports pagination" do follower2.follow(user) - get api_followers_users_path, params: { page: 1, per_page: 1 }, headers: v1_headers + get api_followers_users_path, params: { page: 1, per_page: 1 }, headers: headers expect(response.parsed_body.length).to eq(1) - get api_followers_users_path, params: { page: 2, per_page: 1 }, headers: v1_headers + get api_followers_users_path, params: { page: 2, per_page: 1 }, headers: headers expect(response.parsed_body.length).to eq(1) - get api_followers_users_path, params: { page: 3, per_page: 1 }, headers: v1_headers + get api_followers_users_path, params: { page: 3, per_page: 1 }, headers: headers expect(response.parsed_body.length).to eq(0) end it "orders results by descending following date by default" do follower2.follow(user) - get api_followers_users_path, headers: v1_headers + get api_followers_users_path, headers: headers follows = user.followings.order(id: :desc).last(2).map(&:id) result = response.parsed_body.map { |f| f["id"] } @@ -68,7 +75,7 @@ RSpec.describe "Api::V1::FollowersController", type: :request do follower2.follow(user) follows = user.followings.order(id: :asc).last(2).map(&:id) - get api_followers_users_path, headers: v1_headers, params: { sort: "created_at" } + get api_followers_users_path, headers: headers, params: { sort: "created_at" } result = response.parsed_body.map { |f| f["id"] } expect(result).to eq(follows) end diff --git a/spec/requests/api/v1/follows_spec.rb b/spec/requests/api/v1/follows_spec.rb index 56e552dcb..cf5488101 100644 --- a/spec/requests/api/v1/follows_spec.rb +++ b/spec/requests/api/v1/follows_spec.rb @@ -1,8 +1,6 @@ require "rails_helper" RSpec.describe "Api::V1::FollowsController", type: :request do - let(:api_secret) { create(:api_secret) } - let(:user) { api_secret.user } before { allow(FeatureFlag).to receive(:enabled?).with(:api_v1).and_return(true) } diff --git a/spec/requests/api/v1/instances_spec.rb b/spec/requests/api/v1/instances_spec.rb index a2cf83a21..cf38786c4 100644 --- a/spec/requests/api/v1/instances_spec.rb +++ b/spec/requests/api/v1/instances_spec.rb @@ -1,14 +1,14 @@ require "rails_helper" RSpec.describe "Api::V1::Instances", type: :request do - let(:v1_headers) { { "Accept" => "application/vnd.forem.api-v1+json" } } before { allow(FeatureFlag).to receive(:enabled?).with(:api_v1).and_return(true) } + let(:headers) { { "Accept" => "application/vnd.forem.api-v1+json" } } describe "GET /api/instance" do it "returns the correct attributes", :aggregate_failures do create(:user) - get api_instance_path, headers: v1_headers + get api_instance_path, headers: headers expect(response.parsed_body["context"]).to eq ApplicationConfig["FOREM_CONTEXT"] expect(response.parsed_body["cover_image_url"]).to eq Settings::General.main_social_image @@ -26,7 +26,7 @@ RSpec.describe "Api::V1::Instances", type: :request do it "returns public for visibility" do allow(Settings::General).to receive(:waiting_on_first_user).and_return(false) allow(Settings::UserExperience).to receive(:public).and_return(true) - get api_instance_path, headers: v1_headers + get api_instance_path, headers: headers expect(response.parsed_body["visibility"]).to eq "public" end @@ -36,7 +36,7 @@ RSpec.describe "Api::V1::Instances", type: :request do it "returns private for visibility" do allow(Settings::General).to receive(:waiting_on_first_user).and_return(false) allow(Settings::UserExperience).to receive(:public).and_return(false) - get api_instance_path, headers: v1_headers + get api_instance_path, headers: headers expect(response.parsed_body["visibility"]).to eq "private" end @@ -45,7 +45,7 @@ RSpec.describe "Api::V1::Instances", type: :request do context "when the Forem is pending" do it "returns pending for visibility" do allow(Settings::General).to receive(:waiting_on_first_user).and_return(true) - get api_instance_path, headers: v1_headers + get api_instance_path, headers: headers expect(response.parsed_body["visibility"]).to eq "pending" end diff --git a/spec/requests/api/v1/listings_spec.rb b/spec/requests/api/v1/listings_spec.rb index f2881b82f..0bbea2f2a 100644 --- a/spec/requests/api/v1/listings_spec.rb +++ b/spec/requests/api/v1/listings_spec.rb @@ -8,7 +8,6 @@ RSpec.describe "Api::V1::Listings", type: :request do create(:listing_category) end - let!(:v1_headers) { { "content-type" => "application/json", "Accept" => "application/vnd.forem.api-v1+json" } } before { allow(FeatureFlag).to receive(:enabled?).with(:api_v1).and_return(true) } @@ -16,7 +15,6 @@ RSpec.describe "Api::V1::Listings", type: :request do let(:api_secret) { create(:api_secret) } let(:user) { api_secret.user } let(:api_secret_org) { create(:api_secret, :org_admin) } - let(:headers) { v1_headers.merge({ "api-key" => api_secret }) } end shared_context "when param list is valid" do @@ -62,116 +60,80 @@ RSpec.describe "Api::V1::Listings", type: :request do describe "GET /api/listings" do include_context "with 7 listings and 2 user" - xcontext "when unauthenticated" do - it "returns unauthorized" do - get api_listings_path, headers: { "Accept" => "application/vnd.forem.api-v1+json" } - expect(response).to have_http_status(:unauthorized) - end + it "returns json response and ok status" do + get api_listings_path + + expect(response.media_type).to eq("application/json") + expect(response).to have_http_status(:ok) end - xcontext "when unauthorized" do - it "returns unauthorized" do - get api_listings_path, headers: v1_headers.merge({ "api-key" => "invalid api key" }) - expect(response).to have_http_status(:unauthorized) - end + it "returns listings created" do + get api_listings_path + expect(response.parsed_body.size).to eq(7) + expect(response.parsed_body.first["type_of"]).to eq("listing") + expect(response.parsed_body.first["slug"]).to eq(Listing.last.slug) + expect(response.parsed_body.first["user"]).to include("username") + expect(response.parsed_body.first["user"]["username"]).not_to be_empty end - context "when authorized" do - include_context "when user is authorized" + it "supports pagination" do + get api_listings_path, params: { page: 2, per_page: 2 } + expect(response.parsed_body.length).to eq(2) + get api_listings_path, params: { page: 4, per_page: 2 } + expect(response.parsed_body.length).to eq(1) + end - it "returns json response and ok status" do - get api_listings_path, headers: headers + it "sets the correct caching headers" do + get api_listings_path - expect(response.media_type).to eq("application/json") - expect(response).to have_http_status(:ok) - end + expect(response.headers["cache-control"]).to be_present + expect(response.headers["x-accel-expires"]).to be_present + expect(response.headers["surrogate-control"]).to match(/max-age/).and(match(/stale-if-error/)) + end - it "returns listings created" do - get api_listings_path, headers: headers - expect(response.parsed_body.size).to eq(7) - expect(response.parsed_body.first["type_of"]).to eq("listing") - expect(response.parsed_body.first["slug"]).to eq(Listing.last.slug) - expect(response.parsed_body.first["user"]).to include("username") - expect(response.parsed_body.first["user"]["username"]).not_to be_empty - end + it "sets the correct edge caching surrogate key" do + get api_listings_path - it "supports pagination" do - get api_listings_path, params: { page: 2, per_page: 2 }, headers: headers - expect(response.parsed_body.length).to eq(2) - get api_listings_path, params: { page: 4, per_page: 2 }, headers: headers - expect(response.parsed_body.length).to eq(1) - end + expected_key = ( + ["classified_listings"] + + user1.listings.map(&:record_key) + + user2.listings.map(&:record_key) + ).to_set + expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key) + end - it "sets the correct caching headers" do - get api_listings_path, headers: headers + it "does not return unpublished listings" do + listing = user1.listings.last + listing.update(published: false) - expect(response.headers["cache-control"]).to be_present - expect(response.headers["x-accel-expires"]).to be_present - expect(response.headers["surrogate-control"]).to match(/max-age/).and(match(/stale-if-error/)) - end + get api_listings_path + expect(response.parsed_body.detect { |l| l["published"] == false }).to be_nil + end - it "sets the correct edge caching surrogate key" do - get api_listings_path, headers: headers - - expected_key = ( - ["classified_listings"] + - user1.listings.map(&:record_key) + - user2.listings.map(&:record_key) - ).to_set - expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key) - end - - it "does not return unpublished listings" do - listing = user1.listings.last - listing.update(published: false) - - get api_listings_path, headers: headers - expect(response.parsed_body.detect { |l| l["published"] == false }).to be_nil - end - - # Regression test for https://github.com/forem/forem/issues/14436 - it "includes the created at timestamp for listings" do - get api_listings_path, headers: headers - listing = response.parsed_body.first - expect(listing["created_at"]).to match(/\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z/) - end + # Regression test for https://github.com/forem/forem/issues/14436 + it "includes the created at timestamp for listings" do + get api_listings_path + listing = response.parsed_body.first + expect(listing["created_at"]).to match(/\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z/) end end describe "GET /api/listings/category/:category" do include_context "with 7 listings and 2 user" - xcontext "when unauthenticated" do - it "returns unauthorized" do - get api_listings_category_path("cfp"), headers: v1_headers - expect(response).to have_http_status(:unauthorized) - end + it "displays only listings from the cfp category" do + get api_listings_category_path("cfp") + expect(response).to have_http_status(:ok) + expect(response.parsed_body.size).to eq(3) end - xcontext "when unauthorized" do - it "returns unauthorized" do - get api_listings_category_path("cfp"), headers: v1_headers.merge({ "api-key" => "invalid api key" }) - expect(response).to have_http_status(:unauthorized) - end - end + it "does not return unpublished listings" do + category = "cfp" + listing = user1.listings.in_category(category).last + listing.update(published: false) - context "when authorized" do - include_context "when user is authorized" - - it "displays only listings from the cfp category" do - get api_listings_category_path("cfp"), headers: headers - expect(response).to have_http_status(:ok) - expect(response.parsed_body.size).to eq(3) - end - - it "does not return unpublished listings" do - category = "cfp" - listing = user1.listings.in_category(category).last - listing.update(published: false) - - get api_listings_category_path(category), headers: headers - expect(response.parsed_body.detect { |l| l["published"] == false }).to be_nil - end + get api_listings_category_path(category) + expect(response.parsed_body.detect { |l| l["published"] == false }).to be_nil end end @@ -179,28 +141,53 @@ RSpec.describe "Api::V1::Listings", type: :request do include_context "with 7 listings and 2 user" let(:listing) { Listing.in_category("cfp").last } - xcontext "when unauthenticated" do - it "returns unauthorized" do + context "when unauthenticated" do + it "returns a published listing" do listing.update(published: true) - get api_listing_path(listing.id), headers: v1_headers + get api_listing_path(listing.id) + expect(response).to have_http_status(:ok) + end - expect(response).to have_http_status(:unauthorized) + it "returns a published listing on behalf of an organization" do + org = user_admin_organization(listing.user) + listing.update(published: true, organization: org) + + get api_listing_path(listing.id) + expect(response).to have_http_status(:ok) + end + + it "does not return an unpublished listing" do + listing.update(published: false) + + get api_listing_path(listing.id) + expect(response).to have_http_status(:not_found) end end - xcontext "when unauthorized" do - it "returns unauthorized" do + context "when unauthorized" do + let(:headers) { { "api-key" => "invalid api key", "Accept" => "application/vnd.forem.api-v1+json" } } + + it "returns a published listing" do listing.update(published: true) - get api_listing_path(listing.id), headers: v1_headers.merge({ "api-key" => "invalid api key" }) - expect(response).to have_http_status(:unauthorized) + get api_listing_path(listing.id), headers: headers + expect(response).to have_http_status(:ok) + end + + it "does not return an unpublished listing" do + listing.update(published: false) + + get api_listing_path(listing.id), headers: headers + expect(response).to have_http_status(:not_found) end end context "when authorized" do include_context "when user is authorized" + let(:headers) { { "api-key" => api_secret.secret, "Accept" => "application/vnd.forem.api-v1+json" } } + it "returns a published listing" do listing.update(published: true) @@ -215,44 +202,45 @@ RSpec.describe "Api::V1::Listings", type: :request do expect(response).to have_http_status(:not_found) end - xit "returns an unpublished listing belonging to the authenticated user" do + it "returns an unpublished listing belonging to the authenticated user" do listing.update(published: false, user: api_secret.user) get api_listing_path(listing.id), headers: headers expect(response).to have_http_status(:ok) end + end - it "returns the correct listing format" do - get api_listing_path(listing.id), headers: headers + it "returns the correct listing format" do + get api_listing_path(listing.id) - expect(response).to have_http_status(:ok) + expect(response).to have_http_status(:ok) - expect(response.parsed_body["type_of"]).to eq("listing") - expect(response.parsed_body["slug"]).to eq(listing.slug) - expect(response.parsed_body["user"]).to include("username") - expect(response.parsed_body["user"]["username"]).not_to be_empty - end + expect(response.parsed_body["type_of"]).to eq("listing") + expect(response.parsed_body["slug"]).to eq(listing.slug) + expect(response.parsed_body["user"]).to include("username") + expect(response.parsed_body["user"]["username"]).not_to be_empty + end - it "sets the correct caching headers" do - get api_listing_path(listing.id), headers: headers + it "sets the correct caching headers" do + get api_listing_path(listing.id) - expect(response.headers["cache-control"]).to be_present - expect(response.headers["x-accel-expires"]).to be_present - expect(response.headers["surrogate-control"]).to match(/max-age/).and(match(/stale-if-error/)) - end + expect(response.headers["cache-control"]).to be_present + expect(response.headers["x-accel-expires"]).to be_present + expect(response.headers["surrogate-control"]).to match(/max-age/).and(match(/stale-if-error/)) + end - it "sets the correct edge caching surrogate key" do - get api_listing_path(listing.id), headers: headers + it "sets the correct edge caching surrogate key" do + get api_listing_path(listing.id) - expected_key = [listing.record_key].to_set - expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key) - end + expected_key = [listing.record_key].to_set + expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key) end end describe "POST /api/listings" do def post_listing(key: api_secret.secret, **params) - headers = v1_headers.merge({ "api-key" => key }) + headers = { "api-key" => key, "content-type" => "application/json", + "Accept" => "application/vnd.forem.api-v1+json" } post api_listings_path, params: { listing: params }.to_json, headers: headers end @@ -260,12 +248,15 @@ RSpec.describe "Api::V1::Listings", type: :request do let(:api_secret) { create(:api_secret) } it "fails with no api key" do - post api_listings_path, headers: v1_headers + post api_listings_path, + headers: { "content-type" => "application/json", "Accept" => "application/vnd.forem.api-v1+json" } expect(response).to have_http_status(:unauthorized) end it "fails with the wrong api key" do - post api_listings_path, headers: v1_headers.merge({ "api-key" => "foobar" }) + post api_listings_path, + headers: { "api-key" => "foobar", "content-type" => "application/json", + "Accept" => "application/vnd.forem.api-v1+json" } expect(response).to have_http_status(:unauthorized) end end @@ -449,7 +440,8 @@ RSpec.describe "Api::V1::Listings", type: :request do it "reads the 'classified_listing' key when listing not present" do # this is like post_listing but uses the "classified_listing" key key = api_secret.secret - headers = { "api-key" => key, "content-type" => "application/json" } + headers = { "api-key" => key, "content-type" => "application/json", + "Accept" => "application/vnd.forem.api-v1+json" } expect do post api_listings_path, params: { classified_listing: listing_params }.to_json, headers: headers @@ -461,7 +453,8 @@ RSpec.describe "Api::V1::Listings", type: :request do describe "PUT /api/listings/:id" do def put_listing(id, **params) - headers = v1_headers.merge({ "api-key" => api_secret.secret }) + headers = { "api-key" => api_secret.secret, "content-type" => "application/json", + "Accept" => "application/vnd.forem.api-v1+json" } put api_listing_path(id), params: { classified_listing: params }.to_json, headers: headers end @@ -483,12 +476,15 @@ RSpec.describe "Api::V1::Listings", type: :request do let(:api_secret) { create(:api_secret) } it "fails with no api key" do - put api_listing_path(listing.id), headers: v1_headers + put api_listing_path(listing.id), + headers: { "content-type" => "application/json", "Accept" => "application/vnd.forem.api-v1+json" } expect(response).to have_http_status(:unauthorized) end it "fails with the wrong api key" do - put api_listing_path(listing.id), headers: v1_headers.merge({ "api-key" => "foobar" }) + put api_listing_path(listing.id), + headers: { "api-key" => "foobar", "content-type" => "application/json", + "Accept" => "application/vnd.forem.api-v1+json" } expect(response).to have_http_status(:unauthorized) end end diff --git a/spec/requests/api/v1/organizations_spec.rb b/spec/requests/api/v1/organizations_spec.rb index 0b02e6aed..c02b65e73 100644 --- a/spec/requests/api/v1/organizations_spec.rb +++ b/spec/requests/api/v1/organizations_spec.rb @@ -1,35 +1,20 @@ require "rails_helper" RSpec.describe "Api::V1::Organizations", type: :request do - let(:api_secret) { create(:api_secret) } - let(:v1_headers) { { "api-key" => api_secret.secret, "Accept" => "application/vnd.forem.api-v1+json" } } + let(:headers) { { "Accept" => "application/vnd.forem.api-v1+json" } } describe "GET /api/organizations/:username" do let(:organization) { create(:organization) } before { allow(FeatureFlag).to receive(:enabled?).with(:api_v1).and_return(true) } - context "when unauthenticated" do - it "returns unauthorized" do - get "/api/organizations/invalid-username", headers: { "Accept" => "application/vnd.forem.api-v1+json" } - expect(response).to have_http_status(:unauthorized) - end - end - - context "when unauthorized" do - it "returns unauthorized" do - get "/api/organizations/invalid-username", headers: v1_headers.merge({ "api-key" => "invalid api key" }) - expect(response).to have_http_status(:unauthorized) - end - end - it "returns 404 if the organizations username is not found" do - get "/api/organizations/invalid-username", headers: v1_headers + get "/api/organizations/invalid-username", headers: headers expect(response).to have_http_status(:not_found) end it "returns the correct json representation of the organization", :aggregate_failures do - get api_organization_path(organization.username), headers: v1_headers + get api_organization_path(organization.username), headers: headers response_organization = response.parsed_body expect(response_organization).to include( @@ -54,40 +39,26 @@ RSpec.describe "Api::V1::Organizations", type: :request do before { allow(FeatureFlag).to receive(:enabled?).with(:api_v1).and_return(true) } - context "when unauthenticated" do - it "returns unauthorized" do - get "/api/organizations/invalid-username/users", headers: { "Accept" => "application/vnd.forem.api-v1+json" } - expect(response).to have_http_status(:unauthorized) - end - end - - context "when unauthorized" do - it "returns unauthorized" do - get "/api/organizations/invalid-username/users", headers: v1_headers.merge({ "api-key" => "invalid api key" }) - expect(response).to have_http_status(:unauthorized) - end - end - it "returns 404 if the organizations username is not found" do - get "/api/organizations/invalid-username/users", headers: v1_headers + get "/api/organizations/invalid-username/users", headers: headers expect(response).to have_http_status(:not_found) end it "supports pagination" do create(:organization_membership, user: create(:user), organization: organization) - get api_organization_users_path(organization.username), params: { page: 1, per_page: 1 }, headers: v1_headers + get api_organization_users_path(organization.username), params: { page: 1, per_page: 1 }, headers: headers expect(response.parsed_body.length).to eq(1) - get api_organization_users_path(organization.username), params: { page: 2, per_page: 1 }, headers: v1_headers + get api_organization_users_path(organization.username), params: { page: 2, per_page: 1 }, headers: headers expect(response.parsed_body.length).to eq(1) - get api_organization_users_path(organization.username), params: { page: 3, per_page: 1 }, headers: v1_headers + get api_organization_users_path(organization.username), params: { page: 3, per_page: 1 }, headers: headers expect(response.parsed_body.length).to eq(0) end it "returns the correct json representation of the organizations users", :aggregate_failures do - get api_organization_users_path(organization.username), headers: v1_headers + get api_organization_users_path(organization.username), headers: headers response_org_users = response.parsed_body.first @@ -114,48 +85,32 @@ RSpec.describe "Api::V1::Organizations", type: :request do before { allow(FeatureFlag).to receive(:enabled?).with(:api_v1).and_return(true) } - xcontext "when unauthenticated" do - it "returns unauthorized" do - get api_organization_listings_path(organization.username), - headers: { "Accept" => "application/vnd.forem.api-v1+json" } - expect(response).to have_http_status(:unauthorized) - end - end - - xcontext "when unauthorized" do - it "returns unauthorized" do - get api_organization_listings_path(organization.username), - headers: v1_headers.merge({ "api-key" => "invalid api key" }) - expect(response).to have_http_status(:unauthorized) - end - end - it "returns 404 if the organizations username is not found" do - get "/api/organizations/invalid-username/listings", headers: v1_headers + get "/api/organizations/invalid-username/listings", headers: headers expect(response).to have_http_status(:not_found) end it "returns success for when orgnaization username exists" do create(:listing, user: org_user, organization: organization) - get "/api/organizations/#{organization.username}/listings", headers: v1_headers + get "/api/organizations/#{organization.username}/listings", headers: headers expect(response).to have_http_status(:success) end it "supports pagination" do create(:listing, user: org_user, organization: organization) - get api_organization_listings_path(organization.username), params: { page: 1, per_page: 1 }, headers: v1_headers + get api_organization_listings_path(organization.username), params: { page: 1, per_page: 1 }, headers: headers expect(response.parsed_body.length).to eq(1) - get api_organization_listings_path(organization.username), params: { page: 2, per_page: 1 }, headers: v1_headers + get api_organization_listings_path(organization.username), params: { page: 2, per_page: 1 }, headers: headers expect(response.parsed_body.length).to eq(1) - get api_organization_listings_path(organization.username), params: { page: 3, per_page: 1 }, headers: v1_headers + get api_organization_listings_path(organization.username), params: { page: 3, per_page: 1 }, headers: headers expect(response.parsed_body.length).to eq(0) end it "returns the correct json representation of the organizations listings", :aggregate_failures do - get api_organization_listings_path(organization.username), headers: v1_headers + get api_organization_listings_path(organization.username), headers: headers response_listing = response.parsed_body.first expect(response_listing["type_of"]).to eq("listing") @@ -185,24 +140,8 @@ RSpec.describe "Api::V1::Organizations", type: :request do before { allow(FeatureFlag).to receive(:enabled?).with(:api_v1).and_return(true) } - context "when unauthenticated" do - it "returns unauthorized" do - get api_organization_articles_path(organization.username), - headers: { "Accept" => "application/vnd.forem.api-v1+json" } - expect(response).to have_http_status(:unauthorized) - end - end - - context "when unauthorized" do - it "returns unauthorized" do - get api_organization_articles_path(organization.username), - headers: v1_headers.merge({ "api-key" => "invalid api key" }) - expect(response).to have_http_status(:unauthorized) - end - end - it "returns 404 if the organizations articles is not found" do - get "/api/organizations/invalid-username/articles", headers: v1_headers + get "/api/organizations/invalid-username/articles", headers: headers expect(response).to have_http_status(:not_found) end @@ -211,22 +150,22 @@ RSpec.describe "Api::V1::Organizations", type: :request do get api_organization_articles_path(organization.username), params: { page: 1, per_page: 1 }, - headers: v1_headers + headers: headers expect(response.parsed_body.length).to eq(1) get api_organization_articles_path(organization.username), params: { page: 2, per_page: 1 }, - headers: v1_headers + headers: headers expect(response.parsed_body.length).to eq(1) get api_organization_articles_path(organization.username), params: { page: 3, per_page: 1 }, - headers: v1_headers + headers: headers expect(response.parsed_body.length).to eq(0) end it "returns the correct json representation of the organizations articles", :aggregate_failures do - get api_organization_articles_path(organization.username), headers: v1_headers + get api_organization_articles_path(organization.username), headers: headers response_article = response.parsed_body.first expect(response_article["type_of"]).to eq("article") diff --git a/spec/requests/api/v1/podcasts_episodes_spec.rb b/spec/requests/api/v1/podcasts_episodes_spec.rb index faff42ed9..db7e18695 100644 --- a/spec/requests/api/v1/podcasts_episodes_spec.rb +++ b/spec/requests/api/v1/podcasts_episodes_spec.rb @@ -2,34 +2,13 @@ require "rails_helper" RSpec.describe "Api::V1::PodcastEpisodes", type: :request do let(:podcast) { create(:podcast) } - let(:api_secret) { create(:api_secret) } - let(:v1_headers) { { "api-key" => api_secret.secret, "Accept" => "application/vnd.forem.api-v1+json" } } + let(:headers) { { "Accept" => "application/vnd.forem.api-v1+json" } } describe "GET /api/podcast_episodes" do before { allow(FeatureFlag).to receive(:enabled?).with(:api_v1).and_return(true) } - context "when unauthenticated" do - it "returns unauthorized" do - create(:podcast_episode, podcast: podcast) - - get api_podcast_episodes_path, headers: { "Accept" => "application/vnd.forem.api-v1+json" } - - expect(response).to have_http_status(:unauthorized) - end - end - - context "when unauthorized" do - it "returns unauthorized" do - create(:podcast_episode, podcast: podcast) - - get api_podcast_episodes_path, headers: v1_headers.merge({ "api-key" => "invalid api key" }) - - expect(response).to have_http_status(:unauthorized) - end - end - it "returns json response" do - get api_podcast_episodes_path, headers: v1_headers + get api_podcast_episodes_path, headers: headers expect(response.media_type).to eq("application/json") end @@ -37,7 +16,7 @@ RSpec.describe "Api::V1::PodcastEpisodes", type: :request do it "does not return unreachable podcasts" do create(:podcast_episode, reachable: false, podcast: podcast) - get api_podcast_episodes_path, headers: v1_headers + get api_podcast_episodes_path, headers: headers expect(response.parsed_body.size).to eq(0) end @@ -45,7 +24,7 @@ RSpec.describe "Api::V1::PodcastEpisodes", type: :request do it "does not return reachable podcast episodes belonging to unpublished podcasts" do pe = create(:podcast_episode, reachable: true, podcast: create(:podcast, published: false)) - get api_podcast_episodes_path, headers: v1_headers + get api_podcast_episodes_path, headers: headers expect(response.parsed_body.map { |e| e["id"] }).not_to include(pe.id.to_s) end @@ -53,7 +32,7 @@ RSpec.describe "Api::V1::PodcastEpisodes", type: :request do it "returns correct attributes for an episode", :aggregate_failures do podcast_episode = create(:podcast_episode, podcast: podcast) - get api_podcast_episodes_path, headers: v1_headers + get api_podcast_episodes_path, headers: headers response_episode = response.parsed_body.first expect(response_episode.keys).to match_array(%w[class_name type_of id path image_url title podcast]) @@ -69,7 +48,7 @@ RSpec.describe "Api::V1::PodcastEpisodes", type: :request do it "returns the episode's podcast json representation" do podcast_episode = create(:podcast_episode, podcast: podcast) - get api_podcast_episodes_path, headers: v1_headers + get api_podcast_episodes_path, headers: headers response_episode = response.parsed_body.first expect(response_episode["podcast"]["title"]).to eq(podcast_episode.podcast.title) @@ -81,24 +60,24 @@ RSpec.describe "Api::V1::PodcastEpisodes", type: :request do pe1 = create(:podcast_episode, published_at: 1.day.ago, podcast: podcast) pe2 = create(:podcast_episode, published_at: 1.day.from_now, podcast: podcast) - get api_podcast_episodes_path, headers: v1_headers + get api_podcast_episodes_path, headers: headers expect(response.parsed_body.map { |pe| pe["id"] }).to eq([pe2.id, pe1.id]) end it "supports pagination" do create_list(:podcast_episode, 3, podcast: podcast) - get api_podcast_episodes_path, params: { page: 1, per_page: 2 }, headers: v1_headers + get api_podcast_episodes_path, params: { page: 1, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(2) - get api_podcast_episodes_path, params: { page: 2, per_page: 2 }, headers: v1_headers + get api_podcast_episodes_path, params: { page: 2, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(1) end it "sets the correct edge caching surrogate key for all tags" do podcast_episode = create(:podcast_episode, reachable: true, podcast: podcast) - get api_podcast_episodes_path, headers: v1_headers + get api_podcast_episodes_path, headers: headers expected_key = ["podcast_episodes", podcast_episode.record_key].to_set expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key) @@ -109,7 +88,7 @@ RSpec.describe "Api::V1::PodcastEpisodes", type: :request do pe1 = create(:podcast_episode, podcast: podcast) create(:podcast_episode, podcast: create(:podcast)) - get api_podcast_episodes_path(username: podcast.slug), headers: v1_headers + get api_podcast_episodes_path(username: podcast.slug), headers: headers expect(response.parsed_body.map { |pe| pe["id"] }).to eq([pe1.id]) end @@ -117,7 +96,7 @@ RSpec.describe "Api::V1::PodcastEpisodes", type: :request do unavailable_podcast = create(:podcast, published: false) create(:podcast_episode, podcast: unavailable_podcast) - get api_podcast_episodes_path(username: unavailable_podcast.slug), headers: v1_headers + get api_podcast_episodes_path(username: unavailable_podcast.slug), headers: headers expect(response).to have_http_status(:not_found) end @@ -125,13 +104,13 @@ RSpec.describe "Api::V1::PodcastEpisodes", type: :request do it "returns not found if the podcast episode is unreachable" do create(:podcast_episode, reachable: false, podcast: podcast) - get api_podcast_episodes_path(username: podcast.slug), headers: v1_headers + get api_podcast_episodes_path(username: podcast.slug), headers: headers expect(response).to have_http_status(:not_found) end it "returns not found if the username does not exist" do - get api_podcast_episodes_path(username: "foobar"), headers: v1_headers + get api_podcast_episodes_path(username: "foobar"), headers: headers expect(response).to have_http_status(:not_found) end diff --git a/spec/requests/api/v1/readinglist_spec.rb b/spec/requests/api/v1/readinglist_spec.rb index 59aa44816..418c4c891 100644 --- a/spec/requests/api/v1/readinglist_spec.rb +++ b/spec/requests/api/v1/readinglist_spec.rb @@ -1,10 +1,10 @@ require "rails_helper" -RSpec.describe "Api::V0::ReadingList", type: :request do +RSpec.describe "Api::V1::ReadingList", type: :request do let(:user) { create(:user) } let(:user2) { create(:user) } let(:api_secret) { create(:api_secret, user: user) } - let(:v1_headers) { { "api-key" => api_secret.secret, "Accept" => "application/vnd.forem.api-v1+json" } } + let(:headers) { { "api-key" => api_secret.secret, "Accept" => "application/vnd.forem.api-v1+json" } } describe "GET /api/readinglist" do let(:readinglist) { create_list(:reading_reaction, 3, user: user) } @@ -20,14 +20,14 @@ RSpec.describe "Api::V0::ReadingList", type: :request do context "when unauthorized" do it "returns unauthorized" do - get api_readinglist_index_path, headers: v1_headers.merge({ "api-key" => "invalid api key" }) + get api_readinglist_index_path, headers: headers.merge({ "api-key" => "invalid api key" }) expect(response).to have_http_status(:unauthorized) end end context "when request is authenticated" do it "returns proper response specification" do - get api_readinglist_index_path, headers: v1_headers + get api_readinglist_index_path, headers: headers expect(response.media_type).to eq("application/json") expect(response).to have_http_status(:ok) end @@ -35,20 +35,20 @@ RSpec.describe "Api::V0::ReadingList", type: :request do it "doesn't return archived reactions" do create(:reading_reaction, user: user) create(:reading_reaction, user: user, status: :archived) - get api_readinglist_index_path, headers: v1_headers + get api_readinglist_index_path, headers: headers expect(response.parsed_body.length).to eq(1) end it "supports pagination" do create_list(:reading_reaction, 3, user: user) - get api_readinglist_index_path, headers: v1_headers, params: { page: 2, per_page: 2 } + get api_readinglist_index_path, headers: headers, params: { page: 2, per_page: 2 } expect(response.parsed_body.length).to eq(1) end it "doesn't return reading list items from other users" do create_list(:reading_reaction, 3, user: user) create_list(:reading_reaction, 2, user: user2) - get api_readinglist_index_path, headers: v1_headers + get api_readinglist_index_path, headers: headers expect(response.parsed_body.length).to eq(3) end end diff --git a/spec/requests/api/v1/tags_spec.rb b/spec/requests/api/v1/tags_spec.rb index 91cd04e6c..329bfa275 100644 --- a/spec/requests/api/v1/tags_spec.rb +++ b/spec/requests/api/v1/tags_spec.rb @@ -1,30 +1,15 @@ require "rails_helper" -RSpec.describe "Api::V0::Tags", type: :request do +RSpec.describe "Api::V1::Tags", type: :request do describe "GET /api/tags" do - let(:api_secret) { create(:api_secret) } - let(:v1_headers) { { "api-key" => api_secret.secret, "Accept" => "application/vnd.forem.api-v1+json" } } + let(:headers) { { "Accept" => "application/vnd.forem.api-v1+json" } } before { allow(FeatureFlag).to receive(:enabled?).with(:api_v1).and_return(true) } - context "when unauthenticated" do - it "returns unauthorized" do - get api_tags_path, headers: { "Accept" => "application/vnd.forem.api-v1+json" } - expect(response).to have_http_status(:unauthorized) - end - end - - context "when unauthorized" do - it "returns unauthorized" do - get api_tags_path, headers: v1_headers.merge({ "api-key" => "invalid api key" }) - expect(response).to have_http_status(:unauthorized) - end - end - it "returns tags" do create(:tag, taggings_count: 10) - get api_tags_path, headers: v1_headers + get api_tags_path, headers: headers expect(response.parsed_body.size).to eq(1) end @@ -34,7 +19,7 @@ RSpec.describe "Api::V0::Tags", type: :request do tag = create(:tag, taggings_count: 10) tag_with_badge = create(:tag, taggings_count: 5, badge: badge) - get api_tags_path, headers: v1_headers + get api_tags_path, headers: headers response_tag = response.parsed_body.first response_tag_with_badge = response.parsed_body.last @@ -46,7 +31,7 @@ RSpec.describe "Api::V0::Tags", type: :request do tag = create(:tag, taggings_count: 10) other_tag = create(:tag, taggings_count: tag.taggings_count + 1) - get api_tags_path, headers: v1_headers + get api_tags_path, headers: headers expected_result = [other_tag.id, tag.id] expect(response.parsed_body.map { |t| t["id"] }).to eq(expected_result) @@ -55,17 +40,17 @@ RSpec.describe "Api::V0::Tags", type: :request do it "supports pagination" do create_list(:tag, 3) - get api_tags_path, params: { page: 1, per_page: 2 }, headers: v1_headers + get api_tags_path, params: { page: 1, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(2) - get api_tags_path, params: { page: 2, per_page: 2 }, headers: v1_headers + get api_tags_path, params: { page: 2, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(1) end it "sets the correct edge caching surrogate key for all tags" do tag = create(:tag, taggings_count: 10) - get api_tags_path, headers: v1_headers + get api_tags_path, headers: headers expected_key = ["tags", tag.record_key].to_set expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key) diff --git a/spec/requests/api/v1/users_spec.rb b/spec/requests/api/v1/users_spec.rb index 00a4a7814..b62da68b7 100644 --- a/spec/requests/api/v1/users_spec.rb +++ b/spec/requests/api/v1/users_spec.rb @@ -1,8 +1,9 @@ require "rails_helper" -RSpec.describe "Api::V0::Users", type: :request do +RSpec.describe "Api::V1::Users", type: :request do let(:api_secret) { create(:api_secret) } - let(:v1_headers) { { "api-key" => api_secret.secret, "Accept" => "application/vnd.forem.api-v1+json" } } + let(:headers) { { "Accept" => "application/vnd.forem.api-v1+json" } } + let(:auth_headers) { headers.merge({ "api-key" => api_secret.secret }) } let(:listener) { :admin_api } describe "GET /api/users/:id" do @@ -15,54 +16,36 @@ RSpec.describe "Api::V0::Users", type: :request do profile: create(:profile, summary: "Something something")) end - context "when unauthenticated" do - it "returns unauthorized" do - get api_user_path("by_username"), - params: { url: user.username }, - headers: { "Accept" => "application/vnd.forem.api-v1+json" } - expect(response).to have_http_status(:unauthorized) - end - end - - context "when unauthorized" do - it "returns unauthorized" do - get api_user_path("by_username"), - params: { url: user.username }, - headers: v1_headers.merge({ "api-key" => "invalid api key" }) - expect(response).to have_http_status(:unauthorized) - end - end - it "returns 404 if the user id is not found" do - get api_user_path("invalid-id") + get api_user_path("invalid-id"), headers: headers expect(response).to have_http_status(:not_found) end it "returns 404 if the user username is not found" do - get api_user_path("by_username"), params: { url: "invalid-username" } + get api_user_path("by_username"), params: { url: "invalid-username" }, headers: headers expect(response).to have_http_status(:not_found) end it "returns 404 if the user is not registered" do user.update_column(:registered, false) - get api_user_path(user.id) + get api_user_path(user.id), headers: headers expect(response).to have_http_status(:not_found) end it "returns 200 if the user username is found" do - get api_user_path("by_username"), params: { url: user.username } + get api_user_path("by_username"), params: { url: user.username }, headers: headers expect(response).to have_http_status(:ok) end it "returns unauthenticated if no authentication and the Forem instance is set to private" do allow(Settings::UserExperience).to receive(:public).and_return(false) - get api_user_path("by_username"), params: { url: user.username } + get api_user_path("by_username"), params: { url: user.username }, headers: headers expect(response).to have_http_status(:unauthorized) end it "returns the correct json representation of the user", :aggregate_failures do - get api_user_path(user.id) + get api_user_path(user.id), headers: headers response_user = response.parsed_body @@ -86,14 +69,14 @@ RSpec.describe "Api::V0::Users", type: :request do context "when unauthenticated" do it "returns unauthorized" do - get me_api_users_path, headers: { "Accept" => "application/vnd.forem.api-v1+json" } + get me_api_users_path, headers: headers expect(response).to have_http_status(:unauthorized) end end context "when unauthorized" do it "returns unauthorized" do - get me_api_users_path, headers: v1_headers.merge({ "api-key" => "invalid api key" }) + get me_api_users_path, headers: headers.merge({ "api-key" => "invalid api key" }) expect(response).to have_http_status(:unauthorized) end end @@ -102,7 +85,7 @@ RSpec.describe "Api::V0::Users", type: :request do let(:user) { api_secret.user } it "returns the correct json representation of the user", :aggregate_failures do - get me_api_users_path, headers: v1_headers + get me_api_users_path, headers: auth_headers expect(response).to have_http_status(:ok) @@ -124,7 +107,7 @@ RSpec.describe "Api::V0::Users", type: :request do it "returns 200 if no authentication and the Forem instance is set to private but user is authenticated" do allow(Settings::UserExperience).to receive(:public).and_return(false) - get me_api_users_path, headers: v1_headers + get me_api_users_path, headers: auth_headers response_user = response.parsed_body @@ -157,7 +140,7 @@ RSpec.describe "Api::V0::Users", type: :request do it "returns unauthorized" do put api_user_suspend_path(id: target_user.id), params: payload, - headers: { "Accept" => "application/vnd.forem.api-v1+json" } + headers: headers expect(response).to have_http_status(:unauthorized) end @@ -167,7 +150,7 @@ RSpec.describe "Api::V0::Users", type: :request do it "returns unauthorized if api key is invalid" do put api_user_suspend_path(id: target_user.id), params: payload, - headers: v1_headers.merge({ "api-key" => "invalid api key" }) + headers: headers.merge({ "api-key" => "invalid api key" }) expect(response).to have_http_status(:unauthorized) end @@ -175,7 +158,7 @@ RSpec.describe "Api::V0::Users", type: :request do it "returns unauthorized if api key belongs to non-admin user" do put api_user_suspend_path(id: target_user.id), params: payload, - headers: v1_headers + headers: headers expect(response).to have_http_status(:unauthorized) end @@ -188,7 +171,7 @@ RSpec.describe "Api::V0::Users", type: :request do expect do put api_user_suspend_path(id: target_user.id), params: payload, - headers: v1_headers + headers: auth_headers expect(response).to have_http_status(:no_content) expect(target_user.reload.suspended?).to be true @@ -199,7 +182,7 @@ RSpec.describe "Api::V0::Users", type: :request do it "creates an audit log of the action taken" do put api_user_suspend_path(id: target_user.id), params: payload, - headers: v1_headers + headers: auth_headers log = AuditLog.last expect(log.category).to eq(AuditLog::ADMIN_API_AUDIT_LOG_CATEGORY) @@ -223,7 +206,7 @@ RSpec.describe "Api::V0::Users", type: :request do context "when unauthenticated" do it "returns unauthorized" do put api_user_unpublish_path(id: target_user.id), - headers: { "Accept" => "application/vnd.forem.api-v1+json" } + headers: headers expect(response).to have_http_status(:unauthorized) end @@ -232,14 +215,14 @@ RSpec.describe "Api::V0::Users", type: :request do context "when unauthorized" do it "returns unauthorized if api key is invalid" do put api_user_unpublish_path(id: target_user.id), - headers: v1_headers.merge({ "api-key" => "invalid api key" }) + headers: headers.merge({ "api-key" => "invalid api key" }) expect(response).to have_http_status(:unauthorized) end it "returns unauthorized if api key belongs to non-admin user" do put api_user_unpublish_path(id: target_user.id), - headers: v1_headers + headers: headers expect(response).to have_http_status(:unauthorized) end @@ -254,7 +237,7 @@ RSpec.describe "Api::V0::Users", type: :request do expect(target_comments.map(&:deleted)).to match_array([false, false, false]) put api_user_unpublish_path(id: target_user.id), - headers: v1_headers + headers: auth_headers expect(response).to have_http_status(:no_content) sidekiq_perform_enqueued_jobs @@ -274,7 +257,7 @@ RSpec.describe "Api::V0::Users", type: :request do create(:comment, user: target_user, deleted: true) put api_user_unpublish_path(id: target_user.id), - headers: v1_headers + headers: auth_headers log = AuditLog.last expect(log.category).to eq(AuditLog::ADMIN_API_AUDIT_LOG_CATEGORY) diff --git a/spec/requests/api/v1/videos_spec.rb b/spec/requests/api/v1/videos_spec.rb index 32217cd66..41f81c775 100644 --- a/spec/requests/api/v1/videos_spec.rb +++ b/spec/requests/api/v1/videos_spec.rb @@ -1,9 +1,9 @@ require "rails_helper" -RSpec.describe "Api::V0::Videos", type: :request do +RSpec.describe "Api::V1::Videos", type: :request do let(:user) { create(:user, created_at: 1.month.ago) } let(:api_secret) { create(:api_secret, user: user) } - let(:v1_headers) { { "api-key" => api_secret.secret, "Accept" => "application/vnd.forem.api-v1+json" } } + let(:headers) { { "Accept" => "application/vnd.forem.api-v1+json" } } def create_article(article_params = {}) default_params = { @@ -16,30 +16,10 @@ RSpec.describe "Api::V0::Videos", type: :request do describe "GET /api/videos" do before { allow(FeatureFlag).to receive(:enabled?).with(:api_v1).and_return(true) } - context "when unauthenticated" do - it "returns unauthorized" do - create_article - - get api_videos_path, headers: { "Accept" => "application/vnd.forem.api-v1+json" } - - expect(response).to have_http_status(:unauthorized) - end - end - - context "when unauthorized" do - it "returns unauthorized" do - create_article - - get api_videos_path, headers: v1_headers.merge({ "api-key" => "invalid api key" }) - - expect(response).to have_http_status(:unauthorized) - end - end - it "returns articles with videos" do create_article - get api_videos_path, headers: v1_headers + get api_videos_path, headers: headers expect(response.parsed_body.size).to eq(1) end @@ -48,7 +28,7 @@ RSpec.describe "Api::V0::Videos", type: :request do article = create_article article.update(published: false) - get api_videos_path, headers: v1_headers + get api_videos_path, headers: headers expect(response.parsed_body.size).to eq(1) end @@ -56,7 +36,7 @@ RSpec.describe "Api::V0::Videos", type: :request do it "does not return regular articles without videos" do create(:article) - get api_videos_path, headers: v1_headers + get api_videos_path, headers: headers expect(response.parsed_body.size).to eq(0) end @@ -64,7 +44,7 @@ RSpec.describe "Api::V0::Videos", type: :request do it "does not return video articles with a score that is too low" do create_article(score: -4) - get api_videos_path, headers: v1_headers + get api_videos_path, headers: headers expect(response.parsed_body.size).to eq(0) end @@ -72,7 +52,7 @@ RSpec.describe "Api::V0::Videos", type: :request do it "returns video articles with the correct json representation", :aggregate_failures do video_article = create_article - get api_videos_path, headers: v1_headers + get api_videos_path, headers: headers response_video = response.parsed_body.first expected_keys = %w[type_of id path cloudinary_video_url title user_id video_duration_in_minutes video_source_url @@ -90,7 +70,7 @@ RSpec.describe "Api::V0::Videos", type: :request do video_article = create_article(hotness_score: 10) other_video_article = create_article(hotness_score: 9) - get api_videos_path, headers: v1_headers + get api_videos_path, headers: headers expected_result = [video_article.id, other_video_article.id] expect(response.parsed_body.map { |a| a["id"] }).to eq(expected_result) @@ -102,17 +82,17 @@ RSpec.describe "Api::V0::Videos", type: :request do user: user, video: "https://example.com", video_thumbnail_url: "https://example.com", title: "video" ) - get api_videos_path, params: { page: 1, per_page: 2 }, headers: v1_headers + get api_videos_path, params: { page: 1, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(2) - get api_videos_path, params: { page: 2, per_page: 2 }, headers: v1_headers + get api_videos_path, params: { page: 2, per_page: 2 }, headers: headers expect(response.parsed_body.length).to eq(1) end it "sets the correct edge caching surrogate key for all video articles" do video_article = create_article - get api_videos_path, headers: v1_headers + get api_videos_path, headers: headers expected_key = ["videos", "articles", video_article.record_key].to_set expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key) diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb index 97d423f32..689ce36cc 100644 --- a/spec/swagger_helper.rb +++ b/spec/swagger_helper.rb @@ -20,7 +20,8 @@ RSpec.configure do |config| version: "1.0.0", description: "Access Forem articles, users and other resources via API. For a real-world example of Forem in action, check out [DEV](https://www.dev.to). - All endpoints require an 'api-key' header and a accept header. + All endpoints can be accessed with the 'api-key' header and a accept header, but + some of them are accessible publicly without authentication. Dates and date times, unless otherwise specified, must be in the [RFC 3339](https://tools.ietf.org/html/rfc3339) format." diff --git a/swagger/v1/api_v1.json b/swagger/v1/api_v1.json index 7c8074339..25b059557 100644 --- a/swagger/v1/api_v1.json +++ b/swagger/v1/api_v1.json @@ -3,12 +3,13 @@ "info": { "title": "Forem API V1", "version": "1.0.0", - "description": "Access Forem articles, users and other resources via API.\n For a real-world example of Forem in action, check out [DEV](https://www.dev.to).\n All endpoints require an 'api-key' header and a accept header.\n\n Dates and date times, unless otherwise specified, must be in\n the [RFC 3339](https://tools.ietf.org/html/rfc3339) format." + "description": "Access Forem articles, users and other resources via API.\n For a real-world example of Forem in action, check out [DEV](https://www.dev.to).\n All endpoints can be accessed with the 'api-key' header and a accept header, but \n some of them are accessible publicly without authentication.\n\n Dates and date times, unless otherwise specified, must be in\n the [RFC 3339](https://tools.ietf.org/html/rfc3339) format." }, "paths": { "/api/articles": { "get": { "summary": "Published articles", + "security": [], "tags": ["articles"], "description": "This endpoint allows the client to retrieve a list of articles.\n\n\"Articles\" are all the posts that users create on DEV that typically\nshow up in the feed. They can be a blog post, a discussion question,\na help thread etc. but is referred to as article within the code.\n\nBy default it will return featured, published articles ordered\nby descending popularity.\n\nIt supports pagination, each page will contain `30` articles by default.", "operationId": "getArticles", @@ -102,44 +103,45 @@ "example": [ { "type_of": "article", - "id": 2661, - "title": "To a God Unknown172", - "description": "Thundercats kitsch park bespoke scenester cray irony occupy. Crucifix salvia vegan. Disrupt offal 3...", - "readable_publish_date": "Jul 27", - "slug": "to-a-god-unknown172-3lh1", - "path": "/username469/to-a-god-unknown172-3lh1", - "url": "http://localhost:3000/username469/to-a-god-unknown172-3lh1", + "id": 1357, + "title": "Ego Dominus Tuus168", + "description": "Blog pour-over taxidermy pbr&b bitters. Synth chia pork belly butcher. Mumblecore 8-bit banh mi...", + "readable_publish_date": "Aug 20", + "slug": "ego-dominus-tuus168-32d5", + "path": "/username372/ego-dominus-tuus168-32d5", + "url": "http://localhost:3000/username372/ego-dominus-tuus168-32d5", "comments_count": 0, "public_reactions_count": 0, "collection_id": null, - "published_timestamp": "2022-07-27T20:31:54Z", + "published_timestamp": "2022-08-19T16:09:09Z", "positive_reactions_count": 0, - "cover_image": "http://localhost:3000/assets/19-ed58d3e8defcefc445020631589697a05e725243e834b5192aee4e6b91a3e927.png", - "social_image": "http://localhost:3000/assets/19-ed58d3e8defcefc445020631589697a05e725243e834b5192aee4e6b91a3e927.png", - "canonical_url": "http://localhost:3000/username469/to-a-god-unknown172-3lh1", - "created_at": "2022-07-27T20:31:54Z", + "cover_image": "http://localhost:3000/assets/2-1a96ae446ded018b65b215cce3aecc40a00e701642da521fdd6edd3c593ff6c1.png", + "social_image": "http://localhost:3000/assets/2-1a96ae446ded018b65b215cce3aecc40a00e701642da521fdd6edd3c593ff6c1.png", + "canonical_url": "http://localhost:3000/username372/ego-dominus-tuus168-32d5", + "created_at": "2022-08-19T16:09:09Z", "edited_at": null, "crossposted_at": null, - "published_at": "2022-07-27T20:31:54Z", - "last_comment_at": "2022-07-27T20:31:54Z", + "published_at": "2022-08-19T16:09:09Z", + "last_comment_at": "2022-08-19T16:09:09Z", "reading_time_minutes": 1, "tag_list": ["discuss"], "tags": "discuss", "user": { - "name": "Carey \"Maya\" \\:/ Corwin", - "username": "username469", - "twitter_username": "twitter469", - "github_username": "github469", + "name": "Kyla \"Avery\" \\:/ Morissette", + "username": "username372", + "twitter_username": "twitter372", + "github_username": "github372", + "user_id": 3080, "website_url": null, - "profile_image": "/uploads/user/profile_image/6332/5475485a-eb89-4e7f-b2d2-c548389462e8.jpeg", - "profile_image_90": "/uploads/user/profile_image/6332/5475485a-eb89-4e7f-b2d2-c548389462e8.jpeg" + "profile_image": "/uploads/user/profile_image/3080/001a3d7d-c716-43aa-a46a-8c024888daf9.jpeg", + "profile_image_90": "/uploads/user/profile_image/3080/001a3d7d-c716-43aa-a46a-8c024888daf9.jpeg" }, "organization": { - "name": "Blanda, Barton and Tremblay", + "name": "Koss and Sons", "username": "org56", "slug": "org56", - "profile_image": "/uploads/organization/profile_image/934/ba061551-7cbe-4adc-80d0-bc60abcdaa32.png", - "profile_image_90": "/uploads/organization/profile_image/934/ba061551-7cbe-4adc-80d0-bc60abcdaa32.png" + "profile_image": "/uploads/organization/profile_image/507/464b6b16-79e1-4378-a9fc-76c81403f9e1.png", + "profile_image_90": "/uploads/organization/profile_image/507/464b6b16-79e1-4378-a9fc-76c81403f9e1.png" }, "flare_tag": { "name": "discuss", @@ -156,9 +158,6 @@ } } } - }, - "401": { - "description": "unauthorized" } } }