search

scambra / devise_invitable

An invitation strategy for devise
MIT License
2.66k stars 553 forks source link

invite! may lead to unpredictable behaviour under some conditions #878

Open jeromedoucet opened 2 years ago

jeromedoucet commented 2 years ago

Summary

If config.invite_key is not configured to ensure a strict row unicity, invite! may send invitation to a random user / account.

Details

In models.rb L311, the instruction : 

  invitable = find_or_initialize_with_errors(invite_key_array, attributes_hash)

Will take the first existing row. In most of DBMS (at least all relational one), without explicit clause the order is unpredictable. So if many rows match the underlying find_by* query, we can't predict which one will be chosen.

Expected behaviour

That's a situation devise_invitable can't handle. The current implementation may hide bugs on devise_invitable user's code base.

if many existing invitable row match, the invitation process must stop, through an exception (I believe).

I will submit a PR to change that behaviour soon :smiley:

FYI @TristanBelin @bakster-jv

scambra commented 2 years ago

I don't understand this issue. Why invite_key is not configured to be unique? Invite_key can be set to more than one column, in cases when one column, such as email, is not unique.

TristanBelin commented 2 years ago

I don't understand this issue. Why invite_key is not configured to be unique? Invite_key can be set to more than one column, in cases when one column, such as email, is not unique.

Yes it should be configured to a unique column, but it is technically possible to do otherwise And if we do so, devise_invitable proceed without any message / error even when multiple elements are returned, and this may go unnoticed and result in unexpected behavior