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

Create Users on password reset requests #6458

Closed
rjsparks opened this issue Oct 10, 2023 · 13 comments · Fixed by #7729
Closed

Create Users on password reset requests #6458

rjsparks opened this issue Oct 10, 2023 · 13 comments · Fixed by #7729

Comments

@rjsparks
Copy link
Member

We have people who for whatever reason had a Person record with no User record created decades ago, usually because they were a co-author on a draft. Now when they want to interact with the datatracker (using the same email that was associated with that Person decades ago), the password reset link says it sent something, but it didn't send anything because there was no User.

Instead, create a User in this situation. Tie in the logic that looks for possible duplicate persons and send the warning message to the secretariat (that code already exists).

The security posture of the result should be the same as a password reset for a Person that already has a User - proof of control of the email address.

(If that turns out to be problematic, change this issue to tell the person that they need to contact the secretariat to establish a new password instead).

@rjsparks
Copy link
Member Author

Currently, the behavior drives people, particularly during NomCom season, to create a new account with a different email address, making work for the secratariat/nomcom later to merge the Person records.

@rjsparks
Copy link
Member Author

The current code that creates Person records on draft submission is not creating corresponding User records.

@ThisIsMissEm
Copy link
Contributor

I think this may still be happening with new draft co-authors who don't already have datatracker accounts when their I-Ds are published.

I'm a co-author on https://www.ietf.org/archive/id/draft-parecki-oauth-client-id-metadata-document-00.html and data tracker is telling me I already have an account (I'm currently trying to register for Vancouver 120), when I try to reset password, I receive no email.

So I can't register because I already have an account and I can't reset the password.

@ThisIsMissEm
Copy link
Contributor

If I understand correctly, it'd be correct to modify both https://github.com/ietf-tools/datatracker/blob/main/ietf/ietfauth/views.py#L494-L495 and https://github.com/ietf-tools/datatracker/blob/main/ietf/submit/utils.py#L579 to fix this issue?

So in the reset handler (view?) if there's an email and there's a person, but there's no user record, create a user record for the user before sending reset; and in the submission process, when the Person is created also create a User record?

@ThisIsMissEm
Copy link
Contributor

For now I've gone with creating an account on a separate email for registration to IETF 120 instead.

@rjsparks
Copy link
Member Author

We'll need to correct what happened with your two datatracker accounts (and merge them back into one). Could you please send the usernames to [email protected] (don't post them here).

For addressing the code issue, yes, you're looking in the right places in the code. If you're attending in person, consider joining the codesprint on Saturday.

@ThisIsMissEm
Copy link
Contributor

Alrighty. Whilst it's possible I could write a patch for this, my python skills are quite rusty, so it may be better fixed by someone with more knowledge.

@ThisIsMissEm
Copy link
Contributor

I've just started to look at working on this, and I've a question:

Instead, create a User in this situation. Tie in the logic that looks for possible duplicate persons and send the warning message to the secretariat (that code already exists).

Is the goal here to show an error message like the one in create_account?

i.e.,

                # Either a User or an Email matching new_account_email is in the system but we do not have a
                # "good" email to use to contact its owner. Fail so the user can contact the secretariat to sort
                # things out.
                form.add_error(
                    "email",
                    ValidationError(
                        f"Unable to create account for {new_account_email}. Please contact "
                        f"the Secretariat at {settings.SECRETARIAT_SUPPORT_EMAIL} for assistance."
                    ),
                )

Or is the goal to actually create a User record for that known Email, and then to send a password reset to that freshly created User?

I think the latter would be more user friendly?

@ThisIsMissEm
Copy link
Contributor

I've done up a PR for the latter option described above: #7729

@rjsparks
Copy link
Member Author

Or is the goal to actually create a User record for that known Email, and then to send a password reset to that freshly created User?

Yes, that's the goal. And:

Tie in the logic that looks for possible duplicate persons and send the warning message to the secretariat (that code already exists)

Isn't about the error message you pointed to, but rather about a message to the secretariat asking them to inspect things when it looks like there might be a duplicate Person problem. See

if Person.objects.filter(name=self.name).count() > 1 :
msg = render_to_string('person/mail/possible_duplicates.txt',
dict(name=self.name,
persons=Person.objects.filter(name=self.name),
settings=settings
))
send_mail_preformatted(None, msg)
(which is in Person.save()).

ThisIsMissEm added a commit to ThisIsMissEm/ietf-datatracker that referenced this issue Jul 22, 2024
@ThisIsMissEm
Copy link
Contributor

Ah, okay; I'm not sure exactly where I should tie in the duplicate person check logic?

@rjsparks
Copy link
Member Author

Let's split that part out into separate work

@rjsparks
Copy link
Member Author

huh - I didn't intend to close this, reopening until the PR lands - sorry if that was confusing.

@rjsparks rjsparks reopened this Jul 22, 2024
rjsparks added a commit that referenced this issue Aug 7, 2024
…l and person, but no user. (#7729)

* fix: Send create user email for password resets where we have an email and person, but no user account

This fixes #6458

* fix: create User straight away and use nomral password reset

---------

Co-authored-by: Robert Sparks <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants