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

Support procs for sign_in_after_reset_password config #5653

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

excid3
Copy link
Contributor

@excid3 excid3 commented Nov 8, 2023

Using MFA with Devise, it would be great to support callable objects in the sign_in_after_reset_password config.

Example

config.sign_in_after_reset_password = ->(resource) { !resource.mfa_enabled? }

Originally, I had to override the passwords controller to change this behavior but it's much cleaner if Devise supports procs, lambdas, or any other callable object.

The resource is passed as an argument so this can be applied based upon user preferences.

@excid3 excid3 force-pushed the sign-in-after-reset-password-proc branch 2 times, most recently from cb65878 to 50e4ccd Compare November 8, 2023 17:59
@excid3 excid3 force-pushed the sign-in-after-reset-password-proc branch from 50e4ccd to 82a3d23 Compare November 8, 2023 18:36
Comment on lines +56 to +57
value = resource_class.sign_in_after_reset_password
value.respond_to?(:call) ? value.call(resource) : value
Copy link

Choose a reason for hiding this comment

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

wdyt about renaming the variable to setting or config?

Suggested change
value = resource_class.sign_in_after_reset_password
value.respond_to?(:call) ? value.call(resource) : value
setting = resource_class.sign_in_after_reset_password
setting.respond_to?(:call) ? setting.call(resource) : setting

test/integration/recoverable_test.rb Outdated Show resolved Hide resolved
Co-authored-by: Nuno Costa <nuno.mmc@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants