Merge pull request #4210 from betagouv/cleaning_user_session

Nettoyage du Controller de Session

app/controllers/users/sessions_controller.rb

@@ -7,33 +7,15 @@ class Users::SessionsController < Devise::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,22 +73,4 @@ class Users::SessionsController < Devise::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
-      end
-    end
-    resource
-  end
-
-  def resource_locked?(resource)
-    resource.present? && resource.access_locked?
-  end
 end

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"
 

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

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 }