Comments (8)
Some real nice detective work there. Thank you so much! I'll get a new release out soon
from passwordless.
Thank you for a thorough explanation – however, I am not totally sure what's going on.
Paranoid is for not telling the user whether a user record with their supplied email address exists. Isn't what you're describing here something else?
from passwordless.
Thank you for a thorough explanation – however, I am not totally sure what's going on.
Paranoid is for not telling the user whether a user record with their supplied email address exists. Isn't what you're describing here something else?
The upshot is that for an account that doesn't exist, Passwordless:
- Behaves as if it exists (correctly) by generating a session and associated ephemeral account...
- ...then accidentally creates the account by saving the should-be-ephemeral
authenticatable
as a side-effect of saving the session.
from passwordless.
Ohh, I get it. That's not good.
As a simple fix, I propose adding autosave: false to the association. Although, that may result in breaking the "paranoid" aspect of config.paranoid = true.
Breaking it how?
from passwordless.
Breaking it how?
So I haven't adequately tested it yet, but...
I believe that belongs_to
creates a validation that will fail when the authenticatable object isn't persisted. So then the if @session.save
statement will evaluate the else
clause.
The point of config.paranoid = true
is that whoever is inputting an email address into the form shouldn't be given any information about whether or not that email exists in our system (unless they can read email sent to that address). And evaluating the else
clause breaks that.
If that is how it works, that means that our workaround fixes the autosave problem but breaks the "paranoid" feature.
from passwordless.
So I haven't adequately tested it yet, but...
So, we just tested it and fortunately, it looks like this is not an issue! The Passwordless::Session
object is saved successfully, the authenticatable_type
field has been set, but the authenticatable_id
field is nil
. I think that the default rails belongs_to
presence validation works very simply and literally: it checks that session.authenticatable.present?
is truthy, but it does not check that it has been persisted, nor does it check that session.authenticatable_id.present?
is truthy.
from passwordless.
And after digging just a little bit deeper, I see that the code surrounding this has changed between Rails 7.0 and 7.1: See the docs for config.active_record.belongs_to_required_validates_foreign_key. The app I've been testing is running rails 7.1, but is still configured to use the rails 7.0 defaults.
But, if I'm understanding this correctly, this config setting doesn't actually make any difference to this issue. The relevant source code for it here: https://github.com/rails/rails/blob/9fd8b33ebba3c281c6cc5bbf8f48cde38c6fb0da/activerecord/lib/active_record/associations/builder/belongs_to.rb#L126-L141. And, athough the rails 7.1 default does add an :if
condition to the validation, it does not change the actual validation itself. It's still just model.requires_presence_of reflection.name
, i.e: requires_presence_of :authenticatable
, which is simply going to check that session.authenticatable.present?
. And Session.new(authenticatable: authenticatable_class.new(email:)).authenticatable.present?
will always be truthy.
In summary, I think it should be safe to simply set autosave: false
. :)
from passwordless.
And I see now that passwordless
also adds an explicit presence validation for :authenticatable
, but this will behave identically to the implicit belongs_to
validation so it doesn't change the analysis.
from passwordless.
Related Issues (20)
- Bug: Release has outdated source for router_helpers.rb HOT 6
- Feature Request: Instrumentation via `ActiveSupport::Instrumentation` HOT 1
- troubles upgrading to 1.0.0 HOT 3
- enumerate passwordless_sessions HOT 6
- app/views/passwordless/sessions/new.html.erb - label for does not match field ID HOT 1
- app/views/passwordless/sessions/show.html.erb HOT 2
- Error when registering new users HOT 3
- Bug: default_url_options[:host] is required for mailer HOT 6
- Embed redirect URL in magic link HOT 2
- error: Missing host to link to! Please provide the :host parameter HOT 1
- [DRAFT] readme proofreading HOT 6
- Add support for Turbo Native HOT 1
- autofocus in the show action view HOT 2
- current_user action controller HOT 3
- Configuring a locale in the url: /en/users/sign_in HOT 4
- Support scoped routes
- NoMethodError: undefined method `session' for nil HOT 12
- sign in via QR code 📸 HOT 3
- Add `after_session_confirm` hook after session confirmation HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from passwordless.