-
-
Notifications
You must be signed in to change notification settings - Fork 0
Form helpers should not be the ones storing the challenge in the session #73
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
Merged
nicolastemciuc
merged 22 commits into
master
from
temciuc--not-store-the-challenge-in-helper
Jan 22, 2026
Merged
Changes from 15 commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
a522711
feat: migrate to use options_for_get
nicolastemciuc 673ba40
feat: migrate to use options_for_create
nicolastemciuc 6167f3b
fix: rubocop offenses
nicolastemciuc 7c1b63c
test: call new endpoints to set the challenge in the session
nicolastemciuc c886e27
test: options_for_get and options_for_create endpoints
nicolastemciuc 6a5d083
test: set authentication factors
nicolastemciuc d5dd6a6
chore: fetch options endpoints from javascript
nicolastemciuc ebc2e8f
fix: ensure an authenticated user when upgrading security key
nicolastemciuc 5eb888d
Merge branch 'master' into temciuc--not-store-the-challenge-in-helper
nicolastemciuc 75857db
Merge remote-tracking branch 'origin/master' into temciuc--not-store-…
nicolastemciuc 983d639
style(rubocop): disable `Metrics/ModuleLength` cop
nicolastemciuc 5e231a1
test: assert current paths
joaquintomas2003 d94c938
feat: follow REST standard for getting passkey options
nicolastemciuc 5523100
feat: follow REST standard for getting security key options
nicolastemciuc 40a09c9
Merge branch 'master' into temciuc--not-store-the-challenge-in-helper
nicolastemciuc 7da06bb
Merge remote-tracking branch 'origin/master' into temciuc--not-store-…
nicolastemciuc ceccb3e
chore: scope options controllers under passkey and security_key
nicolastemciuc 5322c74
Merge remote-tracking branch 'origin/master' into temciuc--not-store-…
nicolastemciuc be02455
test: set the options in the session
nicolastemciuc 8c5ede6
chore: allow generating new controllers
nicolastemciuc 81d4261
chore: revert scoping controllers
nicolastemciuc d0ee47d
docs: update CHANGELOG
nicolastemciuc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
17 changes: 17 additions & 0 deletions
17
app/controllers/devise/passkey_authentication_options_controller.rb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Devise | ||
| class PasskeyAuthenticationOptionsController < DeviseController | ||
| def index | ||
| passkey_options = | ||
| WebAuthn::Credential.options_for_get( | ||
| user_verification: "required" | ||
| ) | ||
|
|
||
| # Store challenge in session for later verification | ||
| session[:authentication_challenge] = passkey_options.challenge | ||
|
|
||
| render json: passkey_options | ||
| end | ||
| end | ||
| end |
41 changes: 41 additions & 0 deletions
41
app/controllers/devise/passkey_registration_options_controller.rb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Devise | ||
| class PasskeyRegistrationOptionsController < DeviseController | ||
| before_action :authenticate_scope! | ||
|
|
||
| def index | ||
| passkey_options = | ||
| WebAuthn::Credential.options_for_create( | ||
| user: { | ||
| id: resource.webauthn_id, | ||
| name: resource_human_palatable_identifier | ||
| }, | ||
| exclude: resource.passkeys.pluck(:external_id), | ||
| authenticator_selection: { | ||
| resident_key: "required", | ||
| user_verification: "required" | ||
| } | ||
| ) | ||
|
|
||
| # Store challenge in session for later verification | ||
| session[:webauthn_challenge] = passkey_options.challenge | ||
|
|
||
| render json: passkey_options | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def authenticate_scope! | ||
| send(:"authenticate_#{resource_name}!", force: true) | ||
| self.resource = send(:"current_#{resource_name}") | ||
| end | ||
|
|
||
| def resource_human_palatable_identifier | ||
| authentication_keys = resource.class.authentication_keys | ||
| authentication_keys = authentication_keys.keys if authentication_keys.is_a?(Hash) | ||
|
|
||
| authentication_keys.filter_map { |authentication_key| resource.public_send(authentication_key) }.first | ||
| end | ||
| end | ||
| end | ||
26 changes: 26 additions & 0 deletions
26
app/controllers/devise/security_key_authentication_options_controller.rb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Devise | ||
| class SecurityKeyAuthenticationOptionsController < DeviseController | ||
| before_action :set_resource | ||
|
|
||
| def index | ||
| security_key_authentication_options = | ||
| WebAuthn::Credential.options_for_get( | ||
| allow: @resource.webauthn_credentials.pluck(:external_id), | ||
| user_verification: "discouraged" | ||
| ) | ||
|
|
||
| # Store challenge in session for later verification | ||
| session[:two_factor_authentication_challenge] = security_key_authentication_options.challenge | ||
|
|
||
| render json: security_key_authentication_options | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def set_resource | ||
| @resource = resource_class.find(session[:current_authentication_resource_id]) | ||
| end | ||
| end | ||
| end |
41 changes: 41 additions & 0 deletions
41
app/controllers/devise/security_key_registration_options_controller.rb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Devise | ||
| class SecurityKeyRegistrationOptionsController < DeviseController | ||
| before_action :authenticate_scope! | ||
|
|
||
| def index | ||
| create_security_key_options = | ||
| WebAuthn::Credential.options_for_create( | ||
| user: { | ||
| id: resource.webauthn_id, | ||
| name: resource_human_palatable_identifier | ||
| }, | ||
| exclude: resource.webauthn_credentials.pluck(:external_id), | ||
| authenticator_selection: { | ||
| resident_key: "discouraged", | ||
| user_verification: "discouraged" | ||
| } | ||
| ) | ||
|
|
||
| # Store challenge in session for later verification | ||
| session[:webauthn_challenge] = create_security_key_options.challenge | ||
|
|
||
| render json: create_security_key_options | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def authenticate_scope! | ||
| send(:"authenticate_#{resource_name}!", force: true) | ||
| self.resource = send(:"current_#{resource_name}") | ||
| end | ||
|
|
||
| def resource_human_palatable_identifier | ||
| authentication_keys = resource.class.authentication_keys | ||
| authentication_keys = authentication_keys.keys if authentication_keys.is_a?(Hash) | ||
|
|
||
| authentication_keys.filter_map { |authentication_key| resource.public_send(authentication_key) }.first | ||
| end | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8 changes: 8 additions & 0 deletions
8
...ors/devise/webauthn/templates/controllers/passkey_authentication_options_controller.rb.tt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| class <%= @scope_prefix %>PasskeyAuthenticationOptionsController < Devise::PasskeyAuthenticationOptionsController | ||
| # GET /resource/passkey_authentication_options | ||
| # def index | ||
| # super | ||
| # end | ||
| end |
8 changes: 8 additions & 0 deletions
8
...ators/devise/webauthn/templates/controllers/passkey_registration_options_controller.rb.tt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| class <%= @scope_prefix %>PasskeyRegistrationOptionsController < Devise::PasskeyRegistrationOptionsController | ||
| # GET /resource/passkey_registration_options | ||
| # def index | ||
| # super | ||
| # end | ||
| end |
8 changes: 8 additions & 0 deletions
8
...evise/webauthn/templates/controllers/security_key_authentication_options_controller.rb.tt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| class <%= @scope_prefix %>SecurityKeyAuthenticationOptionsController < Devise::SecurityKeyAuthenticationOptionsController | ||
| # GET /resource/security_key_authentication_options | ||
| # def index | ||
| # super | ||
| # end | ||
| end |
8 changes: 8 additions & 0 deletions
8
.../devise/webauthn/templates/controllers/security_key_registration_options_controller.rb.tt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| class <%= @scope_prefix %>SecurityKeyRegistrationOptionsController < Devise::SecurityKeyRegistrationOptionsController | ||
| # GET /resource/securiy_key_registration_options | ||
| # def index | ||
| # super | ||
| # end | ||
| end |
28 changes: 28 additions & 0 deletions
28
spec/requests/devise/passkey_authentication_options_controller_spec.rb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require "spec_helper" | ||
|
|
||
| RSpec.describe Devise::PasskeyAuthenticationOptionsController, type: :request do | ||
| describe "GET #index" do | ||
| it "stores the challenge in session and returns it as json" do | ||
| get account_passkey_authentication_options_path | ||
|
|
||
| expect(response).to have_http_status(:ok) | ||
|
|
||
| json = response.parsed_body | ||
| expect(json["challenge"]).to be_present | ||
| expect(session[:authentication_challenge]).to eq(json["challenge"]) | ||
| end | ||
|
|
||
| it "generates a new challenge on each request" do | ||
| get account_passkey_authentication_options_path | ||
| first_challenge = session[:authentication_challenge] | ||
|
|
||
| get account_passkey_authentication_options_path | ||
| second_challenge = session[:authentication_challenge] | ||
|
|
||
| expect(first_challenge).to be_present | ||
| expect(second_challenge).not_to eq(first_challenge) | ||
| end | ||
| end | ||
| end |
45 changes: 45 additions & 0 deletions
45
spec/requests/devise/passkey_registration_options_controller_spec.rb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require "spec_helper" | ||
|
|
||
| RSpec.describe Devise::PasskeyRegistrationOptionsController, type: :request do | ||
| let(:user) { Account.create!(email: "test@example.com", password: "password123") } | ||
|
|
||
| describe "GET #index" do | ||
| context "when user is not authenticated" do | ||
| it "redirects to the sign-in page" do | ||
| get account_passkey_registration_options_path | ||
| expect(response).to redirect_to(new_account_session_path) | ||
| end | ||
| end | ||
|
|
||
| context "when user is authenticated" do | ||
| before do | ||
| sign_in user, scope: :account | ||
| end | ||
|
|
||
| it "returns webauthn create options as json and stores the challenge in session" do | ||
| get account_passkey_registration_options_path | ||
|
|
||
| expect(response).to have_http_status(:ok) | ||
|
|
||
| json = response.parsed_body | ||
| expect(json["challenge"]).to be_present | ||
|
|
||
| expect(session[:webauthn_challenge]).to eq(json["challenge"]) | ||
| end | ||
|
|
||
| it "generates a new challenge on each request" do | ||
| get account_passkey_registration_options_path | ||
| first_challenge = session[:webauthn_challenge] | ||
|
|
||
| get account_passkey_registration_options_path | ||
| second_challenge = session[:webauthn_challenge] | ||
|
|
||
| expect(first_challenge).to be_present | ||
| expect(second_challenge).to be_present | ||
| expect(second_challenge).not_to eq(first_challenge) | ||
| end | ||
| end | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
36 changes: 36 additions & 0 deletions
36
spec/requests/devise/security_key_authentication_options_controller_spec.rb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require "spec_helper" | ||
|
|
||
| RSpec.describe Devise::SecurityKeyAuthenticationOptionsController, type: :request do | ||
| let(:user) { Account.create!(email: "test@example.com", password: "password123") } | ||
|
|
||
| describe "GET #index" do | ||
| before do | ||
| user.passkeys.create!( | ||
| external_id: "external-id", | ||
| name: "My Passkey", | ||
| public_key: "public-key", | ||
| sign_count: 0 | ||
| ) | ||
|
|
||
| post account_session_path, params: { | ||
| account: { | ||
| email: user.email, | ||
| password: "password123" | ||
| } | ||
| } | ||
| end | ||
|
|
||
| it "returns authentication options and stores the challenge in the session" do | ||
| get account_security_key_authentication_options_path | ||
|
|
||
| expect(response).to have_http_status(:ok) | ||
|
|
||
| body = response.parsed_body | ||
| expect(body).to include("challenge") | ||
|
|
||
| expect(session[:two_factor_authentication_challenge]).to eq(body["challenge"]) | ||
| end | ||
| end | ||
| end |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.