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

ACCOUNT_PREVENT_ENUMERATION docs are unclear / confusing #3657

Closed
vskov147 opened this issue Feb 25, 2024 · 10 comments
Closed

ACCOUNT_PREVENT_ENUMERATION docs are unclear / confusing #3657

vskov147 opened this issue Feb 25, 2024 · 10 comments

Comments

@vskov147
Copy link

vskov147 commented Feb 25, 2024

Deal allauth team – first of all, a big Thank You to @pennersr and everyone involved for creating & maintaining an awesomely useful library.

This issue is in reference to this bit from https://docs.allauth.org/en/latest/account/configuration.html :

image

Feedback 1: "In case of mandatory verification, enumeration can be properly prevented because the case where an email address is already taken is indistinguishable from the case where it is not." – this is quite unclear / confusing to me. Consider the use-case:

  1. A new user goes through Sign Up using [email protected] email
  2. In the Sign Up form, they enter password1 and password2 as SecretPasswordExample
  3. They successfully complete the Sign Up process
  4. The user logs out
  5. Some time passes, the user has forgotten they Signed Up already, so the user visits the Sign Up form again
  6. The user attempts to sign up using the same [email protected] email for the second time
  7. However, this time, they enter password1 and password2 as NewDifferentSecretPassword

What is the user's password now? Is it the original SecretPasswordExample? Or is it the updated NewDifferentSecretPassword? I would postulate that steps 5 through 7 above with email already taken are indeed distinguishable from the case where the email address is available due to this password situation.

According to my understanding, either the Sign Up also acts as a built-in Password Change form but without email ownership verification (which would be a security hole) – or the Sign Up from steps 5 through 7 ignores the NewDifferentSecretPassword which is a different behavior from steps 1 through 3 (and potentially quite confusing to the user who has forgotten that they have Signed Up already)


Feedback 2: "When enumeration is set to True, email address uniqueness takes precedence over enumeration prevention, and the issue of multiple accounts having the same email address will be avoided, thus leaking information."

  • Doesn't this mean "When enumeration prevention is set to True, email address uniqueness takes precedence over enumeration prevention"? Enumeration itself being set to True is logically equivalent to Enumeration Prevention being set to False as I understand it.

Feedback 3: 'Set it to "strict" to allow for signups to go through.'

  • I can not find any other reference to a "strict" setting on this configuration page, so I am a bit confused whether this is telling me to set ACCOUNT_PREVENT_ENUMERATION to "strict" or is referring to some other setting? But ACCOUNT_PREVENT_ENUMERATION appears to be a Boolean flag and doesn't include a list of possible values. And if this is referring to a different setting, my suggestion is to explicitly mention which one.

I apologize in advance if my understanding of any of the above is incorrect – but my request is to clarify these bits in the documentation if possible.

PS: Happy to upstream a docs PR if it would be helpful – though I feel these docs should ultimately be written by someone more familiar with the allauth source-code than myself.

@pennersr
Copy link
Owner

pennersr commented Feb 25, 2024

When email verification is mandatory, the signup process always ends with sending an email. If the account already exists, the content of the email can explain that, so enumeration prevention is not an issue at all in this case.

When email verification is not mandatory, the signup process ends with a user being signed up and logged in. If you still want to have enumeration prevention in place, then the only way of achieving that is to allow signing up multiple times with the same email address, thus creating multiple accounts, all for the same (unverified) email address, each account with its own password.

Having multiple accounts with the same email is quite non-standard, so allauth doesn't opt for that unless you put it in strict mode. So it sacrifices enumeration prevention in favor of having accounts with unique email addresses in case it is not in strict mode.

Enumeration prevention is one setting that can be turned off (False), turned on but non-strict (True), and on and strict ("strict").

@vskov147
Copy link
Author

vskov147 commented Feb 25, 2024

Thank you @pennersr – this makes sense, and for what it's worth, what you just said in your comment above is a lot clearer than the currently-live docs. I would vote for updating the docs with a version of that note, but of course that's just my 2c.

I do have one outstanding follow-up question. Let's say I have email verification as mandatory. So, the signup process always ends with sending an email as you said. I understand that the content of the verification email can explain to the user that they just tried signing up with an already-existing email.

But just to confirm, what happens with regards to password in my above use-case of the same user going through the Sign Up flow twice with the same email but different passwords? Is the new password ignored?

@pennersr
Copy link
Owner

what happens with regards to password in my above use-case of the same user going through the Sign Up flow twice with the same email but different passwords?

I think that is already answered above... see the "thus creating multiple accounts"

@pennersr
Copy link
Owner

pennersr commented Mar 1, 2024

Closing -- please file a PR on the documentation if you see any way of making things more clear.

@pennersr pennersr closed this as completed Mar 1, 2024
@danryu
Copy link

danryu commented Apr 17, 2024

Having multiple accounts with the same email is quite non-standard, so allauth doesn't opt for that unless you put it in strict mode.

I don't have strict mode enabled, but this is the behavior that is happening for me. I have multiple accounts now with the same email address, and it breaks things...

I'm using 0.57.1 and this in settings.py:

ACCOUNT_EMAIL_REQUIRED = True
ACCOUNT_USERNAME_REQUIRED = True
ACCOUNT_AUTHENTICATION_METHOD = 'username_email'
ACCOUNT_EMAIL_VERIFICATION = 'optional'

@pennersr Can you please advise how I can configure to prevent email duplication (obviously with as much enumeration prevention as possible).

EDIT: I am also using dj-rest-auth/drf. And there's an open issue regarding this: iMerica/dj-rest-auth#617

@pennersr
Copy link
Owner

@danryu With just plain django-allauth these should be no duplicate email addresses. I do not know what happens when you add dj-rest-auth to the mix. You might want to give the feat-headless branch a try to use the new to be introduced builtin API support, see:

@danryu
Copy link

danryu commented May 2, 2024

* [Improve ease of integration for 3rd party REST/JSON APIs #360 (comment)](https://github.com/pennersr/django-allauth/issues/360#issuecomment-2028930361)

@pennersr Just checking this out now - this looks fantastic, I think I definitely want to adopt that.
Is there a TL;DR version (for overstretched backend engineers :) for how to migrate? I'm running vanilla allauth 0.57.1/0.61.1 with a slightly overridden AllauthPasswordResetForm, AllauthAdapter and SocialAccountAdapter.

@pennersr
Copy link
Owner

pennersr commented May 2, 2024

Migrate from... dj-rest-auth? It's a different API altogether -- see https://allauth.org/docs/draft-api/ -- so you will have to update your client code. Other than that, on the allauth side it should only be a matter of installing allauth via the feat-headless branch, and following the installation instructions: https://github.com/pennersr/django-allauth/blob/feat-headless/docs/headless/installation.rst

@danryu
Copy link

danryu commented May 2, 2024

I meant migrate from "normal" allauth, but the installation.rst answers that, thanks.

I'll take a look at the new API docs and see how much needs shifting. Feeling very positive about the cleaner design going forward.

PS the link at https://github.com/pennersr/django-allauth/blob/feat-headless/docs/headless/api.rst needs updating, currently 404

@pennersr
Copy link
Owner

pennersr commented May 2, 2024

@danryu The spec is now linked here: https://docs.allauth.org/en/dev/headless/api.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants