diff --git a/app/api/api_v1/auth/jwt_authenticator.rb b/app/api/api_v1/auth/jwt_authenticator.rb index fa7b833..6e0e999 100644 --- a/app/api/api_v1/auth/jwt_authenticator.rb +++ b/app/api/api_v1/auth/jwt_authenticator.rb @@ -9,18 +9,14 @@ def initialize(token) # # Decodes and verifies JWT. - # Returns authentic member email or raises an exception. + # Returns hash formed with payload including user id (uid) or raises an exception. # - # @param [Hash] options - # @return [String, User, NilClass] - def authenticate!(options = {}) - unless @token_type == 'Bearer' - raise AuthorizationError, 'Token type is not provided or invalid.' - end + # @return [Hash, NilClass] + def authenticate! + check_token_type payload, _header = decode_and_verify_token(@token_value) - fetch_user(payload).yield_self do |user| - options[:return] == :user ? user : fetch_uid(payload) - end + + { uid: fetch_uid(payload) } rescue => e report_exception(e) if AuthorizationError === e @@ -33,14 +29,20 @@ def authenticate!(options = {}) # # Exception-safe version of #authenticate!. # - # @return [String, User, NilClass] + # @return [Hash, NilClass] def authenticate(*args) authenticate!(*args) rescue nil end - private + private + + def check_token_type + unless @token_type == 'Bearer' + raise AuthorizationError, 'Token type is not provided or invalid.' + end + end def decode_and_verify_token(token) JWT.decode(token, Utils.jwt_public_key, true, token_verification_options) @@ -49,49 +51,12 @@ def decode_and_verify_token(token) raise AuthorizationError, "Failed to decode and verify JWT: #{e.inspect}." end - def fetch_email(payload) - payload[:email].to_s.tap do |email| - raise(AuthorizationError, 'E-Mail is blank.') if email.blank? - raise(AuthorizationError, 'E-Mail is invalid.') unless EmailValidator.valid?(email) - end - end - def fetch_uid(payload) payload.fetch(:uid).tap do |uid| raise(AuthorizationError, 'UID is blank.') if uid.blank? end end - def fetch_scopes(payload) - Array.wrap(payload[:scopes]).map(&:to_s).map(&:squash).reject(&:blank).tap do |scopes| - raise(AuthorizationError, 'Token scopes are not defined.') if scopes.empty? - end - end - - def fetch_user(payload) - if payload[:iss] == 'barong' - from_barong_payload(payload) - else - User.find_by(uid: fetch_uid(payload)) - end - end - - def from_barong_payload(payload) - User.find_or_initialize_by(uid: fetch_uid(payload)).tap do |member| - member.transaction do - attributes = { - email: fetch_email(payload), - state: payload.fetch(:state).to_s, - level: payload.fetch(:level).to_i - } - - # Prevent overheat validations. - member.assign_attributes(attributes) - member.save!(validate: member.new_record?) - end - end - end - def token_verification_options { verify_expiration: true, verify_not_before: true, diff --git a/app/api/api_v1/auth/middleware.rb b/app/api/api_v1/auth/middleware.rb index b2a5e67..6dc60f3 100644 --- a/app/api/api_v1/auth/middleware.rb +++ b/app/api/api_v1/auth/middleware.rb @@ -6,8 +6,8 @@ class Middleware < Grape::Middleware::Base def before return unless auth_by_jwt? - env['api.v1.authenticated_uid'] = \ - JWTAuthenticator.new(headers['Authorization']).authenticate!(return: :uid) + env['api.v1.authenticated_data'] = \ + JWTAuthenticator.new(headers['Authorization']).authenticate! end private diff --git a/app/api/api_v1/base.rb b/app/api/api_v1/base.rb index a3d3020..c702aa1 100644 --- a/app/api/api_v1/base.rb +++ b/app/api/api_v1/base.rb @@ -19,7 +19,7 @@ class Base < Grape::API doc_version: '0.0.1', info: { title: 'User API V1', - description: 'User API is client-to-server API' + description: 'User API is client-to-server API v.1' }, hide_format: true, hide_documentation_path: true diff --git a/app/api/api_v1/cors/middleware.rb b/app/api/api_v1/cors/middleware.rb index d556846..1b78247 100644 --- a/app/api/api_v1/cors/middleware.rb +++ b/app/api/api_v1/cors/middleware.rb @@ -36,7 +36,7 @@ def call(env) end end - private + private def request @request ||= Grape::Request.new(env) diff --git a/app/api/api_v1/helpers.rb b/app/api/api_v1/helpers.rb index 994eae2..224844f 100644 --- a/app/api/api_v1/helpers.rb +++ b/app/api/api_v1/helpers.rb @@ -5,15 +5,15 @@ module Helpers extend Memoist def authenticate! - current_user || raise(AuthorizationError) + current_uid || raise(AuthorizationError) end - def current_user - key = 'api.v1.authenticated_uid' # JWT authentication provides user uid. + def current_uid + key = 'api.v1.authenticated_data' # JWT authentication provides user uid. return unless env.key?(key) - User.find_by(uid: env[key]) + env[key][:uid] end - memoize :current_user + memoize :current_uid end end diff --git a/app/api/api_v1/withdraw.rb b/app/api/api_v1/withdraw.rb index 842ea43..0d5c5ed 100644 --- a/app/api/api_v1/withdraw.rb +++ b/app/api/api_v1/withdraw.rb @@ -22,7 +22,7 @@ class Withdraw < Grape::API post '/withdraws' do Peatio::ManagementAPIv1Client.new.create_withdraw \ - uid: env['api.v1.authenticated_uid'], + uid: current_uid, currency: params[:currency], amount: params[:amount], otp_code: params[:otp], diff --git a/spec/api/v1/auth/jwt_authenticator_spec.rb b/spec/api/v1/auth/jwt_authenticator_spec.rb index 0899cf8..63447bb 100644 --- a/spec/api/v1/auth/jwt_authenticator_spec.rb +++ b/spec/api/v1/auth/jwt_authenticator_spec.rb @@ -19,18 +19,14 @@ headers: { 'Authorization' => token } end - let :user do - create(:user, :level_3) - end - let :payload do - { x: 'x', y: 'y', z: 'z', uid: user.uid } + { x: 'x', y: 'y', z: 'z', uid: 'O90Y88JDPQ7167' } end subject { APIv1::Auth::JWTAuthenticator.new(request.headers['Authorization']) } it 'should work in standard conditions' do - expect(subject.authenticate!).to eq user.uid + expect(subject.authenticate!).to eq payload.slice(:uid) end it 'should raise exception when uid is not provided' do @@ -60,23 +56,11 @@ end end - describe 'authentication options' do - it 'should return uid if return: :uid specified' do - expect(subject.authenticate!(return: :uid)).to eq payload[:uid] - end - - it 'should return user if return: :user specified' do - create(:user) - payload[:uid] = user.uid - expect(subject.authenticate!(return: :user)).to eq user - end - end - context 'valid issuer' do before { ENV['JWT_ISSUER'] = 'qux' } before { payload[:iss] = 'qux' } after { ENV.delete('JWT_ISSUER') } - it('should validate issuer') { expect(subject.authenticate!).to eq user.uid } + it('should validate issuer') { expect(subject.authenticate!).to eq payload.slice(:uid) } end context 'invalid issuer' do @@ -92,7 +76,7 @@ before { ENV['JWT_AUDIENCE'] = 'foo,bar' } before { payload[:aud] = ['bar'] } after { ENV.delete('JWT_AUDIENCE') } - it('should validate audience') { expect(subject.authenticate!).to eq user.uid } + it('should validate audience') { expect(subject.authenticate!).to eq payload.slice(:uid) } end context 'invalid audience' do @@ -120,67 +104,6 @@ context 'issued at before future' do before { payload[:iat] = 3.seconds.ago.to_i } - it('should allow JWT') { expect(subject.authenticate!).to eq user.uid } - end - - describe 'on the fly registration' do - context 'token not issued by Barong' do - before { payload[:iss] = 'someone' } - it 'should not register user unless token is issued by Barong' do - expect { subject.authenticate! }.not_to change(User, :count) - end - end - - context 'token issued by Barong' do - before { payload[:iss] = 'barong' } - - it 'should require level to be present in payload' do - payload.merge!(state: 'pending', uid: Faker::Internet.password(14, 14), email: Faker::Internet.email) - expect { subject.authenticate! }.to raise_error(APIv1::AuthorizationError) { |e| expect(e.reason).to match /key not found: :level/ } - end - - it 'should require state to be present in payload' do - payload.merge!(level: 1, uid: Faker::Internet.password(14, 14), email: Faker::Internet.email) - expect { subject.authenticate! }.to raise_error(APIv1::AuthorizationError) { |e| expect(e.reason).to match /key not found: :state/ } - end - - it 'should require UID to be present in payload' do - payload.merge!(level: 1, state: 'disabled', email: Faker::Internet.email).delete(:uid) - expect { subject.authenticate! }.to raise_error(APIv1::AuthorizationError) { |e| expect(e.reason).to match /key not found: :uid/ } - end - - it 'should require UID to be not blank' do - payload.merge!(level: 1, state: 'disabled', email: Faker::Internet.email, uid: ' ') - expect { subject.authenticate! }.to raise_error(APIv1::AuthorizationError) { |e| expect(e.reason).to match /UID is blank/ } - end - - it 'should raise exception when email is invalid' do - payload[:email] = '@gmail.com' - expect { subject.authenticate! }.to raise_error(APIv1::AuthorizationError) { |e| expect(e.reason).to match /invalid/ } - end - - it 'should register user' do - payload.merge!(email: 'guyfrombarong@email.com', uid: 'BARONG1234', state: 'active', level: 2) - expect { subject.authenticate! }.to change(User, :count).by(1) - record = User.last - expect(record.uid).to eq payload[:uid] - expect(record.level).to eq 2 - end - - it 'should update user if exists' do - user = create(:user, :level_1) - payload.merge!(email: Faker::Internet.email, uid: user.uid, state: 'blocked', level: 3) - expect { subject.authenticate! }.not_to change(User, :count) - user.reload - expect(user.uid).to eq payload[:uid] - expect(user.level).to eq 3 - end - - it 'should register new user and return instance' do - payload.merge!(email: 'guyfrombarong@email.com', uid: 'BARONG1234', state: '', level: 100) - expect(subject.authenticate!(return: :user)).to eq User.last - expect(User.last.level).to eq 100 - end - end + it('should allow JWT') { expect(subject.authenticate!).to eq payload.slice(:uid) } end end diff --git a/spec/api/v1/auth/middleware_spec.rb b/spec/api/v1/auth/middleware_spec.rb index b91f577..bb83e74 100644 --- a/spec/api/v1/auth/middleware_spec.rb +++ b/spec/api/v1/auth/middleware_spec.rb @@ -9,15 +9,14 @@ class TestApp < Grape::API get '/' do authenticate! - current_user.uid + current_uid end end let(:app) { TestApp.new } context 'when using JWT authentication' do - let(:user) { create(:user, :level_3) } - let(:payload) { { x: 'x', y: 'y', z: 'z', uid: user.uid } } + let(:payload) { { x: 'x', y: 'y', z: 'z', uid: 'O90Y88JDPQ7167' } } let(:token) { jwt_build(payload) } it 'should deny access when token is not given' do @@ -32,17 +31,10 @@ class TestApp < Grape::API expect(response.body).to eq '{"error":{"code":2001,"message":"Authorization failed"}}' end - it 'should deny access when member doesn\'t exist' do - payload[:uid] = 'BARONG1234' - api_get '/', token: token - expect(response.code).to eq '401' - expect(response.body).to eq '{"error":{"code":2001,"message":"Authorization failed"}}' - end - it 'should allow access when valid token is given' do api_get '/', token: token - expect(response).to be_success - expect(response.body).to eq user.uid + expect(response).to be_successful + expect(response.body).to eq payload[:uid] end end diff --git a/spec/api/v1/cors_spec.rb b/spec/api/v1/cors_spec.rb index 9e3f36b..35f9b96 100644 --- a/spec/api/v1/cors_spec.rb +++ b/spec/api/v1/cors_spec.rb @@ -18,7 +18,7 @@ def check_cors(response) it 'sends CORS headers when requesting using OPTIONS' do reset! unless integration_session integration_session.send :process, 'OPTIONS', '/api/v1/withdraws' - expect(response).to be_success + expect(response).to be_successful check_cors(response) end