diff --git a/app/services/stormpath/rails/account_from_access_token.rb b/app/services/stormpath/rails/account_from_access_token.rb index 9a3514e..617f84f 100644 --- a/app/services/stormpath/rails/account_from_access_token.rb +++ b/app/services/stormpath/rails/account_from_access_token.rb @@ -8,22 +8,22 @@ class AccountFromAccessToken DifferentIssuerError = Class.new(ArgumentError) def initialize(access_token) - raise(NoAccessToken) if access_token.nil? + raise(NoAccessToken) if access_token.blank? @access_token = access_token end def account - @account ||= resolution_class.new(access_token).account + @account ||= resolution_instance.verify(access_token).account end private - def resolution_class + def resolution_instance case Stormpath::Rails.config.web.oauth2.password.validation_strategy.to_sym when :local - LocalAccountResolution + Stormpath::Oauth::VerifyAccessToken.new(Client.application, local: true) when :stormpath - StormpathAccountResolution + Stormpath::Oauth::VerifyAccessToken.new(Client.application) else raise ArgumentError, 'Invalid validation strategy' end diff --git a/app/services/stormpath/rails/account_from_access_token/local_account_resolution.rb b/app/services/stormpath/rails/account_from_access_token/local_account_resolution.rb deleted file mode 100644 index e90a502..0000000 --- a/app/services/stormpath/rails/account_from_access_token/local_account_resolution.rb +++ /dev/null @@ -1,48 +0,0 @@ -module Stormpath - module Rails - class AccountFromAccessToken - class LocalAccountResolution - attr_reader :access_token - - def initialize(access_token) - @access_token = access_token - @application = Client.application - validate_jwt - end - - def account - Stormpath::Rails::Client.client.accounts.get(account_href) - end - - private - - def account_href - jwt_data.first['sub'] - end - - def jwt_data - begin - @jwt_data ||= JWT.decode(access_token, ENV['STORMPATH_API_KEY_SECRET']) - rescue JWT::ExpiredSignature - raise Stormpath::Oauth::Error, :jwt_expired - end - end - - def validate_jwt - validate_jwt_is_an_access_token - validate_jwt_has_a_valid_issuer - end - - def validate_jwt_has_a_valid_issuer - return if jwt_data.first['iss'] == Stormpath::Rails::Client.application.href - raise DifferentIssuerError - end - - def validate_jwt_is_an_access_token - return if jwt_data.second['stt'] == 'access' - raise AuthenticationWithRefreshTokenAttemptError - end - end - end - end -end diff --git a/app/services/stormpath/rails/account_from_access_token/stormpath_account_resolution.rb b/app/services/stormpath/rails/account_from_access_token/stormpath_account_resolution.rb deleted file mode 100644 index f7d4a3c..0000000 --- a/app/services/stormpath/rails/account_from_access_token/stormpath_account_resolution.rb +++ /dev/null @@ -1,27 +0,0 @@ -module Stormpath - module Rails - class AccountFromAccessToken - class StormpathAccountResolution - attr_reader :access_token, :application - - def initialize(access_token) - @access_token = access_token - @application = Client.application - validate_jwt_is_access_token - end - - def account - Stormpath::Oauth::VerifyAccessToken.new(application).verify(access_token).account - end - - def validate_jwt_is_access_token - raise AuthenticationWithRefreshTokenAttemptError if jwt_data.second['stt'] != 'access' - end - - def jwt_data - @jwt_data ||= JWT.decode(access_token, ENV['STORMPATH_API_KEY_SECRET']) - end - end - end - end -end diff --git a/app/services/stormpath/rails/controller_authentication.rb b/app/services/stormpath/rails/controller_authentication.rb index f4bd39c..cd1e543 100644 --- a/app/services/stormpath/rails/controller_authentication.rb +++ b/app/services/stormpath/rails/controller_authentication.rb @@ -18,9 +18,16 @@ def authenticate! if any_auth_cookie_present? FromCookies.new(cookies).authenticate! elsif bearer_authorization_header? - FromBearerAuth.new(authorization_header).authenticate! + Stormpath::Authentication::HttpBearerAuthentication.new( + Stormpath::Rails::Client.application, + authorization_header, + local: validation_strategy + ).authenticate!.account elsif basic_authorization_header? - FromBasicAuth.new(authorization_header).authenticate! + Stormpath::Authentication::HttpBasicAuthentication.new( + Stormpath::Rails::Client.application, + authorization_header + ).authenticate!.account else raise UnauthenticatedRequest end @@ -39,6 +46,14 @@ def any_auth_cookie_present? def basic_authorization_header? authorization_header =~ BASIC_PATTERN end + + def validation_strategy + if Stormpath::Rails.config.web.oauth2.password.validation_strategy == 'stormpath' + true + else + false + end + end end end end diff --git a/app/services/stormpath/rails/controller_authentication/from_basic_auth.rb b/app/services/stormpath/rails/controller_authentication/from_basic_auth.rb deleted file mode 100644 index c4d939d..0000000 --- a/app/services/stormpath/rails/controller_authentication/from_basic_auth.rb +++ /dev/null @@ -1,45 +0,0 @@ -module Stormpath - module Rails - class ControllerAuthentication - class FromBasicAuth - attr_reader :authorization_header - - def initialize(authorization_header) - @authorization_header = authorization_header - end - - def authenticate! - raise UnauthenticatedRequest if fetched_api_key.nil? - raise UnauthenticatedRequest if fetched_api_key.secret != api_key_secret - fetched_api_key.account - end - - private - - def fetched_api_key - @fetched_api_key ||= Client.application.api_keys.search(id: api_key_id).first - end - - def api_key_id - decoded_authorization_header.first - end - - def api_key_secret - decoded_authorization_header.last - end - - def decoded_authorization_header - @decoded_authorization_header ||= begin - api_key_and_secret = Base64.decode64(basic_authorization_header).split(':') - raise UnauthenticatedRequest if api_key_and_secret.count != 2 - api_key_and_secret - end - end - - def basic_authorization_header - authorization_header.gsub(BASIC_PATTERN, '') - end - end - end - end -end diff --git a/app/services/stormpath/rails/controller_authentication/from_bearer_auth.rb b/app/services/stormpath/rails/controller_authentication/from_bearer_auth.rb deleted file mode 100644 index a93005c..0000000 --- a/app/services/stormpath/rails/controller_authentication/from_bearer_auth.rb +++ /dev/null @@ -1,34 +0,0 @@ -module Stormpath - module Rails - class ControllerAuthentication - class FromBearerAuth - attr_reader :authorization_header - - RESCUE_CLASSES = [ - Stormpath::Oauth::Error, - JWT::DecodeError, - AccountFromAccessToken::AuthenticationWithRefreshTokenAttemptError, - AccountFromAccessToken::DifferentIssuerError - ].freeze - - def initialize(authorization_header) - @authorization_header = authorization_header - end - - def authenticate! - begin - AccountFromAccessToken.new(bearer_access_token).account - rescue *RESCUE_CLASSES - raise UnauthenticatedRequest - end - end - - private - - def bearer_access_token - authorization_header.gsub(BEARER_PATTERN, '') - end - end - end - end -end diff --git a/lib/stormpath/rails/controller.rb b/lib/stormpath/rails/controller.rb index 2fe56b4..2a14177 100644 --- a/lib/stormpath/rails/controller.rb +++ b/lib/stormpath/rails/controller.rb @@ -15,7 +15,7 @@ module Controller def current_account @current_account ||= begin ControllerAuthentication.new(cookies, request.headers['Authorization']).authenticate! - rescue ControllerAuthentication::UnauthenticatedRequest + rescue ControllerAuthentication::UnauthenticatedRequest, Stormpath::Error, JWT::DecodeError nil end end diff --git a/spec/factories.rb b/spec/factories.rb index 0064b6e..a78c46a 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -4,12 +4,12 @@ password 'Password1337' given_name { Faker::Name.first_name } surname { Faker::Name.last_name } - username { Faker::Internet.user_name } + username { "#{Faker::Internet.user_name}_#{Faker::Internet.user_name}" } phone_number { Faker::PhoneNumber.cell_phone } end factory :account_without_username, class: Stormpath::Resource::Account do - sequence(:email) { |n| "dev#{n}@example.com" } + sequence(:email) { |n| "dev#{n}@testmail.stormpath.com" } password 'Password1337' given_name { Faker::Name.first_name } surname { Faker::Name.last_name } @@ -18,4 +18,14 @@ factory :unverified_account, parent: :account do status 'UNVERIFIED' end + + factory :directory, class: Stormpath::Resource::Directory do + sequence(:name) { |n| "rails-#{n}-#{Faker::Lorem.word}-directory" } + description 'rails test directory' + end + + factory :application, class: Stormpath::Resource::Application do + sequence(:name) { |n| "rails-#{n}-#{Faker::Lorem.word}-application" } + description 'rails test application' + end end diff --git a/spec/requests/profile/get_spec.rb b/spec/requests/profile/get_spec.rb index 090a44c..893d6e0 100644 --- a/spec/requests/profile/get_spec.rb +++ b/spec/requests/profile/get_spec.rb @@ -6,7 +6,6 @@ def response_body end let(:account) { Stormpath::Rails::Client.application.accounts.create(account_attrs) } - let(:account_attrs) { FactoryGirl.attributes_for(:account) } after { account.delete } diff --git a/spec/services/controller_authenticaton_spec.rb b/spec/services/controller_authenticaton_spec.rb index c5eeb4d..abb0160 100644 --- a/spec/services/controller_authenticaton_spec.rb +++ b/spec/services/controller_authenticaton_spec.rb @@ -415,10 +415,10 @@ ActionDispatch::Request.new('HTTP_AUTHORIZATION' => "Bearer #{expired_token}") end - it 'raises an UnauthenticatedRequest error' do + it 'raises an JWT Verification error' do expect do controller_authenticator.authenticate! - end.to raise_error(Stormpath::Rails::ControllerAuthentication::UnauthenticatedRequest) + end.to raise_error(JWT::VerificationError, 'Signature verification raised') end describe 'with stormpath validation strategy' do @@ -428,10 +428,10 @@ ).to receive(:validation_strategy).and_return('stormpath') end - it 'raises an UnauthenticatedRequest error' do + it 'raises an JWT Verification error' do expect do controller_authenticator.authenticate! - end.to raise_error(Stormpath::Rails::ControllerAuthentication::UnauthenticatedRequest) + end.to raise_error(JWT::VerificationError, 'Signature verification raised') end end end @@ -441,10 +441,10 @@ ActionDispatch::Request.new('HTTP_AUTHORIZATION' => "Bearer INVALID-TOKEN") end - it 'raises an UnauthenticatedRequest error' do + it 'raises an JWT Decode error' do expect do controller_authenticator.authenticate! - end.to raise_error(Stormpath::Rails::ControllerAuthentication::UnauthenticatedRequest) + end.to raise_error(JWT::DecodeError, 'Not enough or too many segments') end describe 'with stormpath validation strategy' do @@ -454,10 +454,10 @@ ).to receive(:validation_strategy).and_return('stormpath') end - it 'raises an UnauthenticatedRequest error' do + it 'raises an JWT Decode error' do expect do controller_authenticator.authenticate! - end.to raise_error(Stormpath::Rails::ControllerAuthentication::UnauthenticatedRequest) + end.to raise_error(JWT::DecodeError, 'Not enough or too many segments') end end end @@ -467,10 +467,10 @@ ActionDispatch::Request.new('HTTP_AUTHORIZATION' => "Bearer ") end - it 'raises an UnauthenticatedRequest error' do + it 'raises an JWT Decode error' do expect do controller_authenticator.authenticate! - end.to raise_error(Stormpath::Rails::ControllerAuthentication::UnauthenticatedRequest) + end.to raise_error(JWT::DecodeError, 'Not enough or too many segments') end describe 'with stormpath validation strategy' do @@ -480,10 +480,10 @@ ).to receive(:validation_strategy).and_return('stormpath') end - it 'raises an UnauthenticatedRequest error' do + it 'raises an JWT Decode error' do expect do controller_authenticator.authenticate! - end.to raise_error(Stormpath::Rails::ControllerAuthentication::UnauthenticatedRequest) + end.to raise_error(JWT::DecodeError, 'Not enough or too many segments') end end end @@ -508,50 +508,50 @@ describe 'with only api key and no secret' do let(:credentials) { Base64.encode64(api_key.id) } - it 'raises an UnauthenticatedRequest error' do + it 'raises a Stormpath error' do expect do controller_authenticator.authenticate! - end.to raise_error(Stormpath::Rails::ControllerAuthentication::UnauthenticatedRequest) + end.to raise_error(Stormpath::Error) end end describe 'with non-existing api key and secret' do let(:credentials) { Base64.encode64("dahgf3q4234fsd:bvcbfgt54332") } - it 'raises an UnauthenticatedRequest error' do + it 'raises an Stormpath error' do expect do controller_authenticator.authenticate! - end.to raise_error(Stormpath::Rails::ControllerAuthentication::UnauthenticatedRequest) + end.to raise_error(Stormpath::Error) end end describe 'with api key and wrong secret' do let(:credentials) { Base64.encode64("#{api_key.id}:2aAbsa3TDFDF") } - it 'raises an UnauthenticatedRequest error' do + it 'raises an Stormpath error' do expect do controller_authenticator.authenticate! - end.to raise_error(Stormpath::Rails::ControllerAuthentication::UnauthenticatedRequest) + end.to raise_error(Stormpath::Error) end end describe 'with un encoded api key and secret' do let(:credentials) { 'unencodedapikeyid:unencodedapikeysecret' } - it 'raises an UnauthenticatedRequest error' do + it 'raises an Stormpath error' do expect do controller_authenticator.authenticate! - end.to raise_error(Stormpath::Rails::ControllerAuthentication::UnauthenticatedRequest) + end.to raise_error(Stormpath::Error) end end describe 'with empty token' do let(:credentials) { '' } - it 'raises an UnauthenticatedRequest error' do + it 'raises an Stormpath error' do expect do controller_authenticator.authenticate! - end.to raise_error(Stormpath::Rails::ControllerAuthentication::UnauthenticatedRequest) + end.to raise_error(Stormpath::Error) end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 4379e25..ecaf5b1 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -53,9 +53,7 @@ RSpec.configure do |config| config.use_transactional_fixtures = true config.include FactoryGirl::Syntax::Methods - config.include Stormpath::Testing::Helpers, type: :request - config.include Stormpath::Testing::Helpers, type: :feature - config.include Stormpath::Testing::Helpers, type: :service + config.include Stormpath::Testing::Helpers config.include Rails.application.routes.url_helpers, type: :service config.include MatchJson::Matchers config.include Capybara::DSL, type: :feature @@ -85,4 +83,4 @@ Capybara::RackTest::Driver.new(app, headers: { 'HTTP_ACCEPT' => 'text/html' }) end -Rails.application.routes.default_url_options[:host]= 'localhost:3000' +Rails.application.routes.default_url_options[:host]= 'localhost:3000' diff --git a/spec/stormpath/authentication_spec.rb b/spec/stormpath/authentication_spec.rb new file mode 100644 index 0000000..e4d60df --- /dev/null +++ b/spec/stormpath/authentication_spec.rb @@ -0,0 +1,150 @@ +require 'spec_helper' + +describe Stormpath::Rails::Controller, vcr: true, type: :request do + shared_examples 'a restricting controller' do + it 'should redirect to login uri' do + get_profile + expect(response).to redirect_to('/login?next=/me') + end + + it 'should have status 302' do + get_profile + expect(response.status).to eq 302 + end + end + + shared_examples 'a profile controller' do + it 'should return account' do + get_profile + expect(JSON.parse(response.body)['account']['email']).to eq account.email + end + + it 'should respond with 200' do + get_profile + expect(response.status).to eq 200 + end + end + + let(:application) { test_application } + let(:directory) { test_client.directories.first } + let(:account_attrs) { FactoryGirl.attributes_for(:account) } + let(:account) { application.accounts.create(account_attrs) } + let(:application2) { test_client.applications.create(FactoryGirl.attributes_for(:application)) } + let(:directory2) { test_client.directories.create(FactoryGirl.attributes_for(:directory)) } + let(:map_another_directory) { map_account_store(application2, directory2, 0, true, true) } + let(:account2) { application2.accounts.create(account_attrs) } + let(:expired_token) do + 'eyJraWQiOiI2VTRIWk1IR0VZMEpHV1ZITjBVVU81QkdXIiwiYWxnIjoiSFMyNTYifQ.eyJqdGkiOiI0MTFwUFh6QlQ1Qmo4ckM2VVZBbGRQIiwiaWF0IjoxNDY0MTc3NzMyLCJpc3MiOiJodHRwczovL2FwaS5zdG9ybXBhdGguY29tL3YxL2FwcGxpY2F0aW9ucy8zblpsTEtWTUlPUHU3MVlDN1RGUjBvIiwic3ViIjoiaHR0cHM6Ly9hcGkuc3Rvcm1wYXRoLmNvbS92MS9hY2NvdW50cy80MDMxTkF2UU9HNEZJMldXSjhRNjExIiwiZXhwIjoxNDY0MTgxMzMyLCJydGkiOiI0MTFwUFVmNllVc2tXMjZGSUZKVjFMIn0.ltcEQqkVnMutBQItQehVn2ckXwsxnBjfTucFIuoGVNY' + end + let(:aquire_token) { application.authenticate_oauth(password_grant_request) } + let(:access_token) { aquire_token.access_token } + let(:api_key) { account.api_keys.create({}) } + let(:encoded_api_key) { Base64.encode64("#{api_key.id}:#{api_key.secret}") } + let(:get_profile) do + get '/me', {}, header => value + end + + after { account.delete } + + describe 'from cookies' do + let(:header) { 'HTTP_COOKIE' } + + context 'without access token' do + let(:value) { 'access_token=' } + it_should_behave_like 'a restricting controller' + end + + context 'expired access token' do + let(:value) { "access_token=#{expired_token}" } + it_should_behave_like 'a restricting controller' + end + + context 'invalid issuer' do + before do + map_another_directory + account2 + end + let(:password_grant_request) do + Stormpath::Oauth::PasswordGrantRequest.new(account2.email, 'Password1337') + end + let(:access_token) { aquire_token.access_token } + let(:aquire_token) { application2.authenticate_oauth(password_grant_request) } + let(:value) { "access_token=#{access_token}" } + after do + account2.delete + directory2.delete + application2.delete + end + it_should_behave_like 'a restricting controller' + end + + context 'valid access token' do + let(:password_grant_request) do + Stormpath::Oauth::PasswordGrantRequest.new(account.email, 'Password1337') + end + let!(:value) { "access_token=#{access_token}" } + it_should_behave_like 'a profile controller' + end + end + + describe 'basic auth' do + let(:header) { 'Authorization' } + + context 'valid authorization_header' do + let(:value) { "Basic #{encoded_api_key}" } + it_should_behave_like 'a profile controller' + end + + context 'without authorization_header' do + let(:value) { 'Basic ' } + it_should_behave_like 'a restricting controller' + end + + context 'with just the api key id as the authorization_header' do + let(:encoded_api_key) { Base64.encode64("#{api_key.id}:") } + let(:value) { "Basic #{encoded_api_key}" } + it_should_behave_like 'a restricting controller' + end + end + + describe 'bearer auth' do + let(:header) { 'Authorization' } + + context 'without access token' do + let(:value) { 'Bearer ' } + it_should_behave_like 'a restricting controller' + end + + context 'expired access token' do + let(:value) { "Bearer #{expired_token}" } + it_should_behave_like 'a restricting controller' + end + + context 'invalid issuer' do + before do + map_another_directory + account2 + end + let(:password_grant_request) do + Stormpath::Oauth::PasswordGrantRequest.new(account2.email, 'Password1337') + end + let(:access_token) { aquire_token.access_token } + let(:aquire_token) { application2.authenticate_oauth(password_grant_request) } + let(:value) { "Bearer #{access_token}" } + after do + account2.delete + directory2.delete + application2.delete + end + it_should_behave_like 'a restricting controller' + end + + context 'valid access token' do + let(:password_grant_request) do + Stormpath::Oauth::PasswordGrantRequest.new(account.email, 'Password1337') + end + let!(:value) { "Bearer #{access_token}" } + it_should_behave_like 'a profile controller' + end + end +end diff --git a/spec/support/stormpath_testing_helpers.rb b/spec/support/stormpath_testing_helpers.rb index 1010e1f..d7679e7 100644 --- a/spec/support/stormpath_testing_helpers.rb +++ b/spec/support/stormpath_testing_helpers.rb @@ -19,6 +19,24 @@ def delete_account(email) Stormpath::Rails::Client.application.accounts.search(email: email).first.delete end + def test_application + Stormpath::Rails::Client.application + end + + def test_client + Stormpath::Rails::Client.client + end + + def map_account_store(app, store, index, default_account_store, default_group_store) + test_client.account_store_mappings.create( + application: app, + account_store: store, + list_index: index, + is_default_account_store: default_account_store, + is_default_group_store: default_group_store + ) + end + def default_domain '@testmail.stormpath.com' end