diff --git a/app/controllers/administrateurs/activate_controller.rb b/app/controllers/administrateurs/activate_controller.rb index 2d45c0da1..b4bf8346d 100644 --- a/app/controllers/administrateurs/activate_controller.rb +++ b/app/controllers/administrateurs/activate_controller.rb @@ -47,7 +47,6 @@ class Administrateurs::ActivateController < ApplicationController if resource&.valid_password?(password) sign_in resource - resource.force_sync_credentials end end end diff --git a/app/controllers/sessions/sessions_controller.rb b/app/controllers/sessions/sessions_controller.rb deleted file mode 100644 index 4bff2a161..000000000 --- a/app/controllers/sessions/sessions_controller.rb +++ /dev/null @@ -1,23 +0,0 @@ -class Sessions::SessionsController < Devise::SessionsController - before_action :before_sign_in, only: [:create] - - layout 'new_application' - - def before_sign_in - if user_signed_in? - sign_out :user - end - - if instructeur_signed_in? - sign_out :instructeur - end - - if administrateur_signed_in? - sign_out :administrateur - end - - if administration_signed_in? - sign_out :administration - end - end -end diff --git a/app/controllers/users/activate_controller.rb b/app/controllers/users/activate_controller.rb index c8553c3a4..657e2cf7c 100644 --- a/app/controllers/users/activate_controller.rb +++ b/app/controllers/users/activate_controller.rb @@ -14,36 +14,25 @@ class Users::ActivateController < ApplicationController end def create - password = create_user_params[:password] user = User.reset_password_by_token({ - password: password, - password_confirmation: password, - reset_password_token: create_user_params[:reset_password_token] + password: user_params[:password], + reset_password_token: user_params[:reset_password_token] }) - if user && user.errors.empty? + if user.valid? sign_in(user, scope: :user) flash.notice = "Mot de passe enregistré" redirect_to instructeur_procedures_path else flash.alert = user.errors.full_messages - redirect_to users_activate_path(token: create_user_params[:reset_password_token]) + redirect_to users_activate_path(token: user_params[:reset_password_token]) end end private - def create_user_params + def user_params params.require(:user).permit(:reset_password_token, :password) end - - def try_to_authenticate(klass, email, password) - resource = klass.find_for_database_authentication(email: email) - - if resource&.valid_password?(password) - sign_in resource - resource.force_sync_credentials - end - end end diff --git a/app/controllers/users/confirmations_controller.rb b/app/controllers/users/confirmations_controller.rb index ec5755987..b66d836be 100644 --- a/app/controllers/users/confirmations_controller.rb +++ b/app/controllers/users/confirmations_controller.rb @@ -42,7 +42,6 @@ class Users::ConfirmationsController < Devise::ConfirmationsController if sign_in_after_confirmation?(resource) resource.remember_me = true sign_in(resource) - resource.force_sync_credentials after_sign_in_path_for(resource_name) else super(resource_name, resource) diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index e3b3f9536..13f751a42 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -1,4 +1,4 @@ -class Users::SessionsController < Sessions::SessionsController +class Users::SessionsController < Devise::SessionsController include ProcedureContextConcern include TrustedDeviceConcern include ActionView::Helpers::DateHelper @@ -7,33 +7,15 @@ class Users::SessionsController < Sessions::SessionsController before_action :restore_procedure_context, only: [:new, :create] - # GET /resource/sign_in - def new - @user = User.new - end - # POST /resource/sign_in def create - remember_me = params[:user][:remember_me] == '1' + user = User.find_by(email: params[:user][:email]) - if resource_locked?(try_to_authenticate(User, remember_me)) - flash.alert = 'Votre compte est verrouillé.' - new - return render :new, status: 401 + if user&.valid_password?(params[:user][:password]) + user.update(loged_in_with_france_connect: nil) end - if user_signed_in? - current_user.update(loged_in_with_france_connect: nil) - end - - if instructeur_signed_in? || user_signed_in? - set_flash_message :notice, :signed_in - redirect_to after_sign_in_path_for(:user) - else - flash.alert = 'Mauvais couple login / mot de passe' - new - render :new, status: 401 - end + super end def link_sent @@ -91,23 +73,4 @@ class Users::SessionsController < Sessions::SessionsController redirect_to link_sent_path(email: instructeur.email) end end - - private - - def try_to_authenticate(klass, remember_me = false) - resource = klass.find_for_database_authentication(email: params[:user][:email]) - - if resource.present? - if resource.valid_password?(params[:user][:password]) - resource.remember_me = remember_me - sign_in resource - resource.force_sync_credentials - end - end - resource - end - - def resource_locked?(resource) - resource.present? && resource.access_locked? - end end diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index 0a3007694..9a9f39174 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -1,5 +1,4 @@ class Administrateur < ApplicationRecord - include CredentialsSyncableConcern include EmailSanitizableConcern include ActiveRecord::SecureToken diff --git a/app/models/concerns/credentials_syncable_concern.rb b/app/models/concerns/credentials_syncable_concern.rb deleted file mode 100644 index f42b4adae..000000000 --- a/app/models/concerns/credentials_syncable_concern.rb +++ /dev/null @@ -1,18 +0,0 @@ -module CredentialsSyncableConcern - extend ActiveSupport::Concern - - included do - after_update :sync_credentials - end - - def sync_credentials - if saved_change_to_email? || saved_change_to_encrypted_password? - return force_sync_credentials - end - true - end - - def force_sync_credentials - SyncCredentialsService.new(self.class, email_before_last_save, email, encrypted_password).change_credentials! - end -end diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index b61fd3b2d..5fcb1b205 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -1,5 +1,4 @@ class Instructeur < ApplicationRecord - include CredentialsSyncableConcern include EmailSanitizableConcern has_and_belongs_to_many :administrateurs diff --git a/app/models/user.rb b/app/models/user.rb index 42f7ceaa1..1f07f518d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,5 +1,4 @@ class User < ApplicationRecord - include CredentialsSyncableConcern include EmailSanitizableConcern enum loged_in_with_france_connect: { diff --git a/app/services/sync_credentials_service.rb b/app/services/sync_credentials_service.rb deleted file mode 100644 index 13322b1f4..000000000 --- a/app/services/sync_credentials_service.rb +++ /dev/null @@ -1,33 +0,0 @@ -class SyncCredentialsService - def initialize(klass, email_before_last_save, email, encrypted_password) - @klass = klass - @email_before_last_save = email_before_last_save - @email = email - @encrypted_password = encrypted_password - end - - def change_credentials! - if @klass != User - user = User.find_by(email: @email_before_last_save) - if user && !user.update_columns(email: @email, encrypted_password: @encrypted_password) - return false - end - end - - if @klass != Instructeur - instructeur = Instructeur.find_by(email: @email_before_last_save) - if instructeur && !instructeur.update_columns(email: @email, encrypted_password: @encrypted_password) - return false - end - end - - if @klass != Administrateur - administrateur = Administrateur.find_by(email: @email_before_last_save) - if administrateur && !administrateur.update_columns(email: @email, encrypted_password: @encrypted_password) - return false - end - end - - true - end -end diff --git a/app/views/administrateurs/sessions/new.html.haml b/app/views/administrateurs/sessions/new.html.haml deleted file mode 100644 index c24523417..000000000 --- a/app/views/administrateurs/sessions/new.html.haml +++ /dev/null @@ -1,26 +0,0 @@ -#form-login - %h2#login_admin - = t('dynamics.admin.connexion_title') - - %br - %br - #new-user - = form_for @administrateur, url: { controller: 'administrateurs/sessions', action: :create } do |f| - %h4 - = f.label :email - .input-group - .input-group-addon - %span.fa.fa-user - = f.email_field :email, class: 'form-control' - %br - %h4 - = f.label 'Mot de passe' - .input-group - .input-group-addon - %span.fa.fa-asterisk - = f.password_field :password, class: 'form-control', value: @administrateur.password - %br - %br - .actions - = f.submit "Se connecter", class: 'btn btn-primary' - %br diff --git a/app/views/instructeurs/sessions/new.html.haml b/app/views/instructeurs/sessions/new.html.haml deleted file mode 100644 index ada6dabbb..000000000 --- a/app/views/instructeurs/sessions/new.html.haml +++ /dev/null @@ -1,26 +0,0 @@ -#form-login - %h2#instructeur_login Instructeur - - %br - %br - #new-user - = form_for @instructeur, url: instructeur_session_path, method: :post do |f| - %h4 - = f.label :email - .input-group - .input-group-addon - %span.fa.fa-user - = f.email_field :email, class: 'form-control' - %br - %h4 - = f.label :password - .input-group - .input-group-addon - %span.fa.fa-asterisk - = f.password_field :password, autocomplete: "off", class: 'form-control', value: @instructeur.password - %br - %br - .actions - = f.submit "Se connecter", class: 'btn btn-primary' - %br - = render "instructeurs/shared/links" diff --git a/app/views/users/sessions/new.html.haml b/app/views/users/sessions/new.html.haml index eb88d230a..9f7a9962f 100644 --- a/app/views/users/sessions/new.html.haml +++ b/app/views/users/sessions/new.html.haml @@ -1,32 +1,29 @@ = content_for(:page_id, 'auth') .auth-form.sign-in-form - - if resource_name == :user - %p.register - %span - Nouveau sur demarches‑simplifiees.fr ? - = link_to "Créer un compte", new_registration_path(resource_name), class: "button primary auth-signup-button" + %p.register + %span + Nouveau sur demarches‑simplifiees.fr ? + = link_to "Créer un compte", new_user_registration_path, class: "button primary auth-signup-button" - %hr + %hr - = form_for @user, url: user_session_path, html: { class: "form" } do |f| + = form_for User.new, url: user_session_path, html: { class: "form" } do |f| %h1 Connectez-vous = f.label :email, "Email" = f.text_field :email, autofocus: true = f.label :password, "Mot de passe" - = f.password_field :password, value: @user.password, placeholder: "8 caractères minimum" + = f.password_field :password, placeholder: "8 caractères minimum" .auth-options - - if devise_mapping.rememberable? - %div - = f.check_box :remember_me, as: :boolean - = f.label :remember_me, "Se souvenir de moi", class: 'remember-me' + %div + = f.check_box :remember_me, as: :boolean + = f.label :remember_me, "Se souvenir de moi", class: 'remember-me' - - if [:user, :instructeur].include?(resource_name) - .text-right - = link_to "Mot de passe oublié ?", new_password_path(resource_name), class: "link" + .text-right + = link_to "Mot de passe oublié ?", new_user_password_path, class: "link" = f.submit "Se connecter", class: "button large primary expand" diff --git a/spec/controllers/sessions/sessions_controller_spec.rb b/spec/controllers/sessions/sessions_controller_spec.rb deleted file mode 100644 index 06b4ec51c..000000000 --- a/spec/controllers/sessions/sessions_controller_spec.rb +++ /dev/null @@ -1,35 +0,0 @@ -require 'spec_helper' - -describe Sessions::SessionsController, type: :controller do - controller Sessions::SessionsController do - def create - render json: '' - end - end - - let(:user) { create(:user) } - - describe '#create' do - before do - @request.env["devise.mapping"] = Devise.mappings[:user] - end - - it 'calls before_sign_in' do - expect_any_instance_of(Sessions::SessionsController).to receive(:before_sign_in) - post :create - end - end - - describe '#create with user connected' do - before do - @request.env["devise.mapping"] = Devise.mappings[:user] - - allow_any_instance_of(described_class).to receive(:user_signed_in?).and_return(true) - end - - it 'calls sign out for user' do - expect_any_instance_of(described_class).to receive(:sign_out).with(:user) - post :create - end - end -end diff --git a/spec/controllers/users/activate_controller_spec.rb b/spec/controllers/users/activate_controller_spec.rb index 917b560be..01aeda490 100644 --- a/spec/controllers/users/activate_controller_spec.rb +++ b/spec/controllers/users/activate_controller_spec.rb @@ -17,4 +17,24 @@ describe Users::ActivateController, type: :controller do it { expect(controller).not_to have_received(:trust_device) } end end + + describe '#create' do + let!(:user) { create(:user) } + let(:token) { user.send(:set_reset_password_token) } + let(:password) { 'another-password-ok?' } + + before { post :create, params: { user: { reset_password_token: token, password: password } } } + + context 'when the token is ok' do + it { expect(user.reload.valid_password?(password)).to be true } + it { expect(response).to redirect_to(instructeur_procedures_path) } + end + + context 'when the token is bad' do + let(:token) { 'bad' } + + it { expect(user.reload.valid_password?(password)).to be false } + it { expect(response).to redirect_to(users_activate_path(token: token)) } + end + end end diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index 0a42fc26a..6f09382fa 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -9,71 +9,74 @@ describe Users::SessionsController, type: :controller do end describe '#create' do - context "when the user is also a instructeur and an administrateur" do - let!(:administrateur) { create(:administrateur, email: email, password: password) } - let(:instructeur) { administrateur.instructeur } - let(:user) { instructeur.user } - let(:trusted_device) { true } - let(:send_password) { password } + let(:user) { create(:user, email: email, password: password, loged_in_with_france_connect: 'particulier') } + let(:send_password) { password } + let(:remember_me) { '0' } - before do - allow(controller).to receive(:trusted_device?).and_return(trusted_device) - allow(InstructeurMailer).to receive(:send_login_token).and_return(double(deliver_later: true)) + subject do + post :create, params: { + user: { + email: email, + password: send_password, + remember_me: remember_me + } + } + end + + context 'when the credentials are right' do + it 'signs in' do + subject + + expect(response).to redirect_to(root_path) + expect(controller.current_user).to eq(user) + expect(user.reload.loged_in_with_france_connect).to be(nil) + expect(user.reload.remember_created_at).to be_nil end - subject do - post :create, params: { user: { email: email, password: send_password } } - user.reload - end + context 'when remember_me is specified' do + let(:remember_me) { '1' } - context 'when the device is not trusted' do - before do - Flipflop::FeatureSet.current.test!.switch!(:bypass_email_login_token, false) - end - let(:trusted_device) { false } - - it 'redirects to the send_linked_path' do + it 'remembers' do subject - expect(controller).to redirect_to(link_sent_path(email: user.email)) - - expect(controller.current_user).to eq(user) - expect(controller.current_instructeur).to eq(instructeur) - # WTF? - # expect(controller.current_administrateur).to eq(administrateur) - expect(user.loged_in_with_france_connect).to eq(nil) + expect(user.reload.remember_created_at).to be_present end end - context 'when the device is trusted' do - it 'signs in as user, instructeur and adminstrateur' do + context 'when a previous path was registered' do + let(:stored_path) { 'a_path' } + + before { controller.store_location_for(:user, stored_path) } + + it 'redirects to that previous path' do subject - expect(response.redirect?).to be(true) - expect(controller).not_to redirect_to link_sent_path(email: email) - # TODO when signing in as non-administrateur, and not starting a demarche, log in to instructeur path - # expect(controller).to redirect_to instructeur_procedures_path - - expect(controller.current_user).to eq(user) - expect(controller.current_instructeur).to eq(instructeur) - expect(controller.current_administrateur).to eq(administrateur) - expect(user.loged_in_with_france_connect).to be(nil) + expect(response).to redirect_to(stored_path) end end - context 'when the credentials are wrong' do - let(:send_password) { 'wrong_password' } + context 'when the user is locked' do + before { user.lock_access! } - it 'fails to sign in with bad credentials' do + it 'redirects to new_path' do subject - expect(response.unauthorized?).to be(true) - expect(controller.current_user).to be(nil) - expect(controller.current_instructeur).to be(nil) - expect(controller.current_administrateur).to be(nil) + expect(response).to render_template(:new) + expect(flash.alert).to eq(I18n.t('devise.failure.invalid')) end end end + + context 'when the credentials are wrong' do + let(:send_password) { 'wrong_password' } + + it 'fails to sign in with bad credentials' do + subject + + expect(response).to render_template(:new) + expect(controller.current_user).to be(nil) + end + end end describe '#destroy' do diff --git a/spec/features/sessions/sign_in_spec.rb b/spec/features/sessions/sign_in_spec.rb index 8c549c23f..958c3b7c2 100644 --- a/spec/features/sessions/sign_in_spec.rb +++ b/spec/features/sessions/sign_in_spec.rb @@ -13,6 +13,17 @@ feature 'Signin in:' do expect(page).to have_current_path dossiers_path end + scenario 'an existing user can lock its account' do + visit root_path + click_on 'Connexion' + + 5.times { sign_in_with user.email, 'bad password' } + expect(user.reload.access_locked?).to be false + + sign_in_with user.email, 'bad password' + expect(user.reload.access_locked?).to be true + end + context 'when visiting a procedure' do let(:procedure) { create :simple_procedure, :with_service }