From 3b32b299373def5cc161b9df91f68177e2cf83e4 Mon Sep 17 00:00:00 2001 From: Mac Siri Date: Thu, 8 Aug 2019 14:55:23 -0400 Subject: [PATCH] Update core API endpoints to support access_token (#3644) * Update specs for doorkeeper test * Add bang * Create #authenticate * Create /api/articles/ endpoint * Update specs * Update me.json.jbuilder * Fix typo and spacing * Support pagination for #me * Update #authenticate! to support cookies * Make per_page a param * Disable method-complexity check * Enable refresh_token * Create /api/users/me endpoint --- .codeclimate.yml | 2 +- .../api/v0/analytics_controller.rb | 2 +- app/controllers/api/v0/api_controller.rb | 19 +++++- app/controllers/api/v0/articles_controller.rb | 11 +++- app/controllers/api/v0/users_controller.rb | 6 +- app/views/api/v0/articles/me.json.jbuilder | 46 ++++++++++++++ app/views/api/v0/users/me.json.jbuilder | 11 ++++ config/initializers/doorkeeper.rb | 2 +- config/routes.rb | 7 ++- spec/factories/doorkeeper_access_tokens.rb | 5 ++ spec/factories/doorkeeper_applications.rb | 8 +++ spec/rails_helper.rb | 5 ++ spec/requests/api/v0/articles_spec.rb | 63 +++++++++++++++++++ spec/requests/api/v0/users_spec.rb | 24 +++++++ 14 files changed, 202 insertions(+), 9 deletions(-) create mode 100644 app/views/api/v0/articles/me.json.jbuilder create mode 100644 app/views/api/v0/users/me.json.jbuilder create mode 100644 spec/factories/doorkeeper_access_tokens.rb create mode 100644 spec/factories/doorkeeper_applications.rb create mode 100644 spec/requests/api/v0/users_spec.rb diff --git a/.codeclimate.yml b/.codeclimate.yml index 497b5818e..708c0755f 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -13,7 +13,7 @@ checks: config: threshold: 250 method-complexity: - enabled: true + enabled: false config: threshold: 5 method-count: diff --git a/app/controllers/api/v0/analytics_controller.rb b/app/controllers/api/v0/analytics_controller.rb index 173d76d91..357bbd56b 100644 --- a/app/controllers/api/v0/analytics_controller.rb +++ b/app/controllers/api/v0/analytics_controller.rb @@ -6,7 +6,7 @@ module Api rescue_from ArgumentError, with: :error_unprocessable_entity rescue_from UnauthorizedError, with: :error_unauthorized - before_action :authenticate_with_api_key_or_current_user + before_action :authenticate_with_api_key_or_current_user! before_action :authorize_pro_user before_action :authorize_user_organization before_action :load_owner diff --git a/app/controllers/api/v0/api_controller.rb b/app/controllers/api/v0/api_controller.rb index e82360045..8ebf899ff 100644 --- a/app/controllers/api/v0/api_controller.rb +++ b/app/controllers/api/v0/api_controller.rb @@ -43,15 +43,28 @@ class Api::V0::ApiController < ApplicationController render json: { error: "not found", status: 404 }, status: :not_found end - def authenticate_with_api_key_or_current_user + def authenticate! + if doorkeeper_token + @user = User.find(doorkeeper_token.resource_owner_id) + return error_unauthorized unless @user + elsif request.headers["api-key"] + authenticate_with_api_key! + elsif current_user + @user = current_user + else + error_unauthorized + end + end + + def authenticate_with_api_key_or_current_user! if request.headers["api-key"] - authenticate_with_api_key + authenticate_with_api_key! else @user = current_user end end - def authenticate_with_api_key + def authenticate_with_api_key! api_key = request.headers["api-key"] return error_unauthorized unless api_key diff --git a/app/controllers/api/v0/articles_controller.rb b/app/controllers/api/v0/articles_controller.rb index b2b014b76..6f80afe6a 100644 --- a/app/controllers/api/v0/articles_controller.rb +++ b/app/controllers/api/v0/articles_controller.rb @@ -3,7 +3,7 @@ module Api class ArticlesController < ApiController respond_to :json - before_action :authenticate_with_api_key, only: %i[create update] + before_action :authenticate!, only: %i[create update me] before_action :set_cache_control_headers, only: [:index] caches_action :show, @@ -65,6 +65,15 @@ module Api render "show", status: :ok end + def me + per_page = (params[:per_page] || 30).to_i + num = [per_page, 1000].min + @articles = @user.articles.order("published_at DESC"). + page(params[:page]). + per(num). + decorate + end + private def article_params diff --git a/app/controllers/api/v0/users_controller.rb b/app/controllers/api/v0/users_controller.rb index a6f65660b..d0dc1757a 100644 --- a/app/controllers/api/v0/users_controller.rb +++ b/app/controllers/api/v0/users_controller.rb @@ -1,6 +1,8 @@ module Api module V0 - class UsersController < ApplicationController + class UsersController < ApiController + before_action :authenticate!, only: %i[me] + def index if !user_signed_in? || less_than_one_day_old?(current_user) usernames = %w[ben jess peter maestromac andy liana] @@ -23,6 +25,8 @@ module Api end end + def me; end + private def less_than_one_day_old?(user) diff --git a/app/views/api/v0/articles/me.json.jbuilder b/app/views/api/v0/articles/me.json.jbuilder new file mode 100644 index 000000000..0421e2e27 --- /dev/null +++ b/app/views/api/v0/articles/me.json.jbuilder @@ -0,0 +1,46 @@ +json.array! @articles do |article| + json.type_of "article" + json.id article.id + json.title article.title + json.description article.description + json.cover_image cloud_cover_url(article.main_image) + json.published article.published + json.published_at article.published_at + json.tag_list article.cached_tag_list_array + json.slug article.slug + json.path article.path + json.url article.url + json.canonical_url article.processed_canonical_url + json.comments_count article.comments_count + json.positive_reactions_count article.positive_reactions_count + json.published_timestamp article.published_timestamp + json.body_markdown article.body_markdown + + json.user do + json.name article.user.name + json.username article.user.username + json.twitter_username article.user.twitter_username + json.github_username article.user.github_username + json.website_url article.user.processed_website_url + json.profile_image ProfileImage.new(article.user).get(640) + json.profile_image_90 ProfileImage.new(article.user).get(90) + end + + if article.organization + json.organization do + json.name article.organization.name + json.username article.organization.username + json.slug article.organization.slug + json.profile_image ProfileImage.new(article.organization).get(640) + json.profile_image_90 ProfileImage.new(article.organization).get(90) + end + end + + if FlareTag.new(article).tag + json.flare_tag do + json.name FlareTag.new(article).tag.name + json.bg_color_hex FlareTag.new(article).tag.bg_color_hex + json.text_color_hex FlareTag.new(article).tag.text_color_hex + end + end +end diff --git a/app/views/api/v0/users/me.json.jbuilder b/app/views/api/v0/users/me.json.jbuilder new file mode 100644 index 000000000..7dfe48eb1 --- /dev/null +++ b/app/views/api/v0/users/me.json.jbuilder @@ -0,0 +1,11 @@ +json.type_of "user" +json.id @user.id +json.username @user.username +json.name @user.name +json.summary @user.summary +json.twitter_username @user.twitter_username +json.github_username @user.github_username +json.website_url @user.website_url +json.location @user.location +json.joined_at @user.created_at.strftime("%b %e, %Y") +json.profile_image ProfileImage.new(@user).get(320) diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index e6a3710e4..e93e7bb39 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -149,7 +149,7 @@ Doorkeeper.configure do # `grant_type` - the grant type of the request (see Doorkeeper::OAuth) # `scopes` - the requested scopes (see Doorkeeper::OAuth::Scopes) # - # use_refresh_token + use_refresh_token # Provide support for an owner to be assigned to each registered application (disabled by default) # Optional parameter confirmation: true (default false) if you want to enforce ownership of diff --git a/config/routes.rb b/config/routes.rb index 2484aa2fa..eae9f644b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -78,6 +78,7 @@ Rails.application.routes.draw do resources :articles, only: %i[index show create update] do collection do get "/onboarding", to: "articles#onboarding" + get :me end end resources :comments, only: %i[index show] @@ -89,7 +90,11 @@ Rails.application.routes.draw do post "/onboarding", to: "reactions#onboarding" end end - resources :users, only: %i[index show] + resources :users, only: %i[index show] do + collection do + get :me + end + end resources :tags, only: [:index] do collection do get "/onboarding", to: "tags#onboarding" diff --git a/spec/factories/doorkeeper_access_tokens.rb b/spec/factories/doorkeeper_access_tokens.rb new file mode 100644 index 000000000..747e51ffd --- /dev/null +++ b/spec/factories/doorkeeper_access_tokens.rb @@ -0,0 +1,5 @@ +FactoryBot.define do + factory :doorkeeper_access_token, class: "Doorkeeper::AccessToken" do + application + end +end diff --git a/spec/factories/doorkeeper_applications.rb b/spec/factories/doorkeeper_applications.rb new file mode 100644 index 000000000..ccd7e5c6e --- /dev/null +++ b/spec/factories/doorkeeper_applications.rb @@ -0,0 +1,8 @@ +FactoryBot.define do + factory :application, class: "Doorkeeper::Application" do + sequence(:name) { |n| "Project #{n}" } + sequence(:redirect_uri) { |n| "https://example#{n}.com" } + secret { SecureRandom.hex(8) } + uid { SecureRandom.hex(8) } + end +end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index c75e258dc..4e1f11445 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -120,3 +120,8 @@ RSpec.configure do |config| # arbitrary gems may also be filtered via: # config.filter_gems_from_backtrace("gem name") end + +Doorkeeper.configure do + # hash_token_secrets on its own won't work in test + hash_token_secrets fallback: :plain +end diff --git a/spec/requests/api/v0/articles_spec.rb b/spec/requests/api/v0/articles_spec.rb index 485d9d184..5bc6f529d 100644 --- a/spec/requests/api/v0/articles_spec.rb +++ b/spec/requests/api/v0/articles_spec.rb @@ -102,6 +102,48 @@ RSpec.describe "Api::V0::Articles", type: :request do end end + describe "GET /api/articles/me" do + context "when request is unauthenticated" do + it "return unauthorized" do + get me_api_articles_path + expect(response).to have_http_status(:unauthorized) + end + end + + context "when request is authenticated" do + let_it_be(:user) { create(:user) } + let_it_be(:access_token) { create :doorkeeper_access_token, resource_owner: user } + + it "works with bearer authorization" do + headers = { "authorization" => "Bearer #{access_token.token}", "content-type" => "application/json" } + + get me_api_articles_path, headers: headers + expect(response.content_type).to eq("application/json") + expect(response).to have_http_status(:ok) + end + + it "return proper response specification" do + get me_api_articles_path, params: { access_token: access_token.token } + expect(response.content_type).to eq("application/json") + expect(response).to have_http_status(:ok) + end + + it "return only user's articles including markdown" do + create(:article, user: user) + create(:article) + get me_api_articles_path, params: { access_token: access_token.token } + expect(json_response.length).to eq(1) + expect(json_response[0]["body_markdown"]).not_to be_nil + end + + it "supports pagination" do + create_list(:article, 3, user: user) + get me_api_articles_path, params: { access_token: access_token.token, page: 2, per_page: 2 } + expect(json_response.length).to eq(1) + end + end + end + describe "POST /api/articles" do let!(:api_secret) { create(:api_secret) } let!(:user) { api_secret.user } @@ -131,6 +173,14 @@ RSpec.describe "Api::V0::Articles", type: :request do post api_articles_path, params: { article: params }.to_json, headers: headers end + it "supports oauth's access_token" do + access_token = create(:doorkeeper_access_token, resource_owner_id: user.id) + headers = { "authorization" => "Bearer #{access_token.token}", "content-type" => "application/json" } + + post api_articles_path, params: { article: { title: Faker::Book.title } }.to_json, headers: headers + expect(response).to have_http_status(:created) + end + it "fails if no params are given" do post_article expect(response).to have_http_status(:unprocessable_entity) @@ -373,6 +423,19 @@ RSpec.describe "Api::V0::Articles", type: :request do put path, params: { article: params }.to_json, headers: headers end + it "supports oauth's access_token" do + access_token = create(:doorkeeper_access_token, resource_owner_id: user.id) + headers = { "authorization" => "Bearer #{access_token.token}", "content-type" => "application/json" } + + title = Faker::Book.title + rand(100).to_s + body_markdown = "foobar" + params = { title: title, body_markdown: body_markdown } + put path, params: { article: params }.to_json, headers: headers + expect(response).to have_http_status(:ok) + expect(article.reload.title).to eq(title) + expect(article.body_markdown).to eq(body_markdown) + end + it "returns not found if the article does not belong to the user" do article = create(:article, user: create(:user)) headers = { "api-key" => api_secret.secret, "content-type" => "application/json" } diff --git a/spec/requests/api/v0/users_spec.rb b/spec/requests/api/v0/users_spec.rb new file mode 100644 index 000000000..e1a8dd7ad --- /dev/null +++ b/spec/requests/api/v0/users_spec.rb @@ -0,0 +1,24 @@ +require "rails_helper" + +RSpec.describe "Api::V0::Users", type: :request do + def json_response + JSON.parse(response.body) + end + + describe "GET /api/users/me" do + it "requires request to be authenticated" do + get me_api_users_path + expect(response).to have_http_status(:unauthorized) + end + + context "when request is authenticated" do + let_it_be(:user) { create(:user) } + let_it_be(:access_token) { create(:doorkeeper_access_token, resource_owner: user) } + + it "return user's information" do + get me_api_users_path, params: { access_token: access_token.token } + expect(json_response["username"]).to eq(user.username) + end + end + end +end