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

feat(users): bulk lookup users by email #3720

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

cdriesler
Copy link
Member

Description & motivation

  • The invites flow reaches a point where it knows:
    • A collection of emails
    • Where it would like to invite them to
  • The current lookup allows searching one query => many user matches, but not many queries => any matches

Changes:

  • Adds usersByEmail query modeled after users query
  • Extracts common logic to lookupUsersBaseQuery
  • Adds tests for both lookups

@cdriesler cdriesler requested a review from alemagio December 18, 2024 19:04
@iainsproat
Copy link
Contributor

@hasServerRole(role: SERVER_GUEST)
@hasScopes(scopes: ["users:read", "profile:read"])

Can you remind me what this does; do they need to be logged in to use this?

Is there a limit on the number of emails which can be passed in? (I see there's a limit of 100 on the results)

What is to stop someone dumping in a list of known or guessed email addresses to app.speckle.systems and using that to confirm whether they are users or not? With the status quo of a single look up, they might hit up against our (generous) rate limits (as a fairly weak source of friction against abuse), but does this make it easier to abuse the system?

Do we need some sort of lookup limit or rate limit?

Copy link
Contributor

@iainsproat iainsproat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would like a clearer description in the PR of the security/confidentiality implications before we merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants