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.

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

Confirm OTP Token #72

Merged
merged 28 commits into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
a83a5dd
scaffold solution for confirming OTP before enabling OTP;
strouptl May 13, 2024
e3b929d
Add instructions to confirm_otp_token form;
strouptl May 13, 2024
f960474
move "Enable Authentication" form to separate "edit" view; reduce "sh…
strouptl May 13, 2024
01a402f
use existing edit/update actions on otp_tokens controller for confirm…
strouptl May 13, 2024
cc9067e
remove token explanation from show page;
strouptl May 15, 2024
fed4602
update flash message for failed confirmation;
strouptl May 15, 2024
fab7c1c
move locales for OTP confirmation form to edit_otp_tokens scope; dele…
strouptl May 15, 2024
7920bd3
differentiate title of show and edit pages; move "title" value for ed…
strouptl May 15, 2024
5ba39da
revert method name to enable_top!;
strouptl May 17, 2024
7074c2f
revert "h2" for otp_tokens#show to locale file;
strouptl May 17, 2024
2b3f893
use enable_link config locale in otp_tokens#show;
strouptl May 17, 2024
ffdf493
use locales for otp_token field and submit button; switch terminology…
strouptl May 17, 2024
ebd03ca
match terminology to AWS MFA form;
strouptl May 17, 2024
debe1c1
replace remaining reference to "Verification Code";
strouptl May 17, 2024
f30aa59
add tests for enabling two-factor authentication via dedicated otp_to…
strouptl May 18, 2024
0a9dc5a
update test helpers and initial sign_in test for new Enable Two-Facto…
strouptl May 18, 2024
8bc2b83
update otp_tokens#update to redirect to show action as before (rather…
strouptl May 18, 2024
074b67e
update disable test to confirm correct status displayed; remove accep…
strouptl May 18, 2024
1e94af0
update EnableOtpForm tests to reload user before checking whether OTP…
strouptl May 18, 2024
61f46a2
add populate_otp! method for populating initial secrets; add instruct…
strouptl May 19, 2024
e9215f4
update otp_tokens controller to populate otp secrets as needed; renam…
strouptl May 19, 2024
747cefa
update button text and warnings for disabling 2FA; remove instruction…
strouptl May 19, 2024
3293345
update tests for change; add otp_failed_attempts to destroy_otp_secre…
strouptl May 19, 2024
dfe9d45
rename destroy_otp_secrets! to clear_otp_fields! for consistency (sin…
strouptl May 19, 2024
0ca939f
simplify populate_otp_secrets! method;
strouptl May 19, 2024
51dca03
draft CHANGELOG insertion for requiring confirmation token and popula…
strouptl May 20, 2024
04fa4a7
rename "otp_token" input to "confirmation_code"; make edit_otp_token …
strouptl May 20, 2024
8cbc133
Update CHANGELOG.md to fix list indentation issue;
May 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
scaffold solution for confirming OTP before enabling OTP;
  • Loading branch information
strouptl committed May 18, 2024
commit a83a5ddd45ecb11541765158b375b5c805c35f5c
54 changes: 54 additions & 0 deletions app/controllers/devise_otp/devise/otp_confirm_tokens_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
module DeviseOtp
module Devise
class OtpConfirmTokensController < DeviseController
include ::Devise::Controllers::Helpers

prepend_before_action :ensure_credentials_refresh
prepend_before_action :authenticate_scope!

#
# Displays the OTP
#
def show
if resource.nil?
redirect_to stored_location_for(scope) || :root
else
render "devise/otp_confirm_tokens/show"
end
end

#
# Confirms the OTP if valid
#
def update
if resource.valid_otp_token?(params[:otp_token])
resource.confirm_otp!
otp_set_flash_message :success, :successfully_updated
redirect_to otp_credential_path_for(resource)
else
otp_set_flash_message :failure, :otp_token_does_not_match
render "devise/otp_confirm_tokens/show"
end
end

private

def ensure_credentials_refresh
ensure_resource!

if needs_credentials_refresh?(resource)
otp_set_flash_message :notice, :need_to_refresh_credentials
redirect_to refresh_otp_credential_path_for(resource)
end
end

def scope
resource_name.to_sym
end

def self.controller_path
"#{::Devise.otp_controller_path}/confirm_otp_tokens"
end
end
end
end
9 changes: 6 additions & 3 deletions app/controllers/devise_otp/devise/otp_tokens_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,14 @@ def show
#
def update
enabled = params[resource_name][:otp_enabled] == "1"
if enabled ? resource.enable_otp! : resource.disable_otp!
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think is the broken behaviour I am seeing. Before your change you could temporarily disable OTP and I think we should preserve the functionality for the time being.

if params[resource_name][:otp_enabled] == "1"
resource.reset_otp_credentials!
redirect_to confirm_otp_token_path_for(resource)
else
resource.disable_otp!
otp_set_flash_message :success, :successfully_updated
redirect_to action: :show
end

render :show
end

#
Expand Down
17 changes: 17 additions & 0 deletions app/views/devise/otp_confirm_tokens/show.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
Confirm the OTP Token shown below:

<%= otp_authenticator_token_image(resource) %>

<p>
<strong><%= I18n.t('manual_provisioning', :scope => 'devise.otp.token_secret') %>:</strong>
<code><%= resource.otp_auth_secret %></code>
</p>

<%= form_with(:url => [resource_name, :otp_confirm_token], :method => :put) do |f| %>

<%= f.label :otp_token %>
<%= f.text_field :otp_token %>

<%= f.submit :enable %>

<% end %>
5 changes: 5 additions & 0 deletions lib/devise_otp_authenticatable/controllers/url_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ def otp_token_path_for(resource_or_scope, opts = {})
send("#{scope}_otp_token_path", opts)
end

def confirm_otp_token_path_for(resource_or_scope, opts = {})
scope = ::Devise::Mapping.find_scope!(resource_or_scope)
send("#{scope}_otp_confirm_token_path", opts)
end

def otp_credential_path_for(resource_or_scope, opts = {})
scope = ::Devise::Mapping.find_scope!(resource_or_scope)
send("#{scope}_otp_credential_path", opts)
Expand Down
4 changes: 3 additions & 1 deletion lib/devise_otp_authenticatable/models/otp_authenticatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,13 @@ def reset_otp_persistence!
save!
end

def enable_otp!
def populate_otp!
if otp_persistence_seed.nil?
reset_otp_credentials!
end
end

def confirm_otp!
update!(otp_enabled: true, otp_enabled_on: Time.now)
end

Expand Down
3 changes: 3 additions & 0 deletions lib/devise_otp_authenticatable/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@ def devise_otp(mapping, controllers)
get :recovery
end

resource :confirm_token, only: [:show, :update], path: mapping.path_names[:confirm_token], controller: controllers[:otp_confirm_tokens]

resource :credential, only: [:show, :update],
path: mapping.path_names[:credentials], controller: controllers[:otp_credentials] do
get :refresh, action: "get_refresh"
put :refresh, action: "set_refresh"
end

end
end
end
Expand Down