[identity] check if fid is already taken when registering new user
ClosedPublic
Actions

Authored by varun on Mar 27 2024, 1:28 PM.

Details

Reviewers

Commits

rCOMMc648927875bc: [identity] check if fid is already taken when registering new user

Summary

we shouldn't let multiple Comm users associate the same FID with their accounts. So, we need to check on registration if the FID is already taken.

Depends on D11411

Test Plan

tried registering a user with an FID that was already in the users table, it failed as expected. Registration with a new FID (not in DDB) continues to work, though.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

varun created this revision.Mar 27 2024, 1:28 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptMar 27 2024, 1:28 PM

varun added a child revision: D11413: [identity] on linkFarcasterUser we should return an error if the FID already exists for a different user in the users table.Mar 27 2024, 1:36 PM

Harbormaster completed remote builds in B27727: Diff 38390.Mar 27 2024, 1:49 PM

varun requested review of this revision.Mar 27 2024, 1:49 PM

@varun looking at the test plan here, is it fair to say you weren't able to reproduce ENG-7390? Is it worth trying to test via the UI to see if that reproduces it?

In D11412#329178, @ashoat wrote:

@varun looking at the test plan here, is it fair to say you weren't able to reproduce ENG-7390? Is it worth trying to test via the UI to see if that reproduces it?

I was able to repro, left some comments on the issue explaining what happened

Read the issue and seems like it was unrelated to this code.

Logic looks good, the only possible way it could fail is if somebody could trigger two registrations so quickly that the DDB index wouldn't have chance to be updated between them (it's milliseconds IIRC). But the only way to address this is to rate-limit registrations.

This revision is now accepted and ready to land.Mar 27 2024, 11:57 PM

Closed by commit rCOMMc648927875bc: [identity] check if fid is already taken when registering new user (authored by varun). · Explain WhyMar 28 2024, 3:17 PM

This revision was automatically updated to reflect the committed changes.

varun added a commit: rCOMMc648927875bc: [identity] check if fid is already taken when registering new user.