Skip to content

Commit

Permalink
TN-3097 confront random failures with 2FA tokens. (#4307)
Browse files Browse the repository at this point in the history
Immediately attempt to use code before emailing out - regenerate if it
doesn't work.

I'll note, via unit testing I could NOT find a reliable way to get this
to break before or after changing, despite many attempts and huge
iteration counts. Therefore, I can't be certain this fixes the issue,
but I was able to verify by hand that some of the codes emailed out do
NOT work against the otp_secret saved. This has a good chance of being
the problem.
  • Loading branch information
pbugni committed Apr 17, 2023
1 parent 130b539 commit cb54eb7
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 8 deletions.
29 changes: 23 additions & 6 deletions portal/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -487,14 +487,31 @@ def current_encounter(

def generate_otp(self):
"""Generate One Time Password for 2FA from user's otp_secret"""
if self.otp_secret is None:
self.otp_secret = generate_random_secret()

# having experienced random problems, regenerate if the token doesn't
# immediately work
secret = self.otp_secret or generate_random_secret()
while True:
token = otp.get_totp(
secret=secret,
token_length=self.TOTP_TOKEN_LEN,
interval_length=self.TOTP_TOKEN_LIFE)
if otp.valid_totp(
token=token,
secret=secret,
token_length=self.TOTP_TOKEN_LEN,
interval_length=self.TOTP_TOKEN_LIFE):
break
secret = generate_random_secret()

# persist if the secret had to be regenerated
if secret != self.otp_secret:
self.otp_secret = secret
db.session.commit()

return otp.get_totp(
self.otp_secret,
token_length=self.TOTP_TOKEN_LEN,
interval_length=self.TOTP_TOKEN_LIFE)
current_app.logger.info(
f"generated 2FA {secret}:{token} for user {self.id}")
return token

def validate_otp(self, token):
assert(self.otp_secret)
Expand Down
4 changes: 2 additions & 2 deletions portal/views/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ def validate_key(form, field):
user_id=user.id, subject_id=user.id)
else:
auditable_event(
message="FAILED 2FA verification", context="login",
user_id=user.id, subject_id=user.id)
message=f"FAILED 2FA verification {user.otp_secret}:{field}",
context="login", user_id=user.id, subject_id=user.id)
raise ValidationError("Invalid access code")


Expand Down

0 comments on commit cb54eb7

Please sign in to comment.