fix: Session controller to not generate auth tokens before mfa verification (#12487)
This PR is the fix for MFA changes, to not generate auth tokens without MFA verification in case MFA is enabled for the account --------- Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com>
This commit is contained in:
committed by
GitHub
parent
d762829519
commit
36cbd5745e
@@ -12,9 +12,11 @@ class DeviseOverrides::SessionsController < DeviseTokenAuth::SessionsController
|
||||
return handle_mfa_verification if mfa_verification_request?
|
||||
return handle_sso_authentication if sso_authentication_request?
|
||||
|
||||
super do |resource|
|
||||
return handle_mfa_required(resource) if resource&.mfa_enabled?
|
||||
end
|
||||
user = find_user_for_authentication
|
||||
return handle_mfa_required(user) if user&.mfa_enabled?
|
||||
|
||||
# Only proceed with standard authentication if no MFA is required
|
||||
super
|
||||
end
|
||||
|
||||
def render_create_success
|
||||
@@ -23,6 +25,17 @@ class DeviseOverrides::SessionsController < DeviseTokenAuth::SessionsController
|
||||
|
||||
private
|
||||
|
||||
def find_user_for_authentication
|
||||
return nil unless params[:email].present? && params[:password].present?
|
||||
|
||||
normalized_email = params[:email].strip.downcase
|
||||
user = User.from_email(normalized_email)
|
||||
return nil unless user&.valid_password?(params[:password])
|
||||
return nil unless user.active_for_authentication?
|
||||
|
||||
user
|
||||
end
|
||||
|
||||
def mfa_verification_request?
|
||||
params[:mfa_token].present?
|
||||
end
|
||||
@@ -59,10 +72,10 @@ class DeviseOverrides::SessionsController < DeviseTokenAuth::SessionsController
|
||||
@resource = user if user&.valid_sso_auth_token?(params[:sso_auth_token])
|
||||
end
|
||||
|
||||
def handle_mfa_required(resource)
|
||||
def handle_mfa_required(user)
|
||||
render json: {
|
||||
mfa_required: true,
|
||||
mfa_token: Mfa::TokenService.new(user: resource).generate_token
|
||||
mfa_token: Mfa::TokenService.new(user: user).generate_token
|
||||
}, status: :partial_content
|
||||
end
|
||||
|
||||
|
||||
65
lib/tasks/mfa.rake
Normal file
65
lib/tasks/mfa.rake
Normal file
@@ -0,0 +1,65 @@
|
||||
module MfaTasks
|
||||
def self.find_user_or_exit(email)
|
||||
abort 'Error: Please provide an email address' if email.blank?
|
||||
user = User.from_email(email)
|
||||
abort "Error: User with email '#{email}' not found" unless user
|
||||
user
|
||||
end
|
||||
|
||||
def self.reset_user_mfa(user)
|
||||
user.update!(
|
||||
otp_required_for_login: false,
|
||||
otp_secret: nil,
|
||||
otp_backup_codes: nil
|
||||
)
|
||||
end
|
||||
|
||||
def self.reset_single(args)
|
||||
user = find_user_or_exit(args[:email])
|
||||
abort "MFA is already disabled for #{args[:email]}" if !user.otp_required_for_login? && user.otp_secret.nil?
|
||||
reset_user_mfa(user)
|
||||
puts "✓ MFA has been successfully reset for #{args[:email]}"
|
||||
rescue StandardError => e
|
||||
abort "Error resetting MFA: #{e.message}"
|
||||
end
|
||||
|
||||
def self.reset_all
|
||||
print 'Are you sure you want to reset MFA for ALL users? This cannot be undone! (yes/no): '
|
||||
abort 'Operation cancelled' unless $stdin.gets.chomp.downcase == 'yes'
|
||||
|
||||
affected_users = User.where(otp_required_for_login: true).or(User.where.not(otp_secret: nil))
|
||||
count = affected_users.count
|
||||
abort 'No users have MFA enabled' if count.zero?
|
||||
|
||||
puts "\nResetting MFA for #{count} user(s)..."
|
||||
affected_users.find_each { |user| reset_user_mfa(user) }
|
||||
puts "✓ MFA has been reset for #{count} user(s)"
|
||||
end
|
||||
|
||||
def self.generate_backup_codes(args)
|
||||
user = find_user_or_exit(args[:email])
|
||||
abort "Error: MFA is not enabled for #{args[:email]}" unless user.otp_required_for_login?
|
||||
|
||||
service = Mfa::ManagementService.new(user: user)
|
||||
codes = service.generate_backup_codes!
|
||||
puts "\nNew backup codes generated for #{args[:email]}:"
|
||||
codes.each { |code| puts code }
|
||||
end
|
||||
end
|
||||
|
||||
namespace :mfa do
|
||||
desc 'Reset MFA for a specific user by email'
|
||||
task :reset, [:email] => :environment do |_task, args|
|
||||
MfaTasks.reset_single(args)
|
||||
end
|
||||
|
||||
desc 'Reset MFA for all users in the system'
|
||||
task reset_all: :environment do
|
||||
MfaTasks.reset_all
|
||||
end
|
||||
|
||||
desc 'Generate new backup codes for a user'
|
||||
task :generate_backup_codes, [:email] => :environment do |_task, args|
|
||||
MfaTasks.generate_backup_codes(args)
|
||||
end
|
||||
end
|
||||
@@ -40,6 +40,26 @@ RSpec.describe DeviseOverrides::SessionsController, type: :controller do
|
||||
expect(json_response['mfa_token']).to be_present
|
||||
end
|
||||
|
||||
it 'does not return authentication tokens before MFA verification' do
|
||||
post :create, params: { email: user.email, password: 'Test@123456' }
|
||||
|
||||
expect(response).to have_http_status(:partial_content)
|
||||
|
||||
# Check that no authentication headers are present
|
||||
expect(response.headers['access-token']).to be_nil
|
||||
expect(response.headers['uid']).to be_nil
|
||||
expect(response.headers['client']).to be_nil
|
||||
expect(response.headers['Authorization']).to be_nil
|
||||
|
||||
# Check that no bearer token is present in any form
|
||||
response.headers.each do |key, value|
|
||||
expect(value.to_s).not_to include('Bearer') if key.downcase.include?('auth')
|
||||
end
|
||||
|
||||
json_response = response.parsed_body
|
||||
expect(json_response['data']).to be_nil
|
||||
end
|
||||
|
||||
context 'when verifying MFA' do
|
||||
let(:mfa_token) { Mfa::TokenService.new(user: user).generate_token }
|
||||
|
||||
|
||||
Reference in New Issue
Block a user