From 1997f45d7ee98d13030071a532670291c382d2c1 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 24 Aug 2018 15:53:57 +0200 Subject: [PATCH] Api Token: do not generate one token by default --- app/controllers/api_controller.rb | 19 ++++++++++++ app/models/administrateur.rb | 7 ----- .../api/v1/dossiers_controller_spec.rb | 2 +- .../api/v1/procedures_controller_spec.rb | 2 +- spec/controllers/api_controller_spec.rb | 31 +++++++++++++++++-- spec/factories/administrateur.rb | 6 ++++ spec/models/administrateur_spec.rb | 21 ------------- 7 files changed, 55 insertions(+), 33 deletions(-) diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index e387c3b69..77581534e 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -7,6 +7,11 @@ class APIController < ApplicationController ``` EOS + # deny request with an empty token as we do not want it + # to match the first admin with an empty token + # it should not happen as an empty token is serialized by '' + # and a administrateur without token has admin.api_token == nil + before_action :ensure_token_is_present before_action :authenticate_user before_action :default_format_json @@ -39,4 +44,18 @@ class APIController < ApplicationController def default_format_json request.format = "json" if !request.params[:format] end + + def ensure_token_is_present + if params[:token].blank? && header_token.blank? + render json: {}, status: 401 + end + end + + def header_token + received_token = nil + authenticate_with_http_token do |token, _options| + received_token = token + end + received_token + end end diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index b4662e58d..e094bc4e6 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -13,7 +13,6 @@ class Administrateur < ApplicationRecord has_many :dossiers, -> { state_not_brouillon }, through: :procedures before_validation -> { sanitize_email(:email) } - before_save :ensure_api_token scope :inactive, -> { where(active: false) } @@ -36,12 +35,6 @@ class Administrateur < ApplicationRecord self.inactive.find(id) end - def ensure_api_token - if api_token.nil? - self.api_token = generate_api_token - end - end - def renew_api_token update(api_token: generate_api_token) end diff --git a/spec/controllers/api/v1/dossiers_controller_spec.rb b/spec/controllers/api/v1/dossiers_controller_spec.rb index 549ee4f95..84f55e0cf 100644 --- a/spec/controllers/api/v1/dossiers_controller_spec.rb +++ b/spec/controllers/api/v1/dossiers_controller_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe API::V1::DossiersController do - let(:admin) { create(:administrateur) } + let(:admin) { create(:administrateur, :with_api_token) } let(:procedure) { create(:procedure, :with_two_type_de_piece_justificative, :with_type_de_champ, :with_type_de_champ_private, administrateur: admin) } let(:wrong_procedure) { create(:procedure) } diff --git a/spec/controllers/api/v1/procedures_controller_spec.rb b/spec/controllers/api/v1/procedures_controller_spec.rb index 3aeb7c023..7751f6db4 100644 --- a/spec/controllers/api/v1/procedures_controller_spec.rb +++ b/spec/controllers/api/v1/procedures_controller_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe API::V1::ProceduresController, type: :controller do - let(:admin) { create(:administrateur) } + let(:admin) { create(:administrateur, :with_api_token) } it { expect(described_class).to be < APIController } describe 'GET show' do diff --git a/spec/controllers/api_controller_spec.rb b/spec/controllers/api_controller_spec.rb index a5eeb335b..2060f3533 100644 --- a/spec/controllers/api_controller_spec.rb +++ b/spec/controllers/api_controller_spec.rb @@ -12,18 +12,43 @@ describe APIController, type: :controller do end describe 'GET index' do + let!(:administrateur) { create(:administrateur) } + let!(:administrateur_with_token) { create(:administrateur, :with_api_token) } + context 'when token is missing' do subject { get :index } + it { expect(subject.status).to eq(401) } end + + context 'when token is empty' do + subject { get :index, params: { token: nil } } + + it { expect(subject.status).to eq(401) } + end + context 'when token does not exist' do let(:token) { 'invalid_token' } + subject { get :index, params: { token: token } } + it { expect(subject.status).to eq(401) } end - context 'when token exist' do - let(:administrateur) { create(:administrateur) } - subject { get :index, params: { token: administrateur.api_token } } + + context 'when token exist in the params' do + subject { get :index, params: { token: administrateur_with_token.api_token } } + + it { expect(subject.status).to eq(200) } + end + + context 'when token exist in the header' do + before do + valid_headers = { 'Authorization' => "Bearer token=#{administrateur_with_token.api_token}" } + request.headers.merge!(valid_headers) + end + + subject { get(:index) } + it { expect(subject.status).to eq(200) } end end diff --git a/spec/factories/administrateur.rb b/spec/factories/administrateur.rb index 973802d40..abcb8dad2 100644 --- a/spec/factories/administrateur.rb +++ b/spec/factories/administrateur.rb @@ -4,4 +4,10 @@ FactoryBot.define do email { generate(:administrateur_email) } password { 'mon chien aime les bananes' } end + + trait :with_api_token do + after(:create) do |admin| + admin.renew_api_token + end + end end diff --git a/spec/models/administrateur_spec.rb b/spec/models/administrateur_spec.rb index 9a3388c0a..c5dd9856f 100644 --- a/spec/models/administrateur_spec.rb +++ b/spec/models/administrateur_spec.rb @@ -8,27 +8,6 @@ describe Administrateur, type: :model do it { is_expected.to have_many(:procedures) } end - describe 'after_save' do - subject { create(:administrateur) } - before do - subject.save - end - it { expect(subject.api_token).not_to be_blank } - end - - describe 'generate_api_token' do - let(:token) { 'bullshit' } - let(:new_token) { 'pocket_master' } - let!(:admin_1) { create(:administrateur, api_token: token) } - before do - allow(SecureRandom).to receive(:hex).and_return(token, new_token) - admin_1.renew_api_token - end - it 'generate a token who does not already exist' do - expect(admin_1.api_token).to eq(new_token) - end - end - context 'unified login' do it 'syncs credentials to associated user' do administrateur = create(:administrateur)