From 32101919c2cd13790a9261699d673a1105c1508e Mon Sep 17 00:00:00 2001 From: Oreoluwa Akinniranye Date: Thu, 26 Jan 2017 09:04:08 +0100 Subject: [PATCH 01/12] first implementation of new andela auth TODO: refactor and specs --- Gemfile | 5 +- Gemfile.lock | 6 +- app/authenticators/andela_auth_v2.rb | 32 ++++++++ .../connection/faraday_connection.rb | 23 ++++++ app/controllers/application_controller.rb | 79 ++++++++++++------- 5 files changed, 113 insertions(+), 32 deletions(-) create mode 100644 app/authenticators/andela_auth_v2.rb create mode 100644 app/authenticators/connection/faraday_connection.rb diff --git a/Gemfile b/Gemfile index 36a2644..b37a952 100644 --- a/Gemfile +++ b/Gemfile @@ -15,7 +15,6 @@ gem "jwt" gem 'rack-cors' # for perfomance and monitoring timeout ensures that when a request is taking too long, it is automatically terminated # new relic provides a dashboard to view the perfomance of our application -gem "rack-timeout" gem 'newrelic_rpm' gem "bugsnag" @@ -23,6 +22,9 @@ gem "bugsnag" gem 'elasticsearch-model' gem 'elasticsearch-rails' +gem 'faraday' +gem 'faraday_middleware' + # sidekiq for asynchronous jobs. relevant to enable app to keep functioning even when there are long running jobs gem 'sidekiq' gem 'sidekiq-failures' @@ -59,6 +61,7 @@ group :test do end group :production, :staging do + gem "rack-timeout" gem "pg" gem "rails_12factor" end diff --git a/Gemfile.lock b/Gemfile.lock index c83bf1f..be03fc4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -88,6 +88,8 @@ GEM i18n (~> 0.5) faraday (0.9.2) multipart-post (>= 1.2, < 3) + faraday_middleware (0.10.0) + faraday (>= 0.7.4, < 0.10) ffi (1.9.10) figaro (1.1.1) thor (~> 0.14) @@ -295,6 +297,8 @@ DEPENDENCIES elasticsearch-rails factory_girl_rails faker + faraday + faraday_middleware figaro guard-rspec jbuilder @@ -330,4 +334,4 @@ RUBY VERSION ruby 2.3.1p112 BUNDLED WITH - 1.12.5 + 1.13.6 diff --git a/app/authenticators/andela_auth_v2.rb b/app/authenticators/andela_auth_v2.rb new file mode 100644 index 0000000..7e823d4 --- /dev/null +++ b/app/authenticators/andela_auth_v2.rb @@ -0,0 +1,32 @@ +class AndelaAuthV2 + attr_reader :response + + def self.authenticate(token) + conn = Connection::FaradayConnection.connection(token) + response = conn.get('/api/v1/users/me') + new(response) + end + + def initialize(response) + @response = response + end + + def body + @body ||= JSON.parse(response.body) + end + + def authenticated? + # check response status and body + !body.include? 'error' + end + + def current_user + if authenticated? + body + else + raise UserNotFoundError.new('User could not be found') + end + end + + class UserNotFoundError < StandardError; end +end diff --git a/app/authenticators/connection/faraday_connection.rb b/app/authenticators/connection/faraday_connection.rb new file mode 100644 index 0000000..3aee864 --- /dev/null +++ b/app/authenticators/connection/faraday_connection.rb @@ -0,0 +1,23 @@ +class Connection::FaradayConnection + BASE_ANDELA_URL = "http://api-staging.andela.com" # use env variable to set the url + + def self.connection(token) + options = { + headers: { + "Authorization" => "Bearer #{token}", + "Accept" => 'application/json; charset=utf-8', + }, + ssl: { + verify: false + } + } + + ::Faraday::Connection.new(BASE_ANDELA_URL, options) do |conn| + conn.use ::Faraday::Request::Multipart + conn.use ::Faraday::Request::UrlEncoded + conn.use FaradayMiddleware::FollowRedirects + conn.response :logger, Rails.logger + conn.adapter ::Faraday.default_adapter + end + end +end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 27f03bd..4ef8dce 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -3,7 +3,7 @@ class ApplicationController < ActionController::API attr_reader :current_user helper_method :current_user - before_action :authenticate_user, except: [:login, :logout] + before_action :authenticate_user#, except: [:login, :logout] def resource_not_found not_found = "The resource you tried to access was not found" @@ -14,41 +14,60 @@ def invalid_request(message = error_msg, status = 400) render json: {errors: message}, status: status end - def login - user = authenticate_cookie - @current_user = User.from_andela_auth(user) if user && !user['isGuest'] - return unauthorized_token unless @current_user + # def login + # user = authenticate_cookie + # @current_user = User.from_andela_auth(user) if user && !user['isGuest'] + # return unauthorized_token unless @current_user + # + # @token = TokenManager.generate_token(@current_user.id) + # end - @token = TokenManager.generate_token(@current_user.id) - end - - def logout - cookie = request.headers['HTTP_ANDELA_COOKIE'] - return invalid_request("Cookie must be provided") unless cookie - CookieHandler.logout_cookie(cookie) - render json: {message: 'logged out'}, status: 200 - end + # def logout + # cookie = request.headers['HTTP_ANDELA_COOKIE'] + # return invalid_request("Cookie must be provided") unless cookie + # CookieHandler.logout_cookie(cookie) + # render json: {message: 'logged out'}, status: 200 + # end private - def authenticate_user - (authenticate_token && authenticate_cookie) || unauthorized_token - # authenticate_cookie || unauthorized_token - end + def authenticate_user + strategy, token = request.headers['Authorization'].split + auth = AndelaAuthV2.authenticate(token) - def authenticate_token - authenticate_with_http_token do |auth_token, _| - user_id = TokenManager.authenticate(auth_token)['user'] - @token = TokenManager.generate_token(user_id) if user_id + # during refactor these methods/checks should be moved out of here + if strategy == 'Bearer' && auth.authenticated? + auth_user = auth.current_user + @current_user = User.find_or_create_by(email: auth_user['email']) + # we always want to ensure these attrs are in sync with the auth system + @current_user.update_attributes( + name: auth_user['name'], + image: auth_user['picture'], + active: (auth_user['status'] == 'active') + ) + else + unauthorized_token end - end - def authenticate_cookie - cookie = request.headers['HTTP_ANDELA_COOKIE'] - return unless cookie - user = CookieHandler.validate_with_cookie(cookie) - @current_user = User.find_by(email: user['email']) if user - user - end + end + # def authenticate_user + # (authenticate_token && authenticate_cookie) || unauthorized_token + # # authenticate_cookie || unauthorized_token + # end + + # def authenticate_token + # authenticate_with_http_token do |auth_token, _| + # user_id = TokenManager.authenticate(auth_token)['user'] + # @token = TokenManager.generate_token(user_id) if user_id + # end + # end + + # def authenticate_cookie + # cookie = request.headers['HTTP_ANDELA_COOKIE'] + # return unless cookie + # user = CookieHandler.validate_with_cookie(cookie) + # @current_user = User.find_by(email: user['email']) if user + # user + # end def unauthorized_token self.headers['WWW-Authenticate'] = 'Token realm="Application"' From d749e450bcc216c000d292c42580405f187234fc Mon Sep 17 00:00:00 2001 From: Ologho Cyril Paul Date: Fri, 27 Jan 2017 16:51:44 +0100 Subject: [PATCH 02/12] Change Authentication mechanism for new Andela Auth system --- app/controllers/application_controller.rb | 40 +++-------------------- app/views/application/login.json.jbuilder | 1 - 2 files changed, 5 insertions(+), 36 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4ef8dce..1585e23 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -3,7 +3,7 @@ class ApplicationController < ActionController::API attr_reader :current_user helper_method :current_user - before_action :authenticate_user#, except: [:login, :logout] + before_action :authenticate_user, except: [:login] def resource_not_found not_found = "The resource you tried to access was not found" @@ -14,27 +14,15 @@ def invalid_request(message = error_msg, status = 400) render json: {errors: message}, status: status end - # def login - # user = authenticate_cookie - # @current_user = User.from_andela_auth(user) if user && !user['isGuest'] - # return unauthorized_token unless @current_user - # - # @token = TokenManager.generate_token(@current_user.id) - # end - - # def logout - # cookie = request.headers['HTTP_ANDELA_COOKIE'] - # return invalid_request("Cookie must be provided") unless cookie - # CookieHandler.logout_cookie(cookie) - # render json: {message: 'logged out'}, status: 200 - # end + def login + @current_user = authenticate_user + end private def authenticate_user strategy, token = request.headers['Authorization'].split auth = AndelaAuthV2.authenticate(token) - # during refactor these methods/checks should be moved out of here if strategy == 'Bearer' && auth.authenticated? auth_user = auth.current_user @current_user = User.find_or_create_by(email: auth_user['email']) @@ -44,30 +32,12 @@ def authenticate_user image: auth_user['picture'], active: (auth_user['status'] == 'active') ) + @current_user else unauthorized_token end end - # def authenticate_user - # (authenticate_token && authenticate_cookie) || unauthorized_token - # # authenticate_cookie || unauthorized_token - # end - - # def authenticate_token - # authenticate_with_http_token do |auth_token, _| - # user_id = TokenManager.authenticate(auth_token)['user'] - # @token = TokenManager.generate_token(user_id) if user_id - # end - # end - - # def authenticate_cookie - # cookie = request.headers['HTTP_ANDELA_COOKIE'] - # return unless cookie - # user = CookieHandler.validate_with_cookie(cookie) - # @current_user = User.find_by(email: user['email']) if user - # user - # end def unauthorized_token self.headers['WWW-Authenticate'] = 'Token realm="Application"' diff --git a/app/views/application/login.json.jbuilder b/app/views/application/login.json.jbuilder index 4250249..e250dfb 100644 --- a/app/views/application/login.json.jbuilder +++ b/app/views/application/login.json.jbuilder @@ -1,6 +1,5 @@ json.partial! "users/user", user: @current_user json.partial! 'tags/tag', tags: @current_user.tags -json.api_key @token # json.user { json.partial! 'users/user', user: @current_user } # json.api_key @token From 24979b9de99131b6bc643be3900dfb5248cdd221 Mon Sep 17 00:00:00 2001 From: Ologho Cyril Paul Date: Tue, 31 Jan 2017 16:21:11 +0100 Subject: [PATCH 03/12] refactor: - remove shared authentication endpoints and request helper for old auth system [#137599493] --- .../shared_authenticated_endpoint.rb | 26 ---------- .../shared/shared_authenticated_endpoint.rb | 48 ------------------- spec/support/request_authentication_helper.rb | 35 -------------- spec/support/valid_request_helper.rb | 17 ------- 4 files changed, 126 deletions(-) delete mode 100644 spec/requests/questions/shared_authenticated_endpoint.rb delete mode 100644 spec/requests/shared/shared_authenticated_endpoint.rb delete mode 100644 spec/support/request_authentication_helper.rb delete mode 100644 spec/support/valid_request_helper.rb diff --git a/spec/requests/questions/shared_authenticated_endpoint.rb b/spec/requests/questions/shared_authenticated_endpoint.rb deleted file mode 100644 index 145cf07..0000000 --- a/spec/requests/questions/shared_authenticated_endpoint.rb +++ /dev/null @@ -1,26 +0,0 @@ -RSpec.shared_examples "question authenticated endpoint" do |endpoint, verb, show| - before(:each) do - path = question_path_helper(endpoint, show) - send(verb, path, { format: :json }, token) - end - - context "with valid token" do - let(:token) { authorization_header } - - describe "response status" do - it { expect(response.status).not_to be 401 } - end - end - - context "with invalid token" do - let(:token) { authorization_header("") } - - describe "response status" do - it { expect(response.status).to be 401 } - end - - describe "response schema" do - it { expect(response).to match_response_schema("error/invalid_token") } - end - end -end diff --git a/spec/requests/shared/shared_authenticated_endpoint.rb b/spec/requests/shared/shared_authenticated_endpoint.rb deleted file mode 100644 index 0502e25..0000000 --- a/spec/requests/shared/shared_authenticated_endpoint.rb +++ /dev/null @@ -1,48 +0,0 @@ -RSpec.shared_examples "authenticated endpoint" do |endpoint, verb, include_answer| - before(:each) do - # 🙈 = Dummy.create_with_methods({body: "{\"email\": \"#{valid_user.email}\"}"}) - - # allow_any_instance_of(Faraday::Connection).to receive(:get).and_return(🙈) - - path = path_helper(endpoint, include_answer) - send(verb, path, { format: :json }, header) - end - - context "with valid token and cookie" do - let(:header) { authorization_token.merge(cookie_header) } - - describe "response status" do - it { expect(response.status).not_to be 401 } - end - end - - context "with invalid token and valid cookie" do - let(:header) { authorization_token("").merge(cookie_header) } - - describe "response status" do - it { expect(response.status).to be 401 } - end - - describe "response schema" do - it { expect(response).to match_response_schema("error/invalid_token") } - end - end - - context "with valid token and no cookie" do - let(:header) { authorization_token } - - describe "response status" do - it { expect(response.status).to be 401 } - end - - describe "response schema" do - it { expect(response).to match_response_schema("error/invalid_token") } - end - end - - context "with valid cookie and no token" do - let(:header) { cookie_header } - - it { expect(response.status).to be 401} - end -end diff --git a/spec/support/request_authentication_helper.rb b/spec/support/request_authentication_helper.rb deleted file mode 100644 index a435591..0000000 --- a/spec/support/request_authentication_helper.rb +++ /dev/null @@ -1,35 +0,0 @@ -module RequestAuthenticationHelper - extend ActiveSupport::Concern - included do - before(:each) do - 🙈 = Dummy.create_with_methods({body: "{\"email\": \"#{valid_user.email}\"}"}) - allow_any_instance_of(Faraday::Connection).to receive(:get).and_return(🙈) - end - end - - def valid_user - @valid_user ||= create(:user) - end - - def valid_user_token(user = nil) - (user || valid_user).refresh_token - end - - def authorization_header(token = valid_user_token) - authorization_token(token).merge(cookie_header) - end - - def authorization_token(token = valid_user_token) - { authorization: "Token token=#{token}" } - end - - def cookie_header - { HTTP_ANDELA_COOKIE: "valid cookie" } - end -end - - -RSpec.configure do |config| - config.include RequestAuthenticationHelper, type: :request - config.include RequestAuthenticationHelper, type: :controller -end diff --git a/spec/support/valid_request_helper.rb b/spec/support/valid_request_helper.rb deleted file mode 100644 index 57d7d38..0000000 --- a/spec/support/valid_request_helper.rb +++ /dev/null @@ -1,17 +0,0 @@ -module ValidRequest - def valid_user - @valid_user ||= create(:user) - end - - extend ActiveSupport::Concern - included do - before(:each, valid_request: true) do - request.headers['Authorization'] = "Token token=#{valid_user.refresh_token}" - request.headers['HTTP_ANDELA_COOKIE'] = "a.valid.cookie" - end - end -end - -RSpec.configure do |config| - config.include ValidRequest, type: :controller -end From 702809cf81c652694e0dccb243f942f0e2e7acd0 Mon Sep 17 00:00:00 2001 From: Ologho Cyril Paul Date: Tue, 31 Jan 2017 16:24:40 +0100 Subject: [PATCH 04/12] test(authentication) mock test - Add gem for webmock - add support for mocking andela auth [#137599493] --- Gemfile | 1 + Gemfile.lock | 11 +++++- spec/support/andela_auth_mock.rb | 65 ++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 spec/support/andela_auth_mock.rb diff --git a/Gemfile b/Gemfile index b37a952..263f5c1 100644 --- a/Gemfile +++ b/Gemfile @@ -58,6 +58,7 @@ group :test do gem "json-schema" gem "mock_redis" gem 'rspec-sidekiq' + gem 'webmock' end group :production, :staging do diff --git a/Gemfile.lock b/Gemfile.lock index be03fc4..7471479 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -57,6 +57,8 @@ GEM term-ansicolor (~> 1.3) thor (~> 0.19.1) tins (~> 1.6.0) + crack (0.4.3) + safe_yaml (~> 1.0.0) database_cleaner (1.5.1) diff-lcs (1.2.5) docile (1.1.5) @@ -110,6 +112,7 @@ GEM guard (~> 2.1) guard-compat (~> 1.1) rspec (>= 2.99.0, < 4.0) + hashdiff (0.3.2) hashie (3.4.3) http-cookie (1.0.2) domain_name (~> 0.5) @@ -236,6 +239,7 @@ GEM rspec-support (3.4.1) ruby-prof (0.15.9) ruby_dep (1.3.1) + safe_yaml (1.0.4) shellany (0.0.1) shoulda-matchers (3.0.1) activesupport (>= 4.0.0) @@ -280,6 +284,10 @@ GEM unf (0.1.4) unf_ext unf_ext (0.0.7.1) + webmock (2.3.2) + addressable (>= 2.3.6) + crack (>= 0.3.2) + hashdiff will_paginate (3.1.0) PLATFORMS @@ -328,10 +336,11 @@ DEPENDENCIES sinatra (>= 1.3.0) spring sqlite3 + webmock will_paginate RUBY VERSION ruby 2.3.1p112 BUNDLED WITH - 1.13.6 + 1.14.1 diff --git a/spec/support/andela_auth_mock.rb b/spec/support/andela_auth_mock.rb new file mode 100644 index 0000000..76c81d4 --- /dev/null +++ b/spec/support/andela_auth_mock.rb @@ -0,0 +1,65 @@ +module AndelaAuthMock + extend ActiveSupport::Concern + + require 'webmock/rspec' + WebMock.disable_net_connect!(allow_localhost: true) + + included do + before(:each) do + stub_authorized_user_auth + stub_unauthorized_user_auth + end + end + + def auth_server + Connection::FaradayConnection::BASE_ANDELA_URL + "/api/v1/users/me" + end + + def stub_authorized_user_auth + stub_request(:get, auth_server).with(headers: authorization_header). + to_return(body: Notifications::UserSerializer.new(valid_user, root: false).to_json) + end + + def stub_unauthorized_user_auth + stub_request(:get, auth_server).with(headers: unauthorized_token). + to_return(status: 401, body: {error: true}.to_json) + end + + def authorization_header + { Authorization: "Bearer samplebearertokencanbefoundhere" } + end + + def unauthorized_token + { Authorization: "Bearer whatever"} + end + + def valid_user + @valid_user ||= create(:user) + end + + module AuthControllerHelper + extend ActiveSupport::Concern + + RSpec.configure do |config| + config.include self#, type: :controller + end + + included do + before(:each, valid_request: true, type: :controller) do + request.headers["Authorization"] = "Bearer samplebearertokencanbefoundhere" + end + + before(:each, invalid_request: true, type: :controller) do + request.headers["Authorization"] = "Bearer whatever" + end + + end + + + end + + RSpec.configure do |config| + config.include self, type: :request + config.include self, type: :controller + end +end \ No newline at end of file From 31888a5f6620449e7264ba73cce2f7a7c3d778d5 Mon Sep 17 00:00:00 2001 From: Ologho Cyril Paul Date: Tue, 31 Jan 2017 16:30:32 +0100 Subject: [PATCH 05/12] test(authentication): - removes shared_authentication_endpoint dependecies from these files [#137599493] --- spec/controllers/votes_controller_spec.rb | 114 +++++++++--------- .../requests/answers/accepting_answer_spec.rb | 2 - .../requests/answers/answer_request_helper.rb | 1 - spec/requests/answers/creating_answer_spec.rb | 2 - .../answers/destroying_answer_spec.rb | 2 - spec/requests/answers/fetching_answer_spec.rb | 1 - spec/requests/answers/updating_answer_spec.rb | 2 - .../create_question_endpoint_spec.rb | 2 - .../delete_question_endpoint_spec.rb | 2 - .../questions/fetch_questions_index_spec.rb | 2 - .../questions/question_request_helper.rb | 1 - .../questions/search_questions_spec.rb | 2 - .../questions/show_question_endpoint_spec.rb | 2 - spec/requests/questions/top_questions_spec.rb | 2 - .../update_question_endpoint_spec.rb | 2 - 15 files changed, 57 insertions(+), 82 deletions(-) diff --git a/spec/controllers/votes_controller_spec.rb b/spec/controllers/votes_controller_spec.rb index 3ce3c7b..41a3b33 100644 --- a/spec/controllers/votes_controller_spec.rb +++ b/spec/controllers/votes_controller_spec.rb @@ -9,64 +9,64 @@ create_list(:answer, 5, user: user2) end - context "when user votes another user's resource" do - before do - request.headers['Authorization'] = "Token token=#{valid_user_token(user2)}" - end - - it "upvotes another user's resource" do - # post :upvote, {resource_name: 'questions', resource_id: 1} - # expect(response.body).to eq "{\"response\":1}" - # expect(response.status).to eq 200 - end - - it "downvotes another user's resource " do - # post :downvote, {resource_name: 'questions', resource_id: 1} - # expect(response.body).to eq "{\"response\":-1}" - # expect(response.status).to eq 200 - end - end + # context "when user votes another user's resource" do + # before do + # request.headers['Authorization'] = "Token token=#{valid_user_token(user2)}" + # end + # + # it "upvotes another user's resource" do + # # post :upvote, {resource_name: 'questions', resource_id: 1} + # # expect(response.body).to eq "{\"response\":1}" + # # expect(response.status).to eq 200 + # end + # + # it "downvotes another user's resource " do + # # post :downvote, {resource_name: 'questions', resource_id: 1} + # # expect(response.body).to eq "{\"response\":-1}" + # # expect(response.status).to eq 200 + # end + # end + # + # context "when user votes own resource" do + # before do + # request.headers['Authorization'] = "Token token=#{valid_user_token(user2)}" + # end + # + # it "doesnt downvote resource" do + # # post :downvote, {resource_name: 'answers', resource_id: 1} + # # expect(response.body).to include "You can't vote for your post" + # # expect(response.status).to eq 403 + # end + # + # it "doesnt upvote resource" do + # # post :upvote, {resource_name: 'answers', resource_id: 1} + # # expect(response.body).to include "You can't vote for your post" + # # expect(response.status).to eq 403 + # end + # end + # + # context "when user cannot vote on resource" do + # before do + # request.headers['Authorization'] = "Token token=#{valid_user_token(user3)}" + # end + # + # it "doesnt upvotes another user's resource" do + # # post :upvote, {resource_name: 'questions', resource_id: 1} + # # expect(response.body).to include "Not qualified to vote" + # # expect(response.status).to eq 403 + # end + # + # it "doesnt downvote another user's resource" do + # # post :downvote, {resource_name: 'answers', resource_id: 1} + # # expect(response.body).to include "Not qualified to vote" + # # expect(response.status).to eq 403 + # end + # end - context "when user votes own resource" do - before do - request.headers['Authorization'] = "Token token=#{valid_user_token(user2)}" - end - - it "doesnt downvote resource" do - # post :downvote, {resource_name: 'answers', resource_id: 1} - # expect(response.body).to include "You can't vote for your post" - # expect(response.status).to eq 403 - end - - it "doesnt upvote resource" do - # post :upvote, {resource_name: 'answers', resource_id: 1} - # expect(response.body).to include "You can't vote for your post" - # expect(response.status).to eq 403 - end - end - - context "when user cannot vote on resource" do - before do - request.headers['Authorization'] = "Token token=#{valid_user_token(user3)}" - end - - it "doesnt upvotes another user's resource" do - # post :upvote, {resource_name: 'questions', resource_id: 1} - # expect(response.body).to include "Not qualified to vote" - # expect(response.status).to eq 403 - end - - it "doesnt downvote another user's resource" do - # post :downvote, {resource_name: 'answers', resource_id: 1} - # expect(response.body).to include "Not qualified to vote" - # expect(response.status).to eq 403 - end - end - - context "when user tries to vote with invalid token" do - before do - request.headers['Authorization'] = "Token token=badtoken" - end + context "when user tries to vote with invalid token", invalid_request: true do + # before do + # request.headers['Authorization'] = "Token token=badtoken" + # end it "rejects invalid token for upvote" do post :upvote, {resource_name: 'questions', resource_id: 1} diff --git a/spec/requests/answers/accepting_answer_spec.rb b/spec/requests/answers/accepting_answer_spec.rb index 5db0757..6d25178 100644 --- a/spec/requests/answers/accepting_answer_spec.rb +++ b/spec/requests/answers/accepting_answer_spec.rb @@ -6,8 +6,6 @@ let(:answer) { create(:answer, question: question, user: user2) } let(:path) { accept_question_answer_path(question, answer) } - it_behaves_like "authenticated endpoint", :accept_question_answer_path, 'post', true - describe "validates that question belongs to user" do let(:question) { create(:question_with_answers, user: user2) } it "returns unauthorized_access if question doesn't belong to user" do diff --git a/spec/requests/answers/answer_request_helper.rb b/spec/requests/answers/answer_request_helper.rb index 148fa4e..b8bcea4 100644 --- a/spec/requests/answers/answer_request_helper.rb +++ b/spec/requests/answers/answer_request_helper.rb @@ -1,5 +1,4 @@ require "rails_helper" -require "requests/shared/shared_authenticated_endpoint" def path_helper(path, answer=false) question = FactoryGirl.create(:question_with_answers) diff --git a/spec/requests/answers/creating_answer_spec.rb b/spec/requests/answers/creating_answer_spec.rb index 4f3d9ce..9aff568 100644 --- a/spec/requests/answers/creating_answer_spec.rb +++ b/spec/requests/answers/creating_answer_spec.rb @@ -5,8 +5,6 @@ let(:user) { create(:active_user) } let(:path) { question_answers_path(question) } - it_behaves_like "authenticated endpoint", :question_answers_path, 'post' - describe "POST /questions/:question_id/answers"do describe "validates content" do it "doesn't save if content is empty" do diff --git a/spec/requests/answers/destroying_answer_spec.rb b/spec/requests/answers/destroying_answer_spec.rb index c34acc8..50d4e09 100644 --- a/spec/requests/answers/destroying_answer_spec.rb +++ b/spec/requests/answers/destroying_answer_spec.rb @@ -4,8 +4,6 @@ let(:answer) { create(:answer, user: valid_user) } let(:path) { question_answer_path(answer.question, answer) } - it_behaves_like "authenticated endpoint", :question_answer_path, 'delete', true - describe "DELETE /questions/:question_id/answers/:id" do describe "invalid answer id" do it "returns 404 if answer is not found" do diff --git a/spec/requests/answers/fetching_answer_spec.rb b/spec/requests/answers/fetching_answer_spec.rb index 1b9864b..9951d85 100644 --- a/spec/requests/answers/fetching_answer_spec.rb +++ b/spec/requests/answers/fetching_answer_spec.rb @@ -1,7 +1,6 @@ require_relative "answer_request_helper" RSpec.describe "Fetching an answer", type: :request do - it_behaves_like "authenticated endpoint", :question_answers_path, 'get' describe "GET /questions/:question_id/answers" do let(:question) { create(:question_with_answers, answers_count: 5) } diff --git a/spec/requests/answers/updating_answer_spec.rb b/spec/requests/answers/updating_answer_spec.rb index df97f47..1195849 100644 --- a/spec/requests/answers/updating_answer_spec.rb +++ b/spec/requests/answers/updating_answer_spec.rb @@ -4,8 +4,6 @@ let(:answer) { create(:answer, user: valid_user) } let(:path) { question_answer_path(answer.question, answer) } - it_behaves_like "authenticated endpoint", :question_answer_path, 'patch', true - describe "PATCH /questions/:question_id/answers/:id" do context "validates content" do let(:new_answer){ attributes_for(:answer).merge(format: :json) } diff --git a/spec/requests/questions/create_question_endpoint_spec.rb b/spec/requests/questions/create_question_endpoint_spec.rb index 4f59f70..014a3d4 100644 --- a/spec/requests/questions/create_question_endpoint_spec.rb +++ b/spec/requests/questions/create_question_endpoint_spec.rb @@ -5,8 +5,6 @@ describe "POST /questions" do let(:path) { questions_path } - it_behaves_like "question authenticated endpoint", :questions_path, :post - context "with valid athorization header" do before(:each) { post path, question_params, authorization_header } diff --git a/spec/requests/questions/delete_question_endpoint_spec.rb b/spec/requests/questions/delete_question_endpoint_spec.rb index f7f2f9e..20b6cfa 100644 --- a/spec/requests/questions/delete_question_endpoint_spec.rb +++ b/spec/requests/questions/delete_question_endpoint_spec.rb @@ -6,8 +6,6 @@ let(:question) { create(:question, user: valid_user) } let(:path) { question_path(question) } - it_behaves_like "question authenticated endpoint", :questions_path, :delete, true - context "with valid athorization header" do before(:each) { delete path, { format: :json }, authorization_header } diff --git a/spec/requests/questions/fetch_questions_index_spec.rb b/spec/requests/questions/fetch_questions_index_spec.rb index 949e26f..a7d1b8b 100644 --- a/spec/requests/questions/fetch_questions_index_spec.rb +++ b/spec/requests/questions/fetch_questions_index_spec.rb @@ -8,8 +8,6 @@ # NOTE the index is used here to fetch the route to be tested let(:path) { [questions_path, all_questions_path ][index] } - it_behaves_like "question authenticated endpoint", :questions_path, :get - context "with valid authorization header" do context "when there are fewer than 25 questions" do let(:question_count) { 10 } diff --git a/spec/requests/questions/question_request_helper.rb b/spec/requests/questions/question_request_helper.rb index 764d9c6..fa510a4 100644 --- a/spec/requests/questions/question_request_helper.rb +++ b/spec/requests/questions/question_request_helper.rb @@ -1,4 +1,3 @@ -require_relative "shared_authenticated_endpoint" def question_path_helper(path, show=false) question = create(:question) diff --git a/spec/requests/questions/search_questions_spec.rb b/spec/requests/questions/search_questions_spec.rb index ff9d7d9..80de73a 100644 --- a/spec/requests/questions/search_questions_spec.rb +++ b/spec/requests/questions/search_questions_spec.rb @@ -11,8 +11,6 @@ let(:path) { search_questions_path } - it_behaves_like "question authenticated endpoint", :search_questions_path, :get - context "with valid authorization header" do before(:each) do create_list(:question, 15) diff --git a/spec/requests/questions/show_question_endpoint_spec.rb b/spec/requests/questions/show_question_endpoint_spec.rb index 1269e93..c3bb17e 100644 --- a/spec/requests/questions/show_question_endpoint_spec.rb +++ b/spec/requests/questions/show_question_endpoint_spec.rb @@ -6,8 +6,6 @@ let(:question) { create(:question) } let(:path) { question_path(question) } - it_behaves_like "question authenticated endpoint", :question_path, :get, true - context "with valid authorization header" do before(:each) { get path, { format: :json }, authorization_header } diff --git a/spec/requests/questions/top_questions_spec.rb b/spec/requests/questions/top_questions_spec.rb index 44a4143..ab54096 100644 --- a/spec/requests/questions/top_questions_spec.rb +++ b/spec/requests/questions/top_questions_spec.rb @@ -5,8 +5,6 @@ describe "GET /top_questions" do let(:path) { top_questions_path } - it_behaves_like "question authenticated endpoint", :top_questions_path, :get - context "with valid authorization header" do before(:each) do create_list(:question, 15) do |question| diff --git a/spec/requests/questions/update_question_endpoint_spec.rb b/spec/requests/questions/update_question_endpoint_spec.rb index a65d11f..9302b9a 100644 --- a/spec/requests/questions/update_question_endpoint_spec.rb +++ b/spec/requests/questions/update_question_endpoint_spec.rb @@ -6,8 +6,6 @@ let(:question) { create(:question, user: valid_user) } let(:path) { question_path(question) } - it_behaves_like "question authenticated endpoint", :questions_path, :put, true - context "with valid athorization header" do before(:each) { put path, question_params, authorization_header } From e233cb0f3cded2c1ca9aae382b3d8da191160225 Mon Sep 17 00:00:00 2001 From: Ologho Cyril Paul Date: Tue, 31 Jan 2017 16:59:24 +0100 Subject: [PATCH 06/12] test(authentication): - removes commented out spec in votes_controller.spec [#137599493] --- spec/controllers/votes_controller_spec.rb | 54 ----------------------- 1 file changed, 54 deletions(-) diff --git a/spec/controllers/votes_controller_spec.rb b/spec/controllers/votes_controller_spec.rb index 41a3b33..87182b3 100644 --- a/spec/controllers/votes_controller_spec.rb +++ b/spec/controllers/votes_controller_spec.rb @@ -9,60 +9,6 @@ create_list(:answer, 5, user: user2) end - # context "when user votes another user's resource" do - # before do - # request.headers['Authorization'] = "Token token=#{valid_user_token(user2)}" - # end - # - # it "upvotes another user's resource" do - # # post :upvote, {resource_name: 'questions', resource_id: 1} - # # expect(response.body).to eq "{\"response\":1}" - # # expect(response.status).to eq 200 - # end - # - # it "downvotes another user's resource " do - # # post :downvote, {resource_name: 'questions', resource_id: 1} - # # expect(response.body).to eq "{\"response\":-1}" - # # expect(response.status).to eq 200 - # end - # end - # - # context "when user votes own resource" do - # before do - # request.headers['Authorization'] = "Token token=#{valid_user_token(user2)}" - # end - # - # it "doesnt downvote resource" do - # # post :downvote, {resource_name: 'answers', resource_id: 1} - # # expect(response.body).to include "You can't vote for your post" - # # expect(response.status).to eq 403 - # end - # - # it "doesnt upvote resource" do - # # post :upvote, {resource_name: 'answers', resource_id: 1} - # # expect(response.body).to include "You can't vote for your post" - # # expect(response.status).to eq 403 - # end - # end - # - # context "when user cannot vote on resource" do - # before do - # request.headers['Authorization'] = "Token token=#{valid_user_token(user3)}" - # end - # - # it "doesnt upvotes another user's resource" do - # # post :upvote, {resource_name: 'questions', resource_id: 1} - # # expect(response.body).to include "Not qualified to vote" - # # expect(response.status).to eq 403 - # end - # - # it "doesnt downvote another user's resource" do - # # post :downvote, {resource_name: 'answers', resource_id: 1} - # # expect(response.body).to include "Not qualified to vote" - # # expect(response.status).to eq 403 - # end - # end - context "when user tries to vote with invalid token", invalid_request: true do # before do # request.headers['Authorization'] = "Token token=badtoken" From af6be9aae41dfc0241b18481fb34277375a0d93e Mon Sep 17 00:00:00 2001 From: Ologho Cyril Paul Date: Tue, 31 Jan 2017 17:22:38 +0100 Subject: [PATCH 07/12] refactor(authentication): application_controller - refactor application_controller [#137599493] --- app/controllers/application_controller.rb | 33 ++++++++++++++--------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1585e23..91d5380 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -20,19 +20,26 @@ def login private def authenticate_user - strategy, token = request.headers['Authorization'].split - auth = AndelaAuthV2.authenticate(token) - - if strategy == 'Bearer' && auth.authenticated? - auth_user = auth.current_user - @current_user = User.find_or_create_by(email: auth_user['email']) - # we always want to ensure these attrs are in sync with the auth system - @current_user.update_attributes( - name: auth_user['name'], - image: auth_user['picture'], - active: (auth_user['status'] == 'active') - ) - @current_user + + if request.headers['Authorization'] + strategy, token = request.headers['Authorization'].split + auth = AndelaAuthV2.authenticate(token) + + if strategy == 'Bearer' && auth.authenticated? + auth_user = auth.current_user + @current_user = User.find_or_create_by(email: auth_user['email']) + # we always want to ensure these attrs are in sync with the auth system + @current_user.update_attributes( + name: auth_user['name'], + image: auth_user['picture'], + active: (auth_user['status'] == 'active') + ) + @current_user + else + unauthorized_token + + end + else unauthorized_token end From 0737513ac53fdf9375571f40a08ba4efe896d419 Mon Sep 17 00:00:00 2001 From: Ologho Cyril Paul Date: Tue, 31 Jan 2017 17:45:47 +0100 Subject: [PATCH 08/12] fix(faraday_connection): - fix environment variable for faraday_connection.rb [#137599493] --- app/authenticators/connection/faraday_connection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/authenticators/connection/faraday_connection.rb b/app/authenticators/connection/faraday_connection.rb index 3aee864..cc7af2f 100644 --- a/app/authenticators/connection/faraday_connection.rb +++ b/app/authenticators/connection/faraday_connection.rb @@ -1,5 +1,5 @@ class Connection::FaradayConnection - BASE_ANDELA_URL = "http://api-staging.andela.com" # use env variable to set the url + BASE_ANDELA_URL = ENV['BASE_ANDELA_URL'] # use env variable to set the url def self.connection(token) options = { From 1a83aec01f3a7964111fc4f0086c686398db9a85 Mon Sep 17 00:00:00 2001 From: Ologho Cyril Paul Date: Wed, 1 Feb 2017 10:00:52 +0100 Subject: [PATCH 09/12] refactor(authentication): - remove login endpoint from controllers and the login route [Delivers #137599493] --- .env.sample | 30 +++++++++++++++++++++++ app/controllers/application_controller.rb | 6 +---- app/controllers/users_controller.rb | 2 +- config/routes.rb | 1 - spec/controllers/votes_controller_spec.rb | 4 --- 5 files changed, 32 insertions(+), 11 deletions(-) create mode 100644 .env.sample diff --git a/.env.sample b/.env.sample new file mode 100644 index 0000000..2614668 --- /dev/null +++ b/.env.sample @@ -0,0 +1,30 @@ + +GOOGLE_CLIENT_ID: '' +GOOGLE_CLIENT_SECRET: '' +BUGSNAG_API_KEY: +BASE_ANDELA_URL: '' + +development: + SLACK_CLIENT_ID: '' + SLACK_CLIENT_SECRET: + ELASTIC_SEARCH_HOST_URL: + REDISTOGO_URL: + ZI_NOTIFICATION_URL: + BASE_URL: + ANDELA_AUTH_URL: + NEW_RELIC_LICENSE_KEY: + +staging: + ZI_NOTIFICATION_URL: + BASE_URL: + ANDELA_AUTH_URL: + +production: + ZI_NOTIFICATION_URL: + BASE_URL: + ANDELA_AUTH_URL: + +test: + ZI_NOTIFICATION_URL: + BASE_URL: + ANDELA_AUTH_URL: diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 91d5380..d216ff8 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -3,7 +3,7 @@ class ApplicationController < ActionController::API attr_reader :current_user helper_method :current_user - before_action :authenticate_user, except: [:login] + before_action :authenticate_user def resource_not_found not_found = "The resource you tried to access was not found" @@ -14,10 +14,6 @@ def invalid_request(message = error_msg, status = 400) render json: {errors: message}, status: status end - def login - @current_user = authenticate_user - end - private def authenticate_user diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 724d5e3..cc80aaf 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -2,7 +2,7 @@ class UsersController < ApplicationController before_action :set_user, only: [:update, :questions, :tags] before_action :set_user_with_activities, only: [:activities] before_action :set_user_with_associations_and_statistics, only: [:show] - skip_before_action :authenticate_user, only: [:login, :authenticate] + skip_before_action :authenticate_user, only: [:authenticate] def index users = User.paginate(page: params[:page]) diff --git a/config/routes.rb b/config/routes.rb index 0f3a638..27fab1a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -50,7 +50,6 @@ end end - post "login" => "application#login" get "logout" => "application#logout" mount Sidekiq::Web => '/sidekiq' diff --git a/spec/controllers/votes_controller_spec.rb b/spec/controllers/votes_controller_spec.rb index 87182b3..3549e06 100644 --- a/spec/controllers/votes_controller_spec.rb +++ b/spec/controllers/votes_controller_spec.rb @@ -10,10 +10,6 @@ end context "when user tries to vote with invalid token", invalid_request: true do - # before do - # request.headers['Authorization'] = "Token token=badtoken" - # end - it "rejects invalid token for upvote" do post :upvote, {resource_name: 'questions', resource_id: 1} expect(response.body).to include "invalid token" From 23812a87ec279fccbaa3f0c39b753013673dd5d4 Mon Sep 17 00:00:00 2001 From: Ologho Cyril Paul Date: Wed, 1 Feb 2017 13:07:28 +0100 Subject: [PATCH 10/12] refactor(authentication): authenticator - refactors application_controller to abstract core authentication workflow - creates authenticator to handle core workflow [#137599493] --- app/authenticators/authenticator.rb | 24 +++++++++++++ app/controllers/application_controller.rb | 41 ++++++++++------------- 2 files changed, 41 insertions(+), 24 deletions(-) create mode 100644 app/authenticators/authenticator.rb diff --git a/app/authenticators/authenticator.rb b/app/authenticators/authenticator.rb new file mode 100644 index 0000000..22d88c6 --- /dev/null +++ b/app/authenticators/authenticator.rb @@ -0,0 +1,24 @@ +require_relative 'andela_auth_v2' + +class Authenticator + attr_reader :request + attr_accessor :user + + def initialize(request) + @request = request + end + + def authenticated? + if request.headers['Authorization'] + strategy, token = request.headers['Authorization'].split + auth = AndelaAuthV2.authenticate(token) + if strategy == 'Bearer' && auth.authenticated? + return @user = auth.current_user + else + return false + end + else + return false + end + end +end \ No newline at end of file diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index d216ff8..64b614a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -16,32 +16,25 @@ def invalid_request(message = error_msg, status = 400) private def authenticate_user - - if request.headers['Authorization'] - strategy, token = request.headers['Authorization'].split - auth = AndelaAuthV2.authenticate(token) - - if strategy == 'Bearer' && auth.authenticated? - auth_user = auth.current_user - @current_user = User.find_or_create_by(email: auth_user['email']) - # we always want to ensure these attrs are in sync with the auth system - @current_user.update_attributes( - name: auth_user['name'], - image: auth_user['picture'], - active: (auth_user['status'] == 'active') - ) - @current_user - else - unauthorized_token - - end - - else - unauthorized_token - end - + auth = Authenticator.new(request) + if auth.authenticated? + create_or_update_user(auth.user) + else + unauthorized_token + end end + def create_or_update_user(user) + @current_user = User.find_or_create_by(email: user['email']) + # we always want to ensure these attrs are in sync with the auth system + @current_user.update_attributes( + name: user['name'], + image: user['picture'], + active: (user['status'] == 'active') + ) + @current_user + end + def unauthorized_token self.headers['WWW-Authenticate'] = 'Token realm="Application"' render json: {errors: "Request was made with invalid token"}, status: 401 From cb463961892e57799ba95669a6489cd63a4295be Mon Sep 17 00:00:00 2001 From: Ologho Cyril Paul Date: Wed, 1 Feb 2017 13:39:10 +0100 Subject: [PATCH 11/12] test(authentication) - remove comment from andela_auth_mock.rb [#137599493] --- spec/support/andela_auth_mock.rb | 128 +++++++++++++++---------------- 1 file changed, 64 insertions(+), 64 deletions(-) diff --git a/spec/support/andela_auth_mock.rb b/spec/support/andela_auth_mock.rb index 76c81d4..bd30483 100644 --- a/spec/support/andela_auth_mock.rb +++ b/spec/support/andela_auth_mock.rb @@ -1,65 +1,65 @@ -module AndelaAuthMock - extend ActiveSupport::Concern - - require 'webmock/rspec' - WebMock.disable_net_connect!(allow_localhost: true) - - included do - before(:each) do - stub_authorized_user_auth - stub_unauthorized_user_auth - end - end - - def auth_server - Connection::FaradayConnection::BASE_ANDELA_URL + "/api/v1/users/me" - end - - def stub_authorized_user_auth - stub_request(:get, auth_server).with(headers: authorization_header). - to_return(body: Notifications::UserSerializer.new(valid_user, root: false).to_json) - end - - def stub_unauthorized_user_auth - stub_request(:get, auth_server).with(headers: unauthorized_token). - to_return(status: 401, body: {error: true}.to_json) - end - - def authorization_header - { Authorization: "Bearer samplebearertokencanbefoundhere" } - end - - def unauthorized_token - { Authorization: "Bearer whatever"} - end - - def valid_user - @valid_user ||= create(:user) - end - - module AuthControllerHelper - extend ActiveSupport::Concern - - RSpec.configure do |config| - config.include self#, type: :controller - end - - included do - before(:each, valid_request: true, type: :controller) do - request.headers["Authorization"] = "Bearer samplebearertokencanbefoundhere" - end - - before(:each, invalid_request: true, type: :controller) do - request.headers["Authorization"] = "Bearer whatever" - end - - end - - - end - - RSpec.configure do |config| - config.include self, type: :request - config.include self, type: :controller - end +module AndelaAuthMock + extend ActiveSupport::Concern + + require 'webmock/rspec' + WebMock.disable_net_connect!(allow_localhost: true) + + included do + before(:each) do + stub_authorized_user_auth + stub_unauthorized_user_auth + end + end + + def auth_server + Connection::FaradayConnection::BASE_ANDELA_URL + "/api/v1/users/me" + end + + def stub_authorized_user_auth + stub_request(:get, auth_server).with(headers: authorization_header). + to_return(body: Notifications::UserSerializer.new(valid_user, root: false).to_json) + end + + def stub_unauthorized_user_auth + stub_request(:get, auth_server).with(headers: unauthorized_token). + to_return(status: 401, body: {error: true}.to_json) + end + + def authorization_header + { Authorization: "Bearer samplebearertokencanbefoundhere" } + end + + def unauthorized_token + { Authorization: "Bearer whatever"} + end + + def valid_user + @valid_user ||= create(:user) + end + + module AuthControllerHelper + extend ActiveSupport::Concern + + RSpec.configure do |config| + config.include self + end + + included do + before(:each, valid_request: true, type: :controller) do + request.headers["Authorization"] = "Bearer samplebearertokencanbefoundhere" + end + + before(:each, invalid_request: true, type: :controller) do + request.headers["Authorization"] = "Bearer whatever" + end + + end + + + end + + RSpec.configure do |config| + config.include self, type: :request + config.include self, type: :controller + end end \ No newline at end of file From 4f1518fc19f029dd406a82ff0ea849a5ef295238 Mon Sep 17 00:00:00 2001 From: Ologho Cyril Paul Date: Wed, 1 Feb 2017 13:45:54 +0100 Subject: [PATCH 12/12] refactor(authentication) - move resource_not_found method to private namespace - refactor users_controller [Deliver #137599493] --- app/controllers/application_controller.rb | 10 +++++----- app/controllers/users_controller.rb | 1 - 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 64b614a..2218f18 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -5,11 +5,6 @@ class ApplicationController < ActionController::API helper_method :current_user before_action :authenticate_user - def resource_not_found - not_found = "The resource you tried to access was not found" - render json: {errors: not_found}, status: 404 - end - def invalid_request(message = error_msg, status = 400) render json: {errors: message}, status: status end @@ -40,6 +35,11 @@ def unauthorized_token render json: {errors: "Request was made with invalid token"}, status: 401 end + def resource_not_found + not_found = "The resource you tried to access was not found" + render json: {errors: not_found}, status: 404 + end + def error_msg "The operation could not be performed."\ " Please check your request or try again later" diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index cc80aaf..007c76e 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -2,7 +2,6 @@ class UsersController < ApplicationController before_action :set_user, only: [:update, :questions, :tags] before_action :set_user_with_activities, only: [:activities] before_action :set_user_with_associations_and_statistics, only: [:show] - skip_before_action :authenticate_user, only: [:authenticate] def index users = User.paginate(page: params[:page])