generate a confirmation_token over an empty string

[ajsharp]

Fixes #5071

[ajsharp]

@mracos Any thoughts here? This should probably be considered a security issue, as it allows anyone to access the system with an empty token.

[mracos]

Sorry @ajsharp, I've been really busy with work lately, so I didn't have much time to take a deeper look at this but will try to give you some feedback in the next days.
Thanks!

[amangano-privy]

Would it be possible to add a validation to prevent the model from being saved when confirmation_token is an empty string?

[tegon]

@ajsharp Thanks for your contribution!

I was digging into this, and a better approach for us is to ensure it's not possible to confirm a user with a blank confirmation_token parameter - i.e. to return a validation error inside the ConfirmationsController#show action.
That should be enough to ensure no "invalid" records will be confirmed by mistake.

As for the confirmation_token being set to blank, that doesn't seem to be possible with the current code we have. I'm assuming something else could've happened in the system to let the records in that state. Unless we find a real case where Devise is setting the token to a blank string, I think it's not worth it to include more code to handle this.

@amangano-privy I thought about adding the validation, but we can't always require this field, so we would need a context validation. That would only protect the model against the known use cases. Another thing to take into account is that a bunch of places inside Devise uses validate: false.
So I think the validation wouldn't add that much value. The current code is already generating the token in a before_create callback, so I'm guessing it would require some external action to leave the field with a blank string - instead of nil.

WDYT you think? Thoughts?

[tegon]

Closed in favor of #5132