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.

Sign up for GitHub

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

Jump to bottom

ActiveRecord::RecordInvalid: Validation failed: Contact location has already been taken (ActiveRecord::RecordInvalid) #1809

Closed
Tracked by #118
sentry-io bot opened this issue Sep 7, 2023 · 2 comments
Closed
Tracked by #118

ActiveRecord::RecordInvalid: Validation failed: Contact location has already been taken (ActiveRecord::RecordInvalid) #1809

sentry-io bot opened this issue Sep 7, 2023 · 2 comments
Assignees
@zspencer
Labels
🐞 bug Something isn't working

Comments

@sentry-io
Copy link

sentry-io bot commented Sep 7, 2023 •
edited by zspencer

Sentry Issue: CONVENE-1Y

ActiveRecord::RecordInvalid: Validation failed: Contact location has already been taken (ActiveRecord::RecordInvalid)
  app/models/authentication_method.rb:56:in `bump_one_time_password!'
    update!(last_one_time_password_at: Time.zone.now)
  app/models/authentication_method.rb:60:in `send_one_time_password!'
    bump_one_time_password!
  app/models/authenticated_session.rb:39:in `save'
    authentication_method.send_one_time_password!(space)
  app/controllers/authenticated_sessions_controller.rb:13:in `create'
    if authenticated_session.save
  app/models/neighborhood/time_zone_mixin.rb:7:in `set_time_zone'
    Time.use_zone(ENV.fetch("NEIGHBORHOOD_TIME_ZONE"), &block)
...
(100 additional frame(s) were not displayed)
@anaulin anaulin closed this as completed Sep 7, 2023
@anaulin anaulin reopened this Sep 7, 2023
@anaulin anaulin mentioned this issue Sep 7, 2023
Closed
@zspencer zspencer self-assigned this Sep 21, 2023
@zspencer zspencer added the 🐞 bug Something isn't working label Sep 21, 2023
@zspencer zspencer mentioned this issue Oct 1, 2023
Open
6 tasks
zspencer added a commit that referenced this issue Oct 12, 2023
@zspencer
🐞🔨 Authentication: Don't explode when OTP or OTP secrets are nil
c4446ba 
- #1809

There's a couple things happening, one of which is if an
`AuthenticationMethod` does not have a One Time Password Secret, it
can't actually do the verification.

So i've added a check to make sure it returns false in cases when the
OTP secret has not been set yet; as well as when a nil OTP is provided.
@zspencer zspencer mentioned this issue Oct 12, 2023
Merged
zspencer added a commit that referenced this issue Oct 12, 2023
@zspencer
🐞🔨Authentication Prevent a condition when double-tapping sign-in
d8b04cf 
- #1809

OK, so my working assumption is that the bug is caused by two requests
being processed at the same time for a new email address, and the first
one creates the `AuthenticationMethod` and the second one *tries* to,
and fails; resulting in the validation error.

This fixes the racecondition at the controller level, but I think we
want to do it at in AuthenticatedSession#save instead...
zspencer added a commit that referenced this issue Oct 12, 2023
@zspencer
🐞🔨Authentication Prevent a condition when double-tapping sign-in
f931f7d 
- #1809

OK, so my working assumption is that the bug is caused by two requests
being processed at the same time for a new email address, and the first
one creates the `AuthenticationMethod` and the second one *tries* to,
and fails; resulting in the validation error.

This requeries the database and retries the rest of the work.

Would love to have names for methods to pull this out into, rather than
copy-pasting the method body but oh well.
@zspencer zspencer mentioned this issue Oct 12, 2023
Merged
@zspencer
Copy link
Member

OK, I think I've got this fixed. Once code review comes in I'll merge and mark resolved and move on with my life.

zspencer added a commit that referenced this issue Oct 12, 2023
@zspencer
🐞🔨 Authentication: Don't explode when OTP or OTP secrets are nil (#…
af354e9 
…1891)

- #1809

There's a couple things happening, one of which is if an
`AuthenticationMethod` does not have a One Time Password Secret, it
can't actually do the verification.

So i've added a check to make sure it returns false in cases when the
OTP secret has not been set yet; as well as when a nil OTP is provided.
zspencer added a commit that referenced this issue Oct 14, 2023
@zspencer
🐞🔨Authentication Prevent a condition when double-tapping sign-in
4cc94d3 
- #1809

OK, so my working assumption is that the bug is caused by two requests
being processed at the same time for a new email address, and the first
one creates the `AuthenticationMethod` and the second one *tries* to,
and fails; resulting in the validation error.

This requeries the database and retries the rest of the work.

Would love to have names for methods to pull this out into, rather than
copy-pasting the method body but oh well.
zspencer added a commit that referenced this issue Oct 14, 2023
@zspencer
🐞🔨Authentication Prevent race condition when double-tapping sign-in (…
8c6c880 
…#1892)

* 🐞🔨`Authentication` Prevent a condition when double-tapping sign-in

- #1809

OK, so my working assumption is that the bug is caused by two requests
being processed at the same time for a new email address, and the first
one creates the `AuthenticationMethod` and the second one *tries* to,
and fails; resulting in the validation error.

This requeries the database and retries the rest of the work.
@zspencer
Copy link
Member

OK, it's fixed! Wooo!

rosschapman pushed a commit that referenced this issue Oct 23, 2023
@zspencer @rosschapman
🐞🔨 Authentication: Don't explode when OTP or OTP secrets are nil
a395a0f 
- #1809

There's a couple things happening, one of which is if an
`AuthenticationMethod` does not have a One Time Password Secret, it
can't actually do the verification.

So i've added a check to make sure it returns false in cases when the
OTP secret has not been set yet; as well as when a nil OTP is provided.
rosschapman pushed a commit that referenced this issue Oct 23, 2023
@zspencer @rosschapman
🐞🔨Authentication Prevent race condition when double-tapping sign-in (…
060ccac 
…#1892)

* 🐞🔨`Authentication` Prevent a condition when double-tapping sign-in

- #1809

OK, so my working assumption is that the bug is caused by two requests
being processed at the same time for a new email address, and the first
one creates the `AuthenticationMethod` and the second one *tries* to,
and fails; resulting in the validation error.

This requeries the database and retries the rest of the work.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zspencer zspencer

Labels
🐞 bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@zspencer @anaulin