Require email confirmation for TOTP-based logins #18689
Conversation
| def send_unrecognized_login_email(request, user, *, ip_address, user_agent, token): | ||
| return { | ||
| "username": user.username, | ||
| "ip_address": ip_address, |
There was a problem hiding this comment.
suggestion: include geo data as well
| user_service.update_user( | ||
| userid, last_totp_value=form.totp_value.data | ||
| ) |
There was a problem hiding this comment.
question: would it make sense to also update a unique_login.last_used or something like that when we see it again?
There was a problem hiding this comment.
I'm not sure what that would give us. Do we want to "expire" trusted devices after some period of time?
| ) | ||
| user: Mapped[User] = orm.relationship(back_populates="unique_logins") | ||
|
|
||
| ip_address: Mapped[str] = mapped_column(String, nullable=False) |
There was a problem hiding this comment.
question: Should this be its own string, or a relationship to warehouse.events.models.IpAddress?
There was a problem hiding this comment.
Given that that model is primarily used for banning IPs, I didn't want to overload it, but I think if we need to unify these in the future we could.
There was a problem hiding this comment.
When running a manual verification, the email sent was only text, not HTML. Any clue as to why?
There was a problem hiding this comment.
Are you talking about the ConsoleAndSMTPEmailSender in local dev? I don't think it displays HTML, only text:
warehouse/warehouse/email/services.py
Line 157 in f891447
There was a problem hiding this comment.
I am - one line above it is the note that you can visualize the HTML version in MailDev, which usually displays something richer for other emails.
Fixes #18425.
This PR maintains a record of device information across logins for each user: