Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Switch webauthn_credentials.any? and .present? to be webauthn_enabled? #3867

Merged
merged 1 commit into from
Jun 16, 2023
Merged

Switch webauthn_credentials.any? and .present? to be webauthn_enabled? #3867

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/api/v1/api_keys_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def check_mfa(user)
return render_mfa_strong_level_required_error if user.mfa_required_weak_level_enabled?

yield
elsif user&.mfa_enabled? || user&.webauthn_credentials.present?
elsif user&.mfa_enabled?
prompt_text = otp.present? ? t(:otp_incorrect) : t(:otp_missing)
render plain: prompt_text, status: :unauthorized
else
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/webauthn_verifications_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class Api::V1::WebauthnVerificationsController < Api::BaseController
before_action :authenticate_with_credentials

def create
if @user.webauthn_credentials.present?
if @user.webauthn_enabled?
verification = @user.refresh_webauthn_verification
webauthn_path = webauthn_verification_url(verification.path_token)
respond_to do |format|
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/email_confirmations_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def create
end

def update
if @user.mfa_enabled? || @user.webauthn_credentials.any?
if @user.mfa_enabled?
setup_mfa_authentication
setup_webauthn_authentication(form_url: webauthn_update_email_confirmations_url(token: @user.confirmation_token))

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/passwords_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class PasswordsController < Clearance::PasswordsController
after_action :delete_mfa_expiry_session, only: %i[mfa_edit webauthn_edit]

def edit
if @user.mfa_enabled? || @user.webauthn_credentials.any?
if @user.mfa_enabled?
setup_mfa_authentication
setup_webauthn_authentication(form_url: webauthn_edit_user_password_url(token: @user.confirmation_token))

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/sessions_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class SessionsController < Clearance::SessionsController
def create
@user = find_user

if @user && (@user.mfa_enabled? || @user.webauthn_credentials.any?)
if @user&.mfa_enabled?
setup_webauthn_authentication(form_url: webauthn_create_session_path, session_options: { "user" => @user.id })
setup_mfa_authentication

Expand Down
4 changes: 2 additions & 2 deletions app/models/concerns/user_multifactor_methods.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module UserMultifactorMethods
end

def mfa_enabled?
if webauthn_credentials.present? && mfa_disabled?
if webauthn_enabled? && mfa_disabled?
self.mfa_level = :ui_and_gem_signin
save!(validate: false)
end
Comment on lines +12 to 15
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can also be removed once the rake task is run.

Expand All @@ -26,7 +26,7 @@ def no_mfa_devices?
end

def mfa_gem_signin_authorized?(otp)
return true unless strong_mfa_level? || webauthn_credentials.present?
return true unless strong_mfa_level? || webauthn_enabled?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return true unless strong_mfa_level? || webauthn_enabled?
return true unless strong_mfa_level?

This can be removed once the rake task to migrate all webauthn only users is run.

api_mfa_verified?(otp)
end

Expand Down
2 changes: 1 addition & 1 deletion app/views/multifactor_auths/mfa_prompt.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<% @title = t("multifactor_authentication") %>

<div class="mfa__container">
<% if @user.webauthn_credentials.any?%>
<% if @user.webauthn_enabled?%>
<div class="mfa__option">
<h2 class="page__subheading--block"> <%= t("sessions.prompt.security_device") %></h2>
<div class="t-body">
Expand Down
2 changes: 1 addition & 1 deletion app/views/sessions/prompt.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<% @title = t('multifactor_authentication') %>

<div class="mfa__container">
<% if @user.webauthn_credentials.any? %>
<% if @user.webauthn_enabled? %>
<div class="mfa__option">
<h2 class="page__subheading--block"> <%= t(".security_device") %></h2>
<div class="t-body">
Expand Down
2 changes: 1 addition & 1 deletion app/views/webauthn_verifications/prompt.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<% @title = t(".title")%>

<% if @user.webauthn_credentials.any? %>
<% if @user.webauthn_enabled? %>
<div class="t-body">
<% if browser.safari? %>
<p class="l-text-red-600" ><%= t(".safari_note_html") %></p>
Expand Down