From d9a11b265ee4aa76ca53e3a4ebc1fd6e1fccf83f Mon Sep 17 00:00:00 2001 From: Campbell Allen Date: Tue, 13 Nov 2018 23:47:52 +0000 Subject: [PATCH] Credential add specs and refactor (#14) * add pending specs on credential * fix logged_in? credential method * add specs for expired? tokens * spec out accessible_project? spec and optimize the lookup for accessible project for the requesting user * add message on token validity failure --- interventions_gateway_api.rb | 2 +- lib/credential.rb | 91 +++++++++++++-------- spec/interventions_api_gateway_spec.rb | 1 + spec/lib/credential_spec.rb | 108 +++++++++++++++++++++++++ 4 files changed, 167 insertions(+), 35 deletions(-) create mode 100644 spec/lib/credential_spec.rb diff --git a/interventions_gateway_api.rb b/interventions_gateway_api.rb index 3642bd6..a1559ab 100644 --- a/interventions_gateway_api.rb +++ b/interventions_gateway_api.rb @@ -29,7 +29,7 @@ class InterventionsGatewayApi < Sinatra::Base if request.post? setup_credentials unless valid_credentials - halt 401 + halt 401, 'invalid credentials, please check your token details' end end end diff --git a/lib/credential.rb b/lib/credential.rb index 63f60f9..f21f32d 100644 --- a/lib/credential.rb +++ b/lib/credential.rb @@ -11,43 +11,40 @@ def initialize(token) end def logged_in? - return false unless jwt_payload.present? - jwt_payload['login'].present? - rescue JWT::ExpiredSignature + if user_login + true + else + false + end + rescue JWTDecoder::InvalidToken false end def expired? - expires_at < Time.zone.now - end - - def project_ids - @project_ids ||= - fetch_accessible_projects['projects'].map { |prj| prj['id'] } + expires_at < Time.now.utc + rescue JWTDecoder::InvalidToken + true end def accessible_project?(id) - project_ids.include?(id) - end - - def accessible_workflow?(id) - response = client.panoptes.get("/workflows/#{id}") - workflow_hash = response['workflows'][0] - project_id = workflow_hash['links']['project'].to_i - - if project_ids.include?(project_id) - workflow_hash + api_response = client.panoptes.paginate( + '/projects', + { + id: id, + current_user_roles: OWNER_ROLES, + cards: true + } + ) + + if api_response["projects"].empty? + false + else + true end - rescue Panoptes::Client::ResourceNotFound - nil end private - def jwt_payload - @jwt_payload ||= token ? client.current_user : {} - end - def client @client ||= Panoptes::Client.new(env: panoptes_client_env, auth: { token: token }) end @@ -56,18 +53,44 @@ def panoptes_client_env ENV["RACK_ENV"] end + def jwt_payload + @decoder ||= JWTDecoder.new(token, client) + @decoder.payload + end + + def user_login + @user_login ||= jwt_payload.dig('data', 'login') + end + def expires_at - @expires_at ||= begin - payload, _ = JWT.decode token, client.jwt_signing_public_key, algorithm: 'RS512' - Time.at(payload.fetch('exp')) - end + @expires_at ||= Time.at(jwt_payload['exp']) end - def fetch_accessible_projects - puts "Loading accessible projects from Panoptes" - result = client.panoptes.paginate('/projects', current_user_roles: OWNER_ROLES) + class JWTDecoder + class InvalidToken < StandardError; end + + attr_reader :token, :client - puts "done" - result + def initialize(token, client) + @token = token + @client = client + end + + def payload + @payload ||= decode_payload + end + + private + + def decode_payload + payload, _ = JWT.decode( + token, + client.jwt_signing_public_key, + algorithm: 'RS512' + ) + payload + rescue JWT::ExpiredSignature, JWT::VerificationError + raise InvalidToken + end end end diff --git a/spec/interventions_api_gateway_spec.rb b/spec/interventions_api_gateway_spec.rb index 0d90452..6b1a053 100644 --- a/spec/interventions_api_gateway_spec.rb +++ b/spec/interventions_api_gateway_spec.rb @@ -18,6 +18,7 @@ it "should respond with unauthorized without auth headers" do post end_point, json_payload expect(last_response).to be_unauthorized + expect(last_response.body).to eq("invalid credentials, please check your token details") end context "when token is expired" do diff --git a/spec/lib/credential_spec.rb b/spec/lib/credential_spec.rb new file mode 100644 index 0000000..82ca62f --- /dev/null +++ b/spec/lib/credential_spec.rb @@ -0,0 +1,108 @@ +require 'spec_helper.rb' + +describe "Credential" do + let(:token) { "fake_token" } + let(:credential) { Credential.new(token) } + + before do + # override the client env to ensure there is a signing key for decoding + allow(credential) + .to receive(:panoptes_client_env) + .and_return("production") + end + + describe "logged_in?" do + before do + allow(JWT).to receive(:decode).and_return(payload) + end + + context "with a valid token" do + let(:payload) do + { 'data' => { 'login' => 'test-user' } } + end + + it "should pass" do + expect(credential.logged_in?).to eq(true) + end + end + + context "with a token missing the login attribute" do + let(:payload) do + { 'data' => { 'id' => 1 } } + end + + it "should fail" do + expect(credential.logged_in?).to eq(false) + end + end + end + + describe "expired?" do + before do + allow(JWT).to receive(:decode).and_return(payload) + end + + context "with a non expired token" do + let(:one_minute) { 60 } + let(:payload) do + { 'exp' => (Time.now + one_minute).utc.to_i } + end + + it "should be false" do + expect(credential.expired?).to eq(false) + end + end + + context "with a non expired token" do + let(:three_hours) { 3 * 60 * 60 } + let(:payload) do + { 'exp' => (Time.now - three_hours).utc.to_i } + end + + it "should be true" do + expect(credential.expired?).to eq(true) + end + end + + context "with a token that can't be decoded" do + let(:payload) { {} } + + it "should be true" do + allow(JWT).to receive(:decode).and_raise(JWT::ExpiredSignature) + expect(credential.expired?).to eq(true) + end + end + end + + describe "accessible_project?" do + let(:client_double) do + instance_double("Panoptes::Client", panoptes: page_double) + end + + before do + allow(Panoptes::Client).to receive(:new).and_return(client_double) + end + + context "with correct roles on the project for the supplied token" do + let(:response) do + { + "projects"=> [{"id"=>"1"}] + } + end + let(:page_double) { double(paginate: response) } + + it "should pass" do + expect(credential.accessible_project?(1)).to eq(true) + end + end + + context "with no correct roles on the project for the supplied token" do + let(:response) { { "projects"=> [] } } + let(:page_double) { double(paginate: response) } + + it "should fail" do + expect(credential.accessible_project?(1)).to eq(false) + end + end + end +end