diff options
4 files changed, 144 insertions, 23 deletions
diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 9c73b39e2..7afd09e10 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -58,16 +58,20 @@ class Auth::SessionsController < Devise::SessionsController protected def find_user - if session[:attempt_user_id] + if user_params[:email].present? + find_user_from_params + elsif session[:attempt_user_id] User.find_by(id: session[:attempt_user_id]) - else - user = User.authenticate_with_ldap(user_params) if Devise.ldap_authentication - user ||= User.authenticate_with_pam(user_params) if Devise.pam_authentication - user ||= User.find_for_authentication(email: user_params[:email]) - user end end + def find_user_from_params + user = User.authenticate_with_ldap(user_params) if Devise.ldap_authentication + user ||= User.authenticate_with_pam(user_params) if Devise.pam_authentication + user ||= User.find_for_authentication(email: user_params[:email]) + user + end + def user_params params.require(:user).permit(:email, :password, :otp_attempt, :sign_in_token_attempt, credential: {}) end diff --git a/app/controllers/concerns/sign_in_token_authentication_concern.rb b/app/controllers/concerns/sign_in_token_authentication_concern.rb index cbee84569..384c5c50c 100644 --- a/app/controllers/concerns/sign_in_token_authentication_concern.rb +++ b/app/controllers/concerns/sign_in_token_authentication_concern.rb @@ -16,14 +16,18 @@ module SignInTokenAuthenticationConcern end def authenticate_with_sign_in_token - user = self.resource = find_user - - if user.present? && session[:attempt_user_id].present? && session[:attempt_user_updated_at] != user.updated_at.to_s - restart_session - elsif user_params.key?(:sign_in_token_attempt) && session[:attempt_user_id] - authenticate_with_sign_in_token_attempt(user) - elsif user.present? && user.external_or_valid_password?(user_params[:password]) - prompt_for_sign_in_token(user) + if user_params[:email].present? + user = self.resource = find_user_from_params + prompt_for_sign_in_token(user) if user&.external_or_valid_password?(user_params[:password]) + elsif session[:attempt_user_id] + user = self.resource = User.find_by(id: session[:attempt_user_id]) + return if user.nil? + + if session[:attempt_user_updated_at] != user.updated_at.to_s + restart_session + elsif user_params.key?(:sign_in_token_attempt) + authenticate_with_sign_in_token_attempt(user) + end end end diff --git a/app/controllers/concerns/two_factor_authentication_concern.rb b/app/controllers/concerns/two_factor_authentication_concern.rb index 909ab7717..2583d324b 100644 --- a/app/controllers/concerns/two_factor_authentication_concern.rb +++ b/app/controllers/concerns/two_factor_authentication_concern.rb @@ -35,16 +35,20 @@ module TwoFactorAuthenticationConcern end def authenticate_with_two_factor - user = self.resource = find_user + if user_params[:email].present? + user = self.resource = find_user_from_params + prompt_for_two_factor(user) if user&.external_or_valid_password?(user_params[:password]) + elsif session[:attempt_user_id] + user = self.resource = User.find_by(id: session[:attempt_user_id]) + return if user.nil? - if user.present? && session[:attempt_user_id].present? && session[:attempt_user_updated_at] != user.updated_at.to_s - restart_session - elsif user.webauthn_enabled? && user_params.key?(:credential) && session[:attempt_user_id] - authenticate_with_two_factor_via_webauthn(user) - elsif user_params.key?(:otp_attempt) && session[:attempt_user_id] - authenticate_with_two_factor_via_otp(user) - elsif user.present? && user.external_or_valid_password?(user_params[:password]) - prompt_for_two_factor(user) + if session[:attempt_user_updated_at] != user.updated_at.to_s + restart_session + elsif user.webauthn_enabled? && user_params.key?(:credential) + authenticate_with_two_factor_via_webauthn(user) + elsif user_params.key?(:otp_attempt) + authenticate_with_two_factor_via_otp(user) + end end end diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb index d03ae51e8..051a0807d 100644 --- a/spec/controllers/auth/sessions_controller_spec.rb +++ b/spec/controllers/auth/sessions_controller_spec.rb @@ -206,6 +206,38 @@ RSpec.describe Auth::SessionsController, type: :controller do end end + context 'using email and password after an unfinished log-in attempt to a 2FA-protected account' do + let!(:other_user) do + Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret(32)) + end + + before do + post :create, params: { user: { email: other_user.email, password: other_user.password } } + post :create, params: { user: { email: user.email, password: user.password } } + end + + it 'renders two factor authentication page' do + expect(controller).to render_template("two_factor") + expect(controller).to render_template(partial: "_otp_authentication_form") + end + end + + context 'using email and password after an unfinished log-in attempt with a sign-in token challenge' do + let!(:other_user) do + Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: false, current_sign_in_at: 1.month.ago) + end + + before do + post :create, params: { user: { email: other_user.email, password: other_user.password } } + post :create, params: { user: { email: user.email, password: user.password } } + end + + it 'renders two factor authentication page' do + expect(controller).to render_template("two_factor") + expect(controller).to render_template(partial: "_otp_authentication_form") + end + end + context 'using upcase email and password' do before do post :create, params: { user: { email: user.email.upcase, password: user.password } } @@ -231,6 +263,21 @@ RSpec.describe Auth::SessionsController, type: :controller do end end + context 'using a valid OTP, attempting to leverage previous half-login to bypass password auth' do + let!(:other_user) do + Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: false, current_sign_in_at: 1.month.ago) + end + + before do + post :create, params: { user: { email: other_user.email, password: other_user.password } } + post :create, params: { user: { email: user.email, otp_attempt: user.current_otp } }, session: { attempt_user_updated_at: user.updated_at.to_s } + end + + it "doesn't log the user in" do + expect(controller.current_user).to be_nil + end + end + context 'when the server has an decryption error' do before do allow_any_instance_of(User).to receive(:validate_and_consume_otp!).and_raise(OpenSSL::Cipher::CipherError) @@ -380,6 +427,52 @@ RSpec.describe Auth::SessionsController, type: :controller do end end + context 'using email and password after an unfinished log-in attempt to a 2FA-protected account' do + let!(:other_user) do + Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret(32)) + end + + before do + post :create, params: { user: { email: other_user.email, password: other_user.password } } + post :create, params: { user: { email: user.email, password: user.password } } + end + + it 'renders sign in token authentication page' do + expect(controller).to render_template("sign_in_token") + end + + it 'generates sign in token' do + expect(user.reload.sign_in_token).to_not be_nil + end + + it 'sends sign in token e-mail' do + expect(UserMailer).to have_received(:sign_in_token) + end + end + + context 'using email and password after an unfinished log-in attempt with a sign-in token challenge' do + let!(:other_user) do + Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: false, current_sign_in_at: 1.month.ago) + end + + before do + post :create, params: { user: { email: other_user.email, password: other_user.password } } + post :create, params: { user: { email: user.email, password: user.password } } + end + + it 'renders sign in token authentication page' do + expect(controller).to render_template("sign_in_token") + end + + it 'generates sign in token' do + expect(user.reload.sign_in_token).to_not be_nil + end + + it 'sends sign in token e-mail' do + expect(UserMailer).to have_received(:sign_in_token).with(user, any_args) + end + end + context 'using a valid sign in token' do before do user.generate_sign_in_token && user.save @@ -395,6 +488,22 @@ RSpec.describe Auth::SessionsController, type: :controller do end end + context 'using a valid sign in token, attempting to leverage previous half-login to bypass password auth' do + let!(:other_user) do + Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: false, current_sign_in_at: 1.month.ago) + end + + before do + user.generate_sign_in_token && user.save + post :create, params: { user: { email: other_user.email, password: other_user.password } } + post :create, params: { user: { email: user.email, sign_in_token_attempt: user.sign_in_token } }, session: { attempt_user_updated_at: user.updated_at.to_s } + end + + it "doesn't log the user in" do + expect(controller.current_user).to be_nil + end + end + context 'using an invalid sign in token' do before do post :create, params: { user: { sign_in_token_attempt: 'wrongotp' } }, session: { attempt_user_id: user.id, attempt_user_updated_at: user.updated_at.to_s } |