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
Allow looking up users by an email address #11562
Conversation
This does not pose any greater privacy or security risk than the currently available APIs, since they already allow finding a user by their email address, albeit much slower (list all users, iterate over users, request emails; possible to optimise by first checking users with usernames similar to their email addresses). Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
I'm not against this, but I'm also not entirely convinced. So when you write If we decide to go ahead and introduce this feature, it should be documented in the API documentation. We would also need an index on the |
We’re currently writing a tool to synchronise group membership with an external database, so when a (new) user logs in, they’re added to the corresponding groups. The upstream users database does not use usernames at all, but uses emails as the primary identifier; to keep the performance within safe limits, we need this extra API, otherwise as I outlined in the PR description, it’s way too tedious to extract the same information from the API. I must admit this works quite well without any additional indices. The current work-in-progress patchset for OmniAuth-based SSO uses This query is not expected to be used frequently anyway, so slower performance shouldn’t be a big issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm against to enable this for regular users. Only for admins please.
Can you explain the reason please? As I mentioned, this doesn't expose any more information than is already available through other API methods.
…On Wed, 1 Sep 2021, at 12:06, Henne Vogelsang wrote:
***@***.**** requested changes on this pull request.
I'm against to enable this for regular users. Only for admins please.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#11562 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACLQBKUWW3YDDBFFCGKN7LT7YCNFANCNFSM5DEHBDXA>.
Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
--
Cheers,
Andrej
|
It's making it too easy for my taste to check if some email address has an account. |
you can achieve the same via xpath search already /search/person?match=@email=... |
@@ -15,6 +15,8 @@ class PersonController < ApplicationController | |||
def show | |||
@list = if params[:prefix] | |||
User.where('login LIKE ?', params[:prefix] + '%') | |||
elsif params[:email] | |||
User.where(:email => params[:email]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrewshadura User.where(email: params[:email])
to make the linter happy
So yeah this is already (although undocumented) possible...
|
This patch adds an option to look up users by an email address. This is useful for example for tools manipulating or synchronising the user database with other sources.
The new API does not pose any greater privacy or security risk than the currently available APIs, since they already allow finding a user by their email address, albeit much slower (list all users, iterate over users, request emails; possible to optimise by first checking users with usernames similar to their email addresses).
Example of the usage: