From 8ecf724247903ce89f3c5e747571e697dbd6ffff Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Tue, 11 Jul 2023 14:52:43 -0400 Subject: [PATCH 01/10] implement api structure; add auth routes --- Gemfile | 2 ++ Gemfile.lock | 17 ++++++++---- app/controllers/api/v1/base_controller.rb | 18 +++++++++++++ .../api/v1/users/sessions_controller.rb | 27 +++++++++++++++++++ app/models/casa_admin.rb | 1 + app/models/supervisor.rb | 1 + app/models/user.rb | 13 +++++++++ app/models/volunteer.rb | 1 + app/serializers/api/v1/session_serializer.rb | 4 +++ config/environments/development.rb | 2 ++ config/initializers/cors.rb | 6 +++++ config/initializers/rack_attack.rb | 4 +++ config/routes.rb | 9 +++++++ .../20230710025852_add_token_to_users.rb | 11 ++++++++ db/schema.rb | 9 ++++++- spec/api/authentication_spec.rb | 26 ++++++++++++++++++ spec/support/api_helper.rb | 11 ++++++++ 17 files changed, 156 insertions(+), 6 deletions(-) create mode 100644 app/controllers/api/v1/base_controller.rb create mode 100644 app/controllers/api/v1/users/sessions_controller.rb create mode 100644 app/serializers/api/v1/session_serializer.rb create mode 100644 config/initializers/cors.rb create mode 100644 db/migrate/20230710025852_add_token_to_users.rb create mode 100644 spec/api/authentication_spec.rb create mode 100644 spec/support/api_helper.rb diff --git a/Gemfile b/Gemfile index fc8de22ade..6ea22ecfc3 100644 --- a/Gemfile +++ b/Gemfile @@ -38,12 +38,14 @@ gem "pretender" gem "puma", "6.3.0" # 6.2.2 fails to install on m1 # Use Puma as the app server gem "pundit" # for authorization management - based on user.role field gem "rack-attack" # for blocking & throttling abusive requests +gem "rack-cors" # for allowing cross-origin resource sharing gem "request_store" gem "sablon" # Word document templating tool for Case Court Reports gem "scout_apm" gem "sprockets-rails" # The original asset pipeline for Rails [https://github.com/rails/sprockets-rails] gem "strong_migrations" gem "tzinfo-data", platforms: %i[mingw mswin x64_mingw jruby] # Windows does not include zoneinfo files, so bundle the tzinfo-data gem +gem "active_model_serializers" # for JSON serialization group :development, :test do gem "bullet" # Detect and fix N+1 queries diff --git a/Gemfile.lock b/Gemfile.lock index f60a1572a7..dca95df1f1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -46,6 +46,11 @@ GEM erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.1, >= 1.2.0) + active_model_serializers (0.10.13) + actionpack (>= 4.1, < 7.1) + activemodel (>= 4.1, < 7.1) + case_transform (>= 0.2) + jsonapi-renderer (>= 0.1.1.beta1, < 0.3) activejob (7.0.6) activesupport (= 7.0.6) globalid (>= 0.3.6) @@ -115,6 +120,8 @@ GEM capybara-screenshot (1.0.26) capybara (>= 1.0, < 4) launchy + case_transform (0.2) + activesupport caxlsx (3.4.1) htmlentities (~> 4.3, >= 4.3.4) marcel (~> 1.0) @@ -238,6 +245,7 @@ GEM activesupport (>= 5.0.0) jsbundling-rails (1.1.2) railties (>= 6.0.0) + jsonapi-renderer (0.2.2) jwt (2.7.1) launchy (2.5.0) addressable (~> 2.7) @@ -264,7 +272,6 @@ GEM method_source (1.0.0) mini_magick (4.11.0) mini_mime (1.1.2) - mini_portile2 (2.8.2) minitest (5.18.1) multi_xml (0.6.0) multipart-post (2.3.0) @@ -280,9 +287,6 @@ GEM net-smtp (0.3.3) net-protocol nio4r (2.5.9) - nokogiri (1.15.2) - mini_portile2 (~> 2.8.2) - racc (~> 1.4) nokogiri (1.15.2-arm64-darwin) racc (~> 1.4) nokogiri (1.15.2-x86_64-darwin) @@ -319,6 +323,8 @@ GEM rack (2.2.7) rack-attack (6.6.1) rack (>= 1.0, < 3) + rack-cors (2.0.1) + rack (>= 2.0.0) rack-test (2.1.0) rack (>= 1.3) rails (7.0.6) @@ -478,7 +484,6 @@ PLATFORMS arm64-darwin-20 arm64-darwin-21 arm64-darwin-22 - ruby x86_64-darwin-18 x86_64-darwin-19 x86_64-darwin-20 @@ -487,6 +492,7 @@ PLATFORMS x86_64-linux DEPENDENCIES + active_model_serializers after_party amazing_print annotate @@ -530,6 +536,7 @@ DEPENDENCIES puma (= 6.3.0) pundit rack-attack + rack-cors rails (~> 7.0.5) rails-controller-testing rake diff --git a/app/controllers/api/v1/base_controller.rb b/app/controllers/api/v1/base_controller.rb new file mode 100644 index 0000000000..cab12c9e68 --- /dev/null +++ b/app/controllers/api/v1/base_controller.rb @@ -0,0 +1,18 @@ +class Api::V1::BaseController < ActionController::API +rescue_from ActiveRecord::RecordNotFound, with: :not_found + + def authenticate_user! + token, options = ActionController::HttpAuthentication::Token.token_and_options(request) + return nil unless token && options.is_a?(hash) + user = User.find_by(email: options[:email]) + if user && ActiveSupport::SecurityUtils.secure_compare(user.token, token) + @current_user = user + else + return UnauthenticatedError + end + end + + def not_found + return api_error(status: 404, errors: 'Not found') + end +end diff --git a/app/controllers/api/v1/users/sessions_controller.rb b/app/controllers/api/v1/users/sessions_controller.rb new file mode 100644 index 0000000000..1858e2964c --- /dev/null +++ b/app/controllers/api/v1/users/sessions_controller.rb @@ -0,0 +1,27 @@ +class Api::V1::Users::SessionsController < Api::V1::BaseController + # POST /api/v1/users/sign_in + def create + load_resource + #p @user + if @user + render json: @user, serializer: Api::V1::SessionSerializer, status: 201 + else + render json: {message: "Wrong password or username"}, status: 401 + end + end + + private + def user_params + params.permit(:email, :password) + end + + def load_resource + #print user_params + #print user_params + @user = User.find_by(email: user_params[:email]) + if !(@user&.valid_password?(user_params[:password])) + @user = nil + end + #@user = User.find_by(email: user_params[:email])&.valid_password?(user_params[:password]) + end +end diff --git a/app/models/casa_admin.rb b/app/models/casa_admin.rb index cda73e3160..143ebae127 100644 --- a/app/models/casa_admin.rb +++ b/app/models/casa_admin.rb @@ -47,6 +47,7 @@ def change_to_supervisor! # reset_password_sent_at :datetime # reset_password_token :string # sign_in_count :integer default(0), not null +# token :string # type :string # unconfirmed_email :string # created_at :datetime not null diff --git a/app/models/supervisor.rb b/app/models/supervisor.rb index ad49691336..c9e2333efc 100644 --- a/app/models/supervisor.rb +++ b/app/models/supervisor.rb @@ -74,6 +74,7 @@ def recently_unassigned_volunteers # reset_password_sent_at :datetime # reset_password_token :string # sign_in_count :integer default(0), not null +# token :string # type :string # unconfirmed_email :string # created_at :datetime not null diff --git a/app/models/user.rb b/app/models/user.rb index bb47cb8ade..b05f03588b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -9,6 +9,7 @@ class User < ApplicationRecord before_update :record_previous_email after_create :skip_email_confirmation_upon_creation before_save :normalize_phone_number + before_validation :ensure_token validates_with UserValidator @@ -173,6 +174,17 @@ def after_confirmation send_email_changed_notification end + def ensure_token + self.token = generate_hex(:token) unless token.present? + end + + def generate_hex(column) + loop do + hex = SecureRandom.hex(32) + break hex unless self.class.where(column => hex).any? + end + end + private def normalize_phone_number @@ -212,6 +224,7 @@ def normalize_phone_number # reset_password_sent_at :datetime # reset_password_token :string # sign_in_count :integer default(0), not null +# token :string # type :string # unconfirmed_email :string # created_at :datetime not null diff --git a/app/models/volunteer.rb b/app/models/volunteer.rb index 0dbb3e7bb9..5558610ea9 100644 --- a/app/models/volunteer.rb +++ b/app/models/volunteer.rb @@ -167,6 +167,7 @@ def cases_where_contact_made_in_days(num_days = CONTACT_MADE_IN_DAYS_NUM) # reset_password_sent_at :datetime # reset_password_token :string # sign_in_count :integer default(0), not null +# token :string # type :string # unconfirmed_email :string # created_at :datetime not null diff --git a/app/serializers/api/v1/session_serializer.rb b/app/serializers/api/v1/session_serializer.rb new file mode 100644 index 0000000000..1807056fe2 --- /dev/null +++ b/app/serializers/api/v1/session_serializer.rb @@ -0,0 +1,4 @@ +class Api::V1::SessionSerializer < ActiveModel::Serializer + type "session" + attributes :id, :display_name, :email, :token +end diff --git a/config/environments/development.rb b/config/environments/development.rb index 7a63c3c4e3..85ee3c0d7c 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -78,5 +78,7 @@ # Uncomment if you wish to allow Action Cable access from any origin. # config.action_cable.disable_request_forgery_protection = true + config.hosts << ".ngrok-free.app" + config.assets.digest = false end diff --git a/config/initializers/cors.rb b/config/initializers/cors.rb new file mode 100644 index 0000000000..3f261372e0 --- /dev/null +++ b/config/initializers/cors.rb @@ -0,0 +1,6 @@ +Rails.application.config.middleware.insert_before 0, Rack::Cors do + allow do + origins "*" # make sure to change to domain name of frontend + resource "/api/v1/*", headers: :any, methods: [:get, :post, :patch, :put, :delete, :options, :head] + end +end diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index 5d84491bfc..b6fee09ced 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -50,6 +50,10 @@ class Rack::Attack end end + throttle("reg/ip", limit: 5, period: 20.seconds) do |req| + req.ip if req.path.starts_with?("/api/v1") + end + # Throttle POST requests to /xxxx/sign_in by email param # # Key: "rack::attack:#{Time.now.to_i/:period}:logins/email:#{req.email}" diff --git a/config/routes.rb b/config/routes.rb index 0a7d68cea8..32a60ca21b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -176,4 +176,13 @@ end get "/error", to: "error#index" + + namespace :api do + namespace :v1 do + namespace :users do + post 'sign_in', to: 'sessions#create' + #get 'sign_out', to: 'sessions#destroy' + end + end + end end diff --git a/db/migrate/20230710025852_add_token_to_users.rb b/db/migrate/20230710025852_add_token_to_users.rb new file mode 100644 index 0000000000..076f9e9704 --- /dev/null +++ b/db/migrate/20230710025852_add_token_to_users.rb @@ -0,0 +1,11 @@ +class AddTokenToUsers < ActiveRecord::Migration[7.0] + def up + add_column :users, :token, :string + User.find_each{|user| user.save!} + #change_column_null :users, :token, false, 1 + end + + def down + remove_column :users, :token, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 2af08ac6b7..e0285fa682 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_07_04_123327) do +ActiveRecord::Schema[7.0].define(version: 2023_07_10_025852) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -327,6 +327,12 @@ t.index ["casa_org_id"], name: "index_judges_on_casa_org_id" end + create_table "jwt_denylist", force: :cascade do |t| + t.string "jti", null: false + t.datetime "exp", null: false + t.index ["jti"], name: "index_jwt_denylist_on_jti" + end + create_table "languages", force: :cascade do |t| t.string "name" t.bigint "casa_org_id", null: false @@ -545,6 +551,7 @@ t.string "unconfirmed_email" t.string "old_emails", default: [], array: true t.boolean "receive_reimbursement_email", default: false + t.string "token" t.index ["casa_org_id"], name: "index_users_on_casa_org_id" t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true t.index ["email"], name: "index_users_on_email", unique: true diff --git a/spec/api/authentication_spec.rb b/spec/api/authentication_spec.rb new file mode 100644 index 0000000000..21ffe6f378 --- /dev/null +++ b/spec/api/authentication_spec.rb @@ -0,0 +1,26 @@ +require "rails_helper" +require "spec_helper" + +RSpec.describe Api::V1::Users::SessionsController , :type => :api do + let(:casa_org) { create(:casa_org) } + let(:volunteer) { create(:volunteer, casa_org: casa_org) } + + it "should handle correct sign in" do + post "/api/v1/users/sign_in", {email: volunteer.email, password: volunteer.password} + # print last_response.headers + #print last_response.body + #expect(last_response.headers).to have_key "Authorization" + #expect(last_response.headers["Authorization"]).to be_starts_with("Bearer") + expect(last_response.body).to eq Api::V1::SessionSerializer.new(volunteer).to_json + expect(last_response.status).to eq 201 + expect(last_response.content_type).to eq("application/json; charset=utf-8") + end + + it "should handle incorrect sign in" do + post "/api/v1/users/sign_in", {email: "suzume@tojimari.jp", password: ""} + body = JSON.parse(last_response.body, symbolize_names: true) + expect(body.dig(:message)).to eq "Wrong password or username" + expect(last_response.status).to eq 401 + expect(last_response.content_type).to eq("application/json; charset=utf-8") + end +end diff --git a/spec/support/api_helper.rb b/spec/support/api_helper.rb new file mode 100644 index 0000000000..d4a11cfcbb --- /dev/null +++ b/spec/support/api_helper.rb @@ -0,0 +1,11 @@ +module ApiHelper + include Rack::Test::Methods + + def app + Rails.application + end +end + +RSpec.configure do |config| + config.include ApiHelper, :type=>:api #apply to all spec for apis folder +end From fd4603e9fad54e0317635eaf09e885202aa863e7 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Tue, 11 Jul 2023 15:50:27 -0400 Subject: [PATCH 02/10] lint files --- app/controllers/api/v1/base_controller.rb | 6 +++--- app/controllers/api/v1/users/sessions_controller.rb | 11 ++++++----- config/routes.rb | 4 ++-- db/migrate/20230710025852_add_token_to_users.rb | 4 ++-- spec/api/authentication_spec.rb | 10 +++++----- spec/support/api_helper.rb | 2 +- 6 files changed, 19 insertions(+), 18 deletions(-) diff --git a/app/controllers/api/v1/base_controller.rb b/app/controllers/api/v1/base_controller.rb index cab12c9e68..370aea5e6a 100644 --- a/app/controllers/api/v1/base_controller.rb +++ b/app/controllers/api/v1/base_controller.rb @@ -1,5 +1,5 @@ class Api::V1::BaseController < ActionController::API -rescue_from ActiveRecord::RecordNotFound, with: :not_found + rescue_from ActiveRecord::RecordNotFound, with: :not_found def authenticate_user! token, options = ActionController::HttpAuthentication::Token.token_and_options(request) @@ -8,11 +8,11 @@ def authenticate_user! if user && ActiveSupport::SecurityUtils.secure_compare(user.token, token) @current_user = user else - return UnauthenticatedError + UnauthenticatedError end end def not_found - return api_error(status: 404, errors: 'Not found') + api_error(status: 404, errors: "Not found") end end diff --git a/app/controllers/api/v1/users/sessions_controller.rb b/app/controllers/api/v1/users/sessions_controller.rb index 1858e2964c..59f8233db7 100644 --- a/app/controllers/api/v1/users/sessions_controller.rb +++ b/app/controllers/api/v1/users/sessions_controller.rb @@ -2,7 +2,7 @@ class Api::V1::Users::SessionsController < Api::V1::BaseController # POST /api/v1/users/sign_in def create load_resource - #p @user + # p @user if @user render json: @user, serializer: Api::V1::SessionSerializer, status: 201 else @@ -11,17 +11,18 @@ def create end private + def user_params params.permit(:email, :password) end def load_resource - #print user_params - #print user_params + # print user_params + # print user_params @user = User.find_by(email: user_params[:email]) - if !(@user&.valid_password?(user_params[:password])) + if !@user&.valid_password?(user_params[:password]) @user = nil end - #@user = User.find_by(email: user_params[:email])&.valid_password?(user_params[:password]) + # @user = User.find_by(email: user_params[:email])&.valid_password?(user_params[:password]) end end diff --git a/config/routes.rb b/config/routes.rb index 32a60ca21b..f215b296fb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -180,8 +180,8 @@ namespace :api do namespace :v1 do namespace :users do - post 'sign_in', to: 'sessions#create' - #get 'sign_out', to: 'sessions#destroy' + post "sign_in", to: "sessions#create" + # get 'sign_out', to: 'sessions#destroy' end end end diff --git a/db/migrate/20230710025852_add_token_to_users.rb b/db/migrate/20230710025852_add_token_to_users.rb index 076f9e9704..9d06ae6a54 100644 --- a/db/migrate/20230710025852_add_token_to_users.rb +++ b/db/migrate/20230710025852_add_token_to_users.rb @@ -1,8 +1,8 @@ class AddTokenToUsers < ActiveRecord::Migration[7.0] def up add_column :users, :token, :string - User.find_each{|user| user.save!} - #change_column_null :users, :token, false, 1 + User.find_each { |user| user.save! } + # change_column_null :users, :token, false, 1 end def down diff --git a/spec/api/authentication_spec.rb b/spec/api/authentication_spec.rb index 21ffe6f378..58e9c039cc 100644 --- a/spec/api/authentication_spec.rb +++ b/spec/api/authentication_spec.rb @@ -1,16 +1,16 @@ require "rails_helper" require "spec_helper" -RSpec.describe Api::V1::Users::SessionsController , :type => :api do +RSpec.describe Api::V1::Users::SessionsController, type: :api do let(:casa_org) { create(:casa_org) } let(:volunteer) { create(:volunteer, casa_org: casa_org) } it "should handle correct sign in" do post "/api/v1/users/sign_in", {email: volunteer.email, password: volunteer.password} - # print last_response.headers - #print last_response.body - #expect(last_response.headers).to have_key "Authorization" - #expect(last_response.headers["Authorization"]).to be_starts_with("Bearer") + # print last_response.headers + # print last_response.body + # expect(last_response.headers).to have_key "Authorization" + # expect(last_response.headers["Authorization"]).to be_starts_with("Bearer") expect(last_response.body).to eq Api::V1::SessionSerializer.new(volunteer).to_json expect(last_response.status).to eq 201 expect(last_response.content_type).to eq("application/json; charset=utf-8") diff --git a/spec/support/api_helper.rb b/spec/support/api_helper.rb index d4a11cfcbb..018d8c250b 100644 --- a/spec/support/api_helper.rb +++ b/spec/support/api_helper.rb @@ -7,5 +7,5 @@ def app end RSpec.configure do |config| - config.include ApiHelper, :type=>:api #apply to all spec for apis folder + config.include ApiHelper, type: :api # apply to all spec for apis folder end From 602e4cf1015977e143408af0ed54732f18680502 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Mon, 7 Aug 2023 14:42:43 -0400 Subject: [PATCH 03/10] fix some checks like codeclimate and adding extra tests --- app/controllers/api/v1/base_controller.rb | 10 ++++-- app/models/concerns/generate_token.rb | 16 +++++++++ app/models/user.rb | 12 +------ spec/api/controllers/base_controller_spec.rb | 33 +++++++++++++++++++ .../serializers/session_serializer_spec.rb | 20 +++++++++++ ...on_spec.rb => sessions_controller_spec.rb} | 0 spec/factories/users.rb | 1 + spec/models/user_spec.rb | 9 +++++ 8 files changed, 87 insertions(+), 14 deletions(-) create mode 100644 app/models/concerns/generate_token.rb create mode 100644 spec/api/controllers/base_controller_spec.rb create mode 100644 spec/api/serializers/session_serializer_spec.rb rename spec/api/{authentication_spec.rb => sessions_controller_spec.rb} (100%) diff --git a/app/controllers/api/v1/base_controller.rb b/app/controllers/api/v1/base_controller.rb index 370aea5e6a..ea4e7ba94d 100644 --- a/app/controllers/api/v1/base_controller.rb +++ b/app/controllers/api/v1/base_controller.rb @@ -1,14 +1,18 @@ class Api::V1::BaseController < ActionController::API rescue_from ActiveRecord::RecordNotFound, with: :not_found + before_action :authenticate_user!, except: [:create] def authenticate_user! token, options = ActionController::HttpAuthentication::Token.token_and_options(request) - return nil unless token && options.is_a?(hash) + # return nil unless token && options.is_a?(Hash) user = User.find_by(email: options[:email]) - if user && ActiveSupport::SecurityUtils.secure_compare(user.token, token) + p "#######" + print token + p "#######" + if user && token && ActiveSupport::SecurityUtils.secure_compare(user.token, token) @current_user = user else - UnauthenticatedError + render json: {message: "Wrong password or email"}, status: 401 end end diff --git a/app/models/concerns/generate_token.rb b/app/models/concerns/generate_token.rb new file mode 100644 index 0000000000..b1e459e978 --- /dev/null +++ b/app/models/concerns/generate_token.rb @@ -0,0 +1,16 @@ +module GenerateToken + extend ActiveSupport::Concern + + included do + def ensure_token + self.token = generate_hex(:token) unless token.present? + end + + def generate_hex(column) + loop do + hex = SecureRandom.hex(32) + break hex unless self.class.where(column => hex).any? + end + end + end +end diff --git a/app/models/user.rb b/app/models/user.rb index b05f03588b..fc80076036 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -5,6 +5,7 @@ class User < ApplicationRecord include Roles include ByOrganizationScope include DateHelper + include GenerateToken before_update :record_previous_email after_create :skip_email_confirmation_upon_creation @@ -174,17 +175,6 @@ def after_confirmation send_email_changed_notification end - def ensure_token - self.token = generate_hex(:token) unless token.present? - end - - def generate_hex(column) - loop do - hex = SecureRandom.hex(32) - break hex unless self.class.where(column => hex).any? - end - end - private def normalize_phone_number diff --git a/spec/api/controllers/base_controller_spec.rb b/spec/api/controllers/base_controller_spec.rb new file mode 100644 index 0000000000..f65989cc20 --- /dev/null +++ b/spec/api/controllers/base_controller_spec.rb @@ -0,0 +1,33 @@ +require "rails_helper" + +RSpec.describe Api::V1::BaseController, type: :controller do + # assert token and options get parsed correctly + controller do + def index + render json: {message: "Successfully autenticated"} + end + + def authenticated_user! + super + end + end + + # assert authenticate_user! works + describe "GET #index" do + let(:user) { create(:volunteer) } + it "returns http success" do + request.headers["Authorization"] = "Token token=#{user.token}, email=#{user.email}" + # print request.headers["Authorization"] + get :index + expect(response).to have_http_status(:success) + expect(response.body).to eq({message: "Successfully autenticated"}.to_json) + end + # assert authenticate_user! works with invalid email + it "returns http unauthorized" do + request.headers["Authorization"] = "Token token=, email=#{user.email}" + get :index + expect(response).to have_http_status(:unauthorized) + expect(response.body).to eq({message: "Wrong password or email"}.to_json) + end + end +end diff --git a/spec/api/serializers/session_serializer_spec.rb b/spec/api/serializers/session_serializer_spec.rb new file mode 100644 index 0000000000..60ad79f3ea --- /dev/null +++ b/spec/api/serializers/session_serializer_spec.rb @@ -0,0 +1,20 @@ +require "rails_helper" + +RSpec.describe Api::V1::SessionSerializer, type: :serializer do + before(:each) do + @casa_org = create(:casa_org) + @volunteer = create(:volunteer, casa_org: @casa_org) + @serializer = Api::V1::SessionSerializer.new(@volunteer) + @serialization = ActiveModelSerializers::Adapter.create(@serializer) + end + + subject { JSON.parse(@serialization.to_json) } + + it "should have matching attributes" do + expect(subject["id"]).to eq(@volunteer.id) + expect(subject["email"]).to eq(@volunteer.email) + expect(subject["display_name"]).to eq(@volunteer.display_name) + expect(subject["token"]).to eq(@volunteer.token) + expect(subject.length).to eq(4) + end +end diff --git a/spec/api/authentication_spec.rb b/spec/api/sessions_controller_spec.rb similarity index 100% rename from spec/api/authentication_spec.rb rename to spec/api/sessions_controller_spec.rb diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 8c40fb8fc2..fb21549d15 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -8,6 +8,7 @@ case_assignments { [] } phone_number { "" } confirmed_at { Time.now } + token { "verysecuretoken" } trait :inactive do volunteer diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7679cf071a..92e9d7f7db 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -49,6 +49,15 @@ end end + describe "token generation" do + it "generates a token before validation" do + user = User.new + expect(user.token).to be_nil + user.valid? + expect(user.token).not_to be_nil + end + end + describe "#case_contacts_for" do let(:volunteer) { create(:volunteer, :with_casa_cases) } let(:case_of_interest) { volunteer.casa_cases.first } From 12be2daae93e70e33c11155dfed448049b285dfe Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Mon, 7 Aug 2023 15:25:40 -0400 Subject: [PATCH 04/10] change file paths of tests so spec checker is satisfied --- .allow_skipping_tests | 1 + .../controllers => controllers/api/v1}/base_controller_spec.rb | 0 .../api/v1/users}/sessions_controller_spec.rb | 0 .../api/v1}/session_serializer_spec.rb | 0 4 files changed, 1 insertion(+) rename spec/{api/controllers => controllers/api/v1}/base_controller_spec.rb (100%) rename spec/{api => controllers/api/v1/users}/sessions_controller_spec.rb (100%) rename spec/{api/serializers => serializers/api/v1}/session_serializer_spec.rb (100%) diff --git a/.allow_skipping_tests b/.allow_skipping_tests index 0d46f29d2b..fdaf9ab7c6 100644 --- a/.allow_skipping_tests +++ b/.allow_skipping_tests @@ -62,6 +62,7 @@ mailers/application_mailer.rb models/application_record.rb models/concerns/by_organization_scope.rb models/concerns/roles.rb +models/concerns/generate_token.rb models/fund_request.rb models/notification.rb notifications/base_notification.rb diff --git a/spec/api/controllers/base_controller_spec.rb b/spec/controllers/api/v1/base_controller_spec.rb similarity index 100% rename from spec/api/controllers/base_controller_spec.rb rename to spec/controllers/api/v1/base_controller_spec.rb diff --git a/spec/api/sessions_controller_spec.rb b/spec/controllers/api/v1/users/sessions_controller_spec.rb similarity index 100% rename from spec/api/sessions_controller_spec.rb rename to spec/controllers/api/v1/users/sessions_controller_spec.rb diff --git a/spec/api/serializers/session_serializer_spec.rb b/spec/serializers/api/v1/session_serializer_spec.rb similarity index 100% rename from spec/api/serializers/session_serializer_spec.rb rename to spec/serializers/api/v1/session_serializer_spec.rb From de18953151995e1a05996c047b9d30dea4743331 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Wed, 16 Aug 2023 11:05:31 -0400 Subject: [PATCH 05/10] add rswag gem for api documentation --- Gemfile | 3 + Gemfile.lock | 15 +++++ config/initializers/rswag_api.rb | 14 +++++ config/initializers/rswag_ui.rb | 16 ++++++ config/routes.rb | 2 + spec/requests/api/v1/users/sessions_spec.rb | 34 +++++++++++ spec/swagger_helper.rb | 62 +++++++++++++++++++++ swagger/v1/swagger.yaml | 61 ++++++++++++++++++++ 8 files changed, 207 insertions(+) create mode 100644 config/initializers/rswag_api.rb create mode 100644 config/initializers/rswag_ui.rb create mode 100644 spec/requests/api/v1/users/sessions_spec.rb create mode 100644 spec/swagger_helper.rb create mode 100644 swagger/v1/swagger.yaml diff --git a/Gemfile b/Gemfile index 9530ea3b8e..ea3514e823 100644 --- a/Gemfile +++ b/Gemfile @@ -47,6 +47,8 @@ gem "stimulus-rails" gem "strong_migrations" gem "tzinfo-data", platforms: %i[mingw mswin x64_mingw jruby] # Windows does not include zoneinfo files, so bundle the tzinfo-data gem gem "active_model_serializers" # for JSON serialization +gem "rswag-api" +gem "rswag-ui" group :development, :test do gem "bullet" # Detect and fix N+1 queries @@ -57,6 +59,7 @@ group :development, :test do gem "pry" gem "pry-byebug" gem "rspec-rails" + gem "rswag-specs" gem "shoulda-matchers" gem "standard", "1.5.0" # 1.6.0 errors on all factorybot create variables end diff --git a/Gemfile.lock b/Gemfile.lock index e609550d98..37f7326bf6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -251,6 +251,8 @@ GEM activesupport (>= 5.0.0) jsbundling-rails (1.1.2) railties (>= 6.0.0) + json-schema (3.0.0) + addressable (>= 2.8) jsonapi-renderer (0.2.2) jwt (2.7.1) launchy (2.5.0) @@ -391,6 +393,16 @@ GEM rspec-mocks (~> 3.12) rspec-support (~> 3.12) rspec-support (3.12.0) + rswag-api (2.10.1) + railties (>= 3.1, < 7.1) + rswag-specs (2.10.1) + activesupport (>= 3.1, < 7.1) + json-schema (>= 2.2, < 4.0) + railties (>= 3.1, < 7.1) + rspec-core (>= 2.14) + rswag-ui (2.10.1) + actionpack (>= 3.1, < 7.1) + railties (>= 3.1, < 7.1) rubocop (1.23.0) parallel (~> 1.10) parser (>= 3.0.0.0) @@ -549,6 +561,9 @@ DEPENDENCIES request_store rexml rspec-rails + rswag-api + rswag-specs + rswag-ui sablon scout_apm selenium-webdriver diff --git a/config/initializers/rswag_api.rb b/config/initializers/rswag_api.rb new file mode 100644 index 0000000000..4d72f68760 --- /dev/null +++ b/config/initializers/rswag_api.rb @@ -0,0 +1,14 @@ +Rswag::Api.configure do |c| + + # Specify a root folder where Swagger JSON files are located + # This is used by the Swagger middleware to serve requests for API descriptions + # NOTE: If you're using rswag-specs to generate Swagger, you'll need to ensure + # that it's configured to generate files in the same folder + c.swagger_root = Rails.root.to_s + '/swagger' + + # Inject a lambda function to alter the returned Swagger prior to serialization + # The function will have access to the rack env for the current request + # For example, you could leverage this to dynamically assign the "host" property + # + #c.swagger_filter = lambda { |swagger, env| swagger['host'] = env['HTTP_HOST'] } +end diff --git a/config/initializers/rswag_ui.rb b/config/initializers/rswag_ui.rb new file mode 100644 index 0000000000..0a768c17bc --- /dev/null +++ b/config/initializers/rswag_ui.rb @@ -0,0 +1,16 @@ +Rswag::Ui.configure do |c| + + # List the Swagger endpoints that you want to be documented through the + # swagger-ui. The first parameter is the path (absolute or relative to the UI + # host) to the corresponding endpoint and the second is a title that will be + # displayed in the document selector. + # NOTE: If you're using rspec-api to expose Swagger files + # (under swagger_root) as JSON or YAML endpoints, then the list below should + # correspond to the relative paths for those endpoints. + + c.swagger_endpoint '/api-docs/v1/swagger.yaml', 'API V1 Docs' + + # Add Basic Auth in case your API is private + # c.basic_auth_enabled = true + # c.basic_auth_credentials 'username', 'password' +end diff --git a/config/routes.rb b/config/routes.rb index 3da503e3b1..1343c5be1e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true Rails.application.routes.draw do + mount Rswag::Ui::Engine => '/api-docs' + mount Rswag::Api::Engine => '/api-docs' devise_for :all_casa_admins, path: "all_casa_admins", controllers: {sessions: "all_casa_admins/sessions"} devise_for :users, controllers: {sessions: "users/sessions", passwords: "users/passwords"} diff --git a/spec/requests/api/v1/users/sessions_spec.rb b/spec/requests/api/v1/users/sessions_spec.rb new file mode 100644 index 0000000000..d20f316de7 --- /dev/null +++ b/spec/requests/api/v1/users/sessions_spec.rb @@ -0,0 +1,34 @@ +require 'swagger_helper' + +RSpec.describe 'sessions API', type: :request do + path '/api/v1/users/sign_in' do + post 'Signs in a user' do + tags 'Sessions' + consumes 'application/json' + produces 'application/json' + parameter name: :user, in: :body, schema: { + type: :object, + properties: { + email: { type: :string }, + password: { type: :string } + }, + required: %w[email password] + } + + let(:casa_org) { create(:casa_org) } + let(:volunteer) { create(:volunteer, casa_org: casa_org) } + + response '201', 'user signed in' do + let(:user) { { email: volunteer.email, password: volunteer.password } } + schema '$ref' => '#/components/schemas/login_success' + run_test! + end + + response '401', 'invalid credentials' do + let(:user) { { email: 'foo', password: 'bar' } } + schema '$ref' => '#/components/schemas/login_failure' + run_test! + end + end + end +end diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb new file mode 100644 index 0000000000..84b419fdc8 --- /dev/null +++ b/spec/swagger_helper.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.configure do |config| + # Specify a root folder where Swagger JSON files are generated + # NOTE: If you're using the rswag-api to serve API descriptions, you'll need + # to ensure that it's configured to serve Swagger from the same folder + config.swagger_root = Rails.root.join('swagger').to_s + + # Define one or more Swagger documents and provide global metadata for each one + # When you run the 'rswag:specs:swaggerize' rake task, the complete Swagger will + # be generated at the provided relative path under swagger_root + # By default, the operations defined in spec files are added to the first + # document below. You can override this behavior by adding a swagger_doc tag to the + # the root example_group in your specs, e.g. describe '...', swagger_doc: 'v2/swagger.json' + config.swagger_docs = { + 'v1/swagger.yaml' => { + openapi: '3.0.1', + info: { + title: 'API V1', + version: 'v1' + }, + components: { + schemas: { + login_success: { + type: :object, + properties: { + id: { type: :integer }, + display_name: { type: :string }, + email: { type: :string }, + token: { type: :string } + } + }, + login_failure: { + type: :object, + properties: { + message: { type: :string } + } + } + } + }, + paths: {}, + servers: [ + { + url: 'https://{defaultHost}', + variables: { + defaultHost: { + default: 'www.example.com' + } + } + } + ] + } + } + + # Specify the format of the output Swagger file when running 'rswag:specs:swaggerize'. + # The swagger_docs configuration option has the filename including format in + # the key, this may want to be changed to avoid putting yaml in json files. + # Defaults to json. Accepts ':json' and ':yaml'. + config.swagger_format = :yaml +end diff --git a/swagger/v1/swagger.yaml b/swagger/v1/swagger.yaml new file mode 100644 index 0000000000..cade224cf1 --- /dev/null +++ b/swagger/v1/swagger.yaml @@ -0,0 +1,61 @@ +--- +openapi: 3.0.1 +info: + title: API V1 + version: v1 +components: + schemas: + login_success: + type: object + properties: + id: + type: integer + display_name: + type: string + email: + type: string + token: + type: string + login_failure: + type: object + properties: + message: + type: string +paths: + "/api/v1/users/sign_in": + post: + summary: Signs in a user + tags: + - Sessions + parameters: [] + responses: + '201': + description: user signed in + content: + application/json: + schema: + "$ref": "#/components/schemas/login_success" + '401': + description: invalid credentials + content: + application/json: + schema: + "$ref": "#/components/schemas/login_failure" + requestBody: + content: + application/json: + schema: + type: object + properties: + email: + type: string + password: + type: string + required: + - email + - password +servers: +- url: https://{defaultHost} + variables: + defaultHost: + default: www.example.com From 83bb78f6438b620d10ceb7600fa95dc19c1fae76 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Wed, 16 Aug 2023 16:22:45 -0400 Subject: [PATCH 06/10] update gemfile; add blueprinter and remove active model serializer --- Gemfile | 3 ++- Gemfile.lock | 13 ++++--------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/Gemfile b/Gemfile index ea3514e823..f947a26ad9 100644 --- a/Gemfile +++ b/Gemfile @@ -46,9 +46,10 @@ gem "sprockets-rails" # The original asset pipeline for Rails [https://github.co gem "stimulus-rails" gem "strong_migrations" gem "tzinfo-data", platforms: %i[mingw mswin x64_mingw jruby] # Windows does not include zoneinfo files, so bundle the tzinfo-data gem -gem "active_model_serializers" # for JSON serialization gem "rswag-api" gem "rswag-ui" +gem "blueprinter" # for JSON serialization +gem "oj" # faster JSON parsing 🍊 group :development, :test do gem "bullet" # Detect and fix N+1 queries diff --git a/Gemfile.lock b/Gemfile.lock index 37f7326bf6..823d71ad9b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -46,11 +46,6 @@ GEM erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.1, >= 1.2.0) - active_model_serializers (0.10.13) - actionpack (>= 4.1, < 7.1) - activemodel (>= 4.1, < 7.1) - case_transform (>= 0.2) - jsonapi-renderer (>= 0.1.1.beta1, < 0.3) activejob (7.0.6) activesupport (= 7.0.6) globalid (>= 0.3.6) @@ -100,6 +95,7 @@ GEM parser (>= 2.4) smart_properties bindex (0.8.1) + blueprinter (0.25.3) brakeman (6.0.1) bugsnag (6.26.0) concurrent-ruby (~> 1.0) @@ -123,8 +119,6 @@ GEM capybara-screenshot (1.0.26) capybara (>= 1.0, < 4) launchy - case_transform (0.2) - activesupport caxlsx (3.4.1) htmlentities (~> 4.3, >= 4.3.4) marcel (~> 1.0) @@ -253,7 +247,6 @@ GEM railties (>= 6.0.0) json-schema (3.0.0) addressable (>= 2.8) - jsonapi-renderer (0.2.2) jwt (2.7.1) launchy (2.5.0) addressable (~> 2.7) @@ -304,6 +297,7 @@ GEM noticed (1.6.3) http (>= 4.0.0) rails (>= 5.2.0) + oj (3.15.1) orm_adapter (0.5.0) parallel (1.22.1) paranoia (2.6.2) @@ -508,11 +502,11 @@ PLATFORMS x86_64-linux DEPENDENCIES - active_model_serializers after_party amazing_print annotate azure-storage-blob + blueprinter brakeman bugsnag bullet @@ -545,6 +539,7 @@ DEPENDENCIES net-pop net-smtp noticed + oj paranoia pdf-forms pg From 1c5bb654dbaf95a82977a2cd719fbce1399306d0 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Wed, 16 Aug 2023 16:28:31 -0400 Subject: [PATCH 07/10] remove active model serializer files --- app/serializers/api/v1/session_serializer.rb | 4 --- .../api/v1/base_controller_spec.rb | 33 ------------------- .../api/v1/users/sessions_controller_spec.rb | 26 --------------- .../api/v1/session_serializer_spec.rb | 20 ----------- 4 files changed, 83 deletions(-) delete mode 100644 app/serializers/api/v1/session_serializer.rb delete mode 100644 spec/controllers/api/v1/base_controller_spec.rb delete mode 100644 spec/controllers/api/v1/users/sessions_controller_spec.rb delete mode 100644 spec/serializers/api/v1/session_serializer_spec.rb diff --git a/app/serializers/api/v1/session_serializer.rb b/app/serializers/api/v1/session_serializer.rb deleted file mode 100644 index 1807056fe2..0000000000 --- a/app/serializers/api/v1/session_serializer.rb +++ /dev/null @@ -1,4 +0,0 @@ -class Api::V1::SessionSerializer < ActiveModel::Serializer - type "session" - attributes :id, :display_name, :email, :token -end diff --git a/spec/controllers/api/v1/base_controller_spec.rb b/spec/controllers/api/v1/base_controller_spec.rb deleted file mode 100644 index f65989cc20..0000000000 --- a/spec/controllers/api/v1/base_controller_spec.rb +++ /dev/null @@ -1,33 +0,0 @@ -require "rails_helper" - -RSpec.describe Api::V1::BaseController, type: :controller do - # assert token and options get parsed correctly - controller do - def index - render json: {message: "Successfully autenticated"} - end - - def authenticated_user! - super - end - end - - # assert authenticate_user! works - describe "GET #index" do - let(:user) { create(:volunteer) } - it "returns http success" do - request.headers["Authorization"] = "Token token=#{user.token}, email=#{user.email}" - # print request.headers["Authorization"] - get :index - expect(response).to have_http_status(:success) - expect(response.body).to eq({message: "Successfully autenticated"}.to_json) - end - # assert authenticate_user! works with invalid email - it "returns http unauthorized" do - request.headers["Authorization"] = "Token token=, email=#{user.email}" - get :index - expect(response).to have_http_status(:unauthorized) - expect(response.body).to eq({message: "Wrong password or email"}.to_json) - end - end -end diff --git a/spec/controllers/api/v1/users/sessions_controller_spec.rb b/spec/controllers/api/v1/users/sessions_controller_spec.rb deleted file mode 100644 index 58e9c039cc..0000000000 --- a/spec/controllers/api/v1/users/sessions_controller_spec.rb +++ /dev/null @@ -1,26 +0,0 @@ -require "rails_helper" -require "spec_helper" - -RSpec.describe Api::V1::Users::SessionsController, type: :api do - let(:casa_org) { create(:casa_org) } - let(:volunteer) { create(:volunteer, casa_org: casa_org) } - - it "should handle correct sign in" do - post "/api/v1/users/sign_in", {email: volunteer.email, password: volunteer.password} - # print last_response.headers - # print last_response.body - # expect(last_response.headers).to have_key "Authorization" - # expect(last_response.headers["Authorization"]).to be_starts_with("Bearer") - expect(last_response.body).to eq Api::V1::SessionSerializer.new(volunteer).to_json - expect(last_response.status).to eq 201 - expect(last_response.content_type).to eq("application/json; charset=utf-8") - end - - it "should handle incorrect sign in" do - post "/api/v1/users/sign_in", {email: "suzume@tojimari.jp", password: ""} - body = JSON.parse(last_response.body, symbolize_names: true) - expect(body.dig(:message)).to eq "Wrong password or username" - expect(last_response.status).to eq 401 - expect(last_response.content_type).to eq("application/json; charset=utf-8") - end -end diff --git a/spec/serializers/api/v1/session_serializer_spec.rb b/spec/serializers/api/v1/session_serializer_spec.rb deleted file mode 100644 index 60ad79f3ea..0000000000 --- a/spec/serializers/api/v1/session_serializer_spec.rb +++ /dev/null @@ -1,20 +0,0 @@ -require "rails_helper" - -RSpec.describe Api::V1::SessionSerializer, type: :serializer do - before(:each) do - @casa_org = create(:casa_org) - @volunteer = create(:volunteer, casa_org: @casa_org) - @serializer = Api::V1::SessionSerializer.new(@volunteer) - @serialization = ActiveModelSerializers::Adapter.create(@serializer) - end - - subject { JSON.parse(@serialization.to_json) } - - it "should have matching attributes" do - expect(subject["id"]).to eq(@volunteer.id) - expect(subject["email"]).to eq(@volunteer.email) - expect(subject["display_name"]).to eq(@volunteer.display_name) - expect(subject["token"]).to eq(@volunteer.token) - expect(subject.length).to eq(4) - end -end From 3fa1bd14591b4afa0003e5aedddbd20f411bab2a Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Wed, 16 Aug 2023 16:30:26 -0400 Subject: [PATCH 08/10] update files with blueprinter and create request specs --- app/blueprints/api/v1/session_blueprint.rb | 5 +++ .../api/v1/users/sessions_controller.rb | 4 +-- config/initializers/blueprinter.rb | 5 +++ spec/requests/api/v1/base_spec.rb | 33 +++++++++++++++++++ spec/requests/api/v1/users/sessions_spec.rb | 12 +++++-- 5 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 app/blueprints/api/v1/session_blueprint.rb create mode 100644 config/initializers/blueprinter.rb create mode 100644 spec/requests/api/v1/base_spec.rb diff --git a/app/blueprints/api/v1/session_blueprint.rb b/app/blueprints/api/v1/session_blueprint.rb new file mode 100644 index 0000000000..2844774303 --- /dev/null +++ b/app/blueprints/api/v1/session_blueprint.rb @@ -0,0 +1,5 @@ +class Api::V1::SessionBlueprint < Blueprinter::Base + identifier :id + + fields :id, :display_name, :email, :token +end diff --git a/app/controllers/api/v1/users/sessions_controller.rb b/app/controllers/api/v1/users/sessions_controller.rb index 59f8233db7..ecf0bad9d4 100644 --- a/app/controllers/api/v1/users/sessions_controller.rb +++ b/app/controllers/api/v1/users/sessions_controller.rb @@ -4,9 +4,9 @@ def create load_resource # p @user if @user - render json: @user, serializer: Api::V1::SessionSerializer, status: 201 + render json: Api::V1::SessionBlueprint.render(@user), status: 201 else - render json: {message: "Wrong password or username"}, status: 401 + render json: {message: "Wrong password or email"}, status: 401 end end diff --git a/config/initializers/blueprinter.rb b/config/initializers/blueprinter.rb new file mode 100644 index 0000000000..3b0df875d3 --- /dev/null +++ b/config/initializers/blueprinter.rb @@ -0,0 +1,5 @@ +require 'oj' # you can skip this if OJ has already been required. + +Blueprinter.configure do |config| + config.generator = Oj # default is JSON +end diff --git a/spec/requests/api/v1/base_spec.rb b/spec/requests/api/v1/base_spec.rb new file mode 100644 index 0000000000..850a712abd --- /dev/null +++ b/spec/requests/api/v1/base_spec.rb @@ -0,0 +1,33 @@ +require "rails_helper" + +RSpec.describe "Base Controller", type: :request do + before do + base_controller = Class.new(Api::V1::BaseController) do + def index + render json: {message: "Successfully autenticated"} + end + end + stub_const("BaseController", base_controller) + Rails.application.routes.disable_clear_and_finalize = true + Rails.application.routes.draw do + get "/index", to: "base#index" + end + end + + after { Rails.application.reload_routes! } + + # test authenticate_user! works + describe "GET #index" do + let(:user) { create(:volunteer) } + it "returns http success" do + get "/index", headers: { "Authorization" => "Token token=#{user.token}, email=#{user.email}" } + expect(response).to have_http_status(:success) + expect(response.body).to eq({message: "Successfully autenticated"}.to_json) + end + it "returns http unauthorized" do + get "/index", headers: { "Authorization" => "Token token=, email=#{user.email}" } + expect(response).to have_http_status(:unauthorized) + expect(response.body).to eq({message: "Wrong password or email"}.to_json) + end + end +end diff --git a/spec/requests/api/v1/users/sessions_spec.rb b/spec/requests/api/v1/users/sessions_spec.rb index d20f316de7..8360bc3229 100644 --- a/spec/requests/api/v1/users/sessions_spec.rb +++ b/spec/requests/api/v1/users/sessions_spec.rb @@ -21,13 +21,21 @@ response '201', 'user signed in' do let(:user) { { email: volunteer.email, password: volunteer.password } } schema '$ref' => '#/components/schemas/login_success' - run_test! + run_test! do |response| + expect(response.content_type).to eq('application/json; charset=utf-8') + expect(response.body).to eq(Api::V1::SessionBlueprint.render(volunteer)) + expect(response.status).to eq(201) + end end response '401', 'invalid credentials' do let(:user) { { email: 'foo', password: 'bar' } } schema '$ref' => '#/components/schemas/login_failure' - run_test! + run_test! do |response| + expect(response.content_type).to eq('application/json; charset=utf-8') + expect(response.body).to eq({ message: 'Wrong password or email' }.to_json) + expect(response.status).to eq(401) + end end end end From 9fadddda6b1ad8cf8023badf0137f1690964ea2d Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Wed, 16 Aug 2023 17:21:51 -0400 Subject: [PATCH 09/10] remove comments and add lint files --- app/controllers/api/v1/base_controller.rb | 4 -- .../api/v1/users/sessions_controller.rb | 5 --- config/initializers/blueprinter.rb | 2 +- config/initializers/rswag_api.rb | 5 +-- config/initializers/rswag_ui.rb | 3 +- config/routes.rb | 4 +- spec/requests/api/v1/base_spec.rb | 8 ++-- spec/requests/api/v1/users/sessions_spec.rb | 42 +++++++++---------- spec/swagger_helper.rb | 30 ++++++------- 9 files changed, 46 insertions(+), 57 deletions(-) diff --git a/app/controllers/api/v1/base_controller.rb b/app/controllers/api/v1/base_controller.rb index ea4e7ba94d..bba3dcd72d 100644 --- a/app/controllers/api/v1/base_controller.rb +++ b/app/controllers/api/v1/base_controller.rb @@ -4,11 +4,7 @@ class Api::V1::BaseController < ActionController::API def authenticate_user! token, options = ActionController::HttpAuthentication::Token.token_and_options(request) - # return nil unless token && options.is_a?(Hash) user = User.find_by(email: options[:email]) - p "#######" - print token - p "#######" if user && token && ActiveSupport::SecurityUtils.secure_compare(user.token, token) @current_user = user else diff --git a/app/controllers/api/v1/users/sessions_controller.rb b/app/controllers/api/v1/users/sessions_controller.rb index ecf0bad9d4..835e5966e8 100644 --- a/app/controllers/api/v1/users/sessions_controller.rb +++ b/app/controllers/api/v1/users/sessions_controller.rb @@ -1,8 +1,6 @@ class Api::V1::Users::SessionsController < Api::V1::BaseController - # POST /api/v1/users/sign_in def create load_resource - # p @user if @user render json: Api::V1::SessionBlueprint.render(@user), status: 201 else @@ -17,12 +15,9 @@ def user_params end def load_resource - # print user_params - # print user_params @user = User.find_by(email: user_params[:email]) if !@user&.valid_password?(user_params[:password]) @user = nil end - # @user = User.find_by(email: user_params[:email])&.valid_password?(user_params[:password]) end end diff --git a/config/initializers/blueprinter.rb b/config/initializers/blueprinter.rb index 3b0df875d3..da2ee10ab5 100644 --- a/config/initializers/blueprinter.rb +++ b/config/initializers/blueprinter.rb @@ -1,4 +1,4 @@ -require 'oj' # you can skip this if OJ has already been required. +require "oj" # you can skip this if OJ has already been required. Blueprinter.configure do |config| config.generator = Oj # default is JSON diff --git a/config/initializers/rswag_api.rb b/config/initializers/rswag_api.rb index 4d72f68760..e7eaa19e82 100644 --- a/config/initializers/rswag_api.rb +++ b/config/initializers/rswag_api.rb @@ -1,14 +1,13 @@ Rswag::Api.configure do |c| - # Specify a root folder where Swagger JSON files are located # This is used by the Swagger middleware to serve requests for API descriptions # NOTE: If you're using rswag-specs to generate Swagger, you'll need to ensure # that it's configured to generate files in the same folder - c.swagger_root = Rails.root.to_s + '/swagger' + c.swagger_root = Rails.root.to_s + "/swagger" # Inject a lambda function to alter the returned Swagger prior to serialization # The function will have access to the rack env for the current request # For example, you could leverage this to dynamically assign the "host" property # - #c.swagger_filter = lambda { |swagger, env| swagger['host'] = env['HTTP_HOST'] } + # c.swagger_filter = lambda { |swagger, env| swagger['host'] = env['HTTP_HOST'] } end diff --git a/config/initializers/rswag_ui.rb b/config/initializers/rswag_ui.rb index 0a768c17bc..adc10a2c1f 100644 --- a/config/initializers/rswag_ui.rb +++ b/config/initializers/rswag_ui.rb @@ -1,5 +1,4 @@ Rswag::Ui.configure do |c| - # List the Swagger endpoints that you want to be documented through the # swagger-ui. The first parameter is the path (absolute or relative to the UI # host) to the corresponding endpoint and the second is a title that will be @@ -8,7 +7,7 @@ # (under swagger_root) as JSON or YAML endpoints, then the list below should # correspond to the relative paths for those endpoints. - c.swagger_endpoint '/api-docs/v1/swagger.yaml', 'API V1 Docs' + c.swagger_endpoint "/api-docs/v1/swagger.yaml", "API V1 Docs" # Add Basic Auth in case your API is private # c.basic_auth_enabled = true diff --git a/config/routes.rb b/config/routes.rb index 1343c5be1e..eb48752c54 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true Rails.application.routes.draw do - mount Rswag::Ui::Engine => '/api-docs' - mount Rswag::Api::Engine => '/api-docs' + mount Rswag::Ui::Engine => "/api-docs" + mount Rswag::Api::Engine => "/api-docs" devise_for :all_casa_admins, path: "all_casa_admins", controllers: {sessions: "all_casa_admins/sessions"} devise_for :users, controllers: {sessions: "users/sessions", passwords: "users/passwords"} diff --git a/spec/requests/api/v1/base_spec.rb b/spec/requests/api/v1/base_spec.rb index 850a712abd..867949168d 100644 --- a/spec/requests/api/v1/base_spec.rb +++ b/spec/requests/api/v1/base_spec.rb @@ -19,13 +19,13 @@ def index # test authenticate_user! works describe "GET #index" do let(:user) { create(:volunteer) } - it "returns http success" do - get "/index", headers: { "Authorization" => "Token token=#{user.token}, email=#{user.email}" } + it "returns http success when valid credentials" do + get "/index", headers: {"Authorization" => "Token token=#{user.token}, email=#{user.email}"} expect(response).to have_http_status(:success) expect(response.body).to eq({message: "Successfully autenticated"}.to_json) end - it "returns http unauthorized" do - get "/index", headers: { "Authorization" => "Token token=, email=#{user.email}" } + it "returns http unauthorized if invalid token" do + get "/index", headers: {"Authorization" => "Token token=, email=#{user.email}"} expect(response).to have_http_status(:unauthorized) expect(response.body).to eq({message: "Wrong password or email"}.to_json) end diff --git a/spec/requests/api/v1/users/sessions_spec.rb b/spec/requests/api/v1/users/sessions_spec.rb index 8360bc3229..4249d273a1 100644 --- a/spec/requests/api/v1/users/sessions_spec.rb +++ b/spec/requests/api/v1/users/sessions_spec.rb @@ -1,39 +1,39 @@ -require 'swagger_helper' +require "swagger_helper" -RSpec.describe 'sessions API', type: :request do - path '/api/v1/users/sign_in' do - post 'Signs in a user' do - tags 'Sessions' - consumes 'application/json' - produces 'application/json' +RSpec.describe "sessions API", type: :request do + path "/api/v1/users/sign_in" do + post "Signs in a user" do + tags "Sessions" + consumes "application/json" + produces "application/json" parameter name: :user, in: :body, schema: { type: :object, - properties: { - email: { type: :string }, - password: { type: :string } - }, - required: %w[email password] + properties: { + email: {type: :string}, + password: {type: :string} + }, + required: %w[email password] } let(:casa_org) { create(:casa_org) } let(:volunteer) { create(:volunteer, casa_org: casa_org) } - response '201', 'user signed in' do - let(:user) { { email: volunteer.email, password: volunteer.password } } - schema '$ref' => '#/components/schemas/login_success' + response "201", "user signed in" do + let(:user) { {email: volunteer.email, password: volunteer.password} } + schema "$ref" => "#/components/schemas/login_success" run_test! do |response| - expect(response.content_type).to eq('application/json; charset=utf-8') + expect(response.content_type).to eq("application/json; charset=utf-8") expect(response.body).to eq(Api::V1::SessionBlueprint.render(volunteer)) expect(response.status).to eq(201) end end - response '401', 'invalid credentials' do - let(:user) { { email: 'foo', password: 'bar' } } - schema '$ref' => '#/components/schemas/login_failure' + response "401", "invalid credentials" do + let(:user) { {email: "foo", password: "bar"} } + schema "$ref" => "#/components/schemas/login_failure" run_test! do |response| - expect(response.content_type).to eq('application/json; charset=utf-8') - expect(response.body).to eq({ message: 'Wrong password or email' }.to_json) + expect(response.content_type).to eq("application/json; charset=utf-8") + expect(response.body).to eq({message: "Wrong password or email"}.to_json) expect(response.status).to eq(401) end end diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb index 84b419fdc8..aa4752ce5e 100644 --- a/spec/swagger_helper.rb +++ b/spec/swagger_helper.rb @@ -1,12 +1,12 @@ # frozen_string_literal: true -require 'rails_helper' +require "rails_helper" RSpec.configure do |config| # Specify a root folder where Swagger JSON files are generated # NOTE: If you're using the rswag-api to serve API descriptions, you'll need # to ensure that it's configured to serve Swagger from the same folder - config.swagger_root = Rails.root.join('swagger').to_s + config.swagger_root = Rails.root.join("swagger").to_s # Define one or more Swagger documents and provide global metadata for each one # When you run the 'rswag:specs:swaggerize' rake task, the complete Swagger will @@ -15,27 +15,27 @@ # document below. You can override this behavior by adding a swagger_doc tag to the # the root example_group in your specs, e.g. describe '...', swagger_doc: 'v2/swagger.json' config.swagger_docs = { - 'v1/swagger.yaml' => { - openapi: '3.0.1', + "v1/swagger.yaml" => { + openapi: "3.0.1", info: { - title: 'API V1', - version: 'v1' + title: "API V1", + version: "v1" }, components: { schemas: { login_success: { type: :object, properties: { - id: { type: :integer }, - display_name: { type: :string }, - email: { type: :string }, - token: { type: :string } - } - }, + id: {type: :integer}, + display_name: {type: :string}, + email: {type: :string}, + token: {type: :string} + } + }, login_failure: { type: :object, properties: { - message: { type: :string } + message: {type: :string} } } } @@ -43,10 +43,10 @@ paths: {}, servers: [ { - url: 'https://{defaultHost}', + url: "https://{defaultHost}", variables: { defaultHost: { - default: 'www.example.com' + default: "www.example.com" } } } From 8bb45abe50a45c7bd6fcbc921499aa9a581b0bf9 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Thu, 31 Aug 2023 17:26:46 -0400 Subject: [PATCH 10/10] resolve pr changes --- .allow_skipping_tests | 1 + .../api/v1/users/sessions_controller.rb | 2 +- app/models/concerns/generate_token.rb | 16 ---------------- app/models/user.rb | 3 +-- config/credentials/development.yml.enc | 2 +- config/environments/development.rb | 4 +--- db/migrate/20230710025852_add_token_to_users.rb | 2 -- .../20230822152341_drop_jwt_denylist_table.rb | 5 +++++ db/schema.rb | 8 +------- .../20230822145532_populate_api_tokens.rake | 14 ++++++++++++++ spec/models/user_spec.rb | 9 --------- 11 files changed, 25 insertions(+), 41 deletions(-) delete mode 100644 app/models/concerns/generate_token.rb create mode 100644 db/migrate/20230822152341_drop_jwt_denylist_table.rb create mode 100644 lib/tasks/deployment/20230822145532_populate_api_tokens.rake diff --git a/.allow_skipping_tests b/.allow_skipping_tests index 906e7b14e9..e84b4effc8 100644 --- a/.allow_skipping_tests +++ b/.allow_skipping_tests @@ -52,3 +52,4 @@ values/casa_admin_parameters.rb values/supervisor_parameters.rb values/user_parameters.rb values/volunteer_parameters.rb +blueprints/api/v1/session_blueprint.rb diff --git a/app/controllers/api/v1/users/sessions_controller.rb b/app/controllers/api/v1/users/sessions_controller.rb index 835e5966e8..98635860bf 100644 --- a/app/controllers/api/v1/users/sessions_controller.rb +++ b/app/controllers/api/v1/users/sessions_controller.rb @@ -16,7 +16,7 @@ def user_params def load_resource @user = User.find_by(email: user_params[:email]) - if !@user&.valid_password?(user_params[:password]) + unless @user&.valid_password?(user_params[:password]) @user = nil end end diff --git a/app/models/concerns/generate_token.rb b/app/models/concerns/generate_token.rb deleted file mode 100644 index b1e459e978..0000000000 --- a/app/models/concerns/generate_token.rb +++ /dev/null @@ -1,16 +0,0 @@ -module GenerateToken - extend ActiveSupport::Concern - - included do - def ensure_token - self.token = generate_hex(:token) unless token.present? - end - - def generate_hex(column) - loop do - hex = SecureRandom.hex(32) - break hex unless self.class.where(column => hex).any? - end - end - end -end diff --git a/app/models/user.rb b/app/models/user.rb index fc80076036..bdabbf1146 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -5,12 +5,11 @@ class User < ApplicationRecord include Roles include ByOrganizationScope include DateHelper - include GenerateToken before_update :record_previous_email after_create :skip_email_confirmation_upon_creation before_save :normalize_phone_number - before_validation :ensure_token + has_secure_token :token, length: 36 validates_with UserValidator diff --git a/config/credentials/development.yml.enc b/config/credentials/development.yml.enc index 4c0cc7ab67..c0e4e30d44 100644 --- a/config/credentials/development.yml.enc +++ b/config/credentials/development.yml.enc @@ -1 +1 @@ -svCtLWmi6TUWfy4jhsNxZgGKdzBrjq5JjKkGUaDA5tlP2XFn6XY8lJDVhF+T82kGjwT4EgsBheMZqPMbytlJ6iSDBIq/bHfjl1E5Zx3DqCkd4gDYgVK0roJffesKQPuWUSQUzvJV9pZ9VQEKbh+YA/I/N6aWGbkYlKXTOPHMY7F+rfiKXb8vHodUGWxCTycsWLpe/ohBvF7zzSwxkG7sEmbnRnqYd2Tmn0ASf6vNKXOzPamQ21rrgUss427/zjCjzWHCk4iUaHnhQQYwC2zJ+m1/0Uu+sM5CkYJhddsPbeeQkd7vgPjHBylgkT6L86XTz8sBrQDZB51TbmNouygu96NzQwE472c0csFEWwjz7fepy7sZkHN5KqQ=--dx6D/QqFOeacGYGg--+r3ffqcg8wONL9oMId9u5g== \ No newline at end of file +aewvdbZoQz8v7s3UlJ/+XOIrxpj1/nP2/dA7FkLGvTgmu8lZrnyecC19sDE6bcZN4XsnIqDomjSg/CL8TefHKXOsaoNNKmW8YPVfoH8AmlqXxvJduiZNuXlOcf7SR01E7E0r1VIdRga6g9KtOHBbgtc6hQyOs/2ajSxbD3gY5IFWnWNHIqMEWMUMy/PXtSSxUr+FdNCgdod9Rx0EEiecfEz1tMBP/V69dRwSrM5yfTeogkUPpOqReFisTbn9f0yolmNhhxo7nPoPzyeEcGHl4+maS1GHa6uYQ2n2d2t34FmhcDttI+rV7ITU9LmuwVcjgCE9fPxMUZ9bX2UBUEHialBZ8S+izXyBAKGTvbQw+/Wk9KNT98Tl3Gg=--BRmMgMTOgyAZUyw4--2OyLty/a3xH0OjlI0sf9Yw== \ No newline at end of file diff --git a/config/environments/development.rb b/config/environments/development.rb index 4a48e22576..29d10dbab1 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -77,7 +77,5 @@ # Uncomment if you wish to allow Action Cable access from any origin. # config.action_cable.disable_request_forgery_protection = true - config.hosts << ".ngrok-free.app" - - config.assets.digest = false + config.hosts << ENV["DEV_HOSTS"] end diff --git a/db/migrate/20230710025852_add_token_to_users.rb b/db/migrate/20230710025852_add_token_to_users.rb index 9d06ae6a54..7a072c1e90 100644 --- a/db/migrate/20230710025852_add_token_to_users.rb +++ b/db/migrate/20230710025852_add_token_to_users.rb @@ -1,8 +1,6 @@ class AddTokenToUsers < ActiveRecord::Migration[7.0] def up add_column :users, :token, :string - User.find_each { |user| user.save! } - # change_column_null :users, :token, false, 1 end def down diff --git a/db/migrate/20230822152341_drop_jwt_denylist_table.rb b/db/migrate/20230822152341_drop_jwt_denylist_table.rb new file mode 100644 index 0000000000..7d25b5c652 --- /dev/null +++ b/db/migrate/20230822152341_drop_jwt_denylist_table.rb @@ -0,0 +1,5 @@ +class DropJwtDenylistTable < ActiveRecord::Migration[7.0] + def change + drop_table :jwt_denylist, if_exists: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 5fbb61a153..55a07520a1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_08_09_002819) do +ActiveRecord::Schema[7.0].define(version: 2023_08_22_152341) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -362,12 +362,6 @@ t.index ["casa_org_id"], name: "index_judges_on_casa_org_id" end - create_table "jwt_denylist", force: :cascade do |t| - t.string "jti", null: false - t.datetime "exp", null: false - t.index ["jti"], name: "index_jwt_denylist_on_jti" - end - create_table "languages", force: :cascade do |t| t.string "name" t.bigint "casa_org_id", null: false diff --git a/lib/tasks/deployment/20230822145532_populate_api_tokens.rake b/lib/tasks/deployment/20230822145532_populate_api_tokens.rake new file mode 100644 index 0000000000..8b7dbb4ebe --- /dev/null +++ b/lib/tasks/deployment/20230822145532_populate_api_tokens.rake @@ -0,0 +1,14 @@ +namespace :after_party do + desc "Deployment task: populate_api_tokens" + task populate_api_tokens: :environment do + puts "Running deploy task 'populate_api_tokens'" unless Rails.env.test? + + # Put your task implementation HERE. + User.find_each { |user| user.save! } + + # Update task as completed. If you remove the line below, the task will + # run with every deploy (or every time you call after_party:run). + AfterParty::TaskRecord + .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 92e9d7f7db..7679cf071a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -49,15 +49,6 @@ end end - describe "token generation" do - it "generates a token before validation" do - user = User.new - expect(user.token).to be_nil - user.valid? - expect(user.token).not_to be_nil - end - end - describe "#case_contacts_for" do let(:volunteer) { create(:volunteer, :with_casa_cases) } let(:case_of_interest) { volunteer.casa_cases.first }