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

Using username instead of user_id in signer #48

Closed
itzmanish opened this issue Apr 29, 2019 · 8 comments
Closed

Using username instead of user_id in signer #48

itzmanish opened this issue Apr 29, 2019 · 8 comments

Comments

@itzmanish
Copy link
Contributor

I am trying to use username instead of user_id. But i am getting this error
File "/home/krishna/Projects/django/cms/account/views.py", line 79, in _calculate_salt if registration_settings.REGISTER_VERIFICATION_ONE_TIME_USE: File "/home/krishna/anaconda3/envs/django/lib/python3.7/site-packages/rest_registration/utils/nested_settings.py", line 38, in __getattr__ self=self, attr=attr)) AttributeError: Invalid REST_REGISTRATION setting: 'REGISTER_VERIFICATION_ONE_TIME_USE'
However this is not related to username or userid data but yet i am getting this error. I tried to trackdown this error in nested_settings.py and settings_field.py but there is no problem.

Also i am using whole view.py from
rest-registration/api/views/register.py

@apragacz
Copy link
Owner

Why do you want to use username instead of user_id in the signer? Currently Django REST Registration does not allow to customize the signing process in that way.

@itzmanish
Copy link
Contributor Author

itzmanish commented Apr 30, 2019

Beacuse i don't want to disclose user's id. Using username in verification mail is a better security concern instead of user-id. Also username should not be used in reset password link instead of this use email address to send reset password link.

@apragacz
Copy link
Owner

Beacuse i don't want to disclose user's id

OK, but could you provide your use case? I'm still not getting how revealing the user id could cause a problem.
It is assumed that the user can access it's user ID anyway from the profile endpoint (but ok, you can replace the profile serializer).
There is a possibility that someone else than the user may have access to the verification link and therefore to the user ID (This should not happen; but in case we have REGISTER_VERIFICATION_ONE_TIME_USE to at least to protect the the verification url for being reused)

Using username in verification mail is a better security concern instead of user-id

Could you explain what you mean by that? I understand that in some cases the username may be more exposed to the user (in the UI), but I don't see how identifying the user by username and identifying the user by user id is different in regards to security concerns. Actually, there may be cases where you allow the user to change the username - in this case I imagine identifying by username can pose potential security threat.

Also username should not be used in reset password link instead of this use email address to send reset password link

There is a similar problem like the one I described above: you can change the e-mail for a given user, but you can't change the user ID. So theoretically using a e-mail in reset password link could cause a possibility to reset a password for a different user (but it will probably not happen with RESET_PASSWORD_VERIFICATION_ONE_TIME_USE enabled).

@itzmanish
Copy link
Contributor Author

The main reason to not use user_id is this shows the primary key of user database. And i don't like to show my user how many users i have.

So theoretically using a e-mail in reset password link could cause a possibility to reset a password for a different user

I can't understand this. But what i can tell you in daily life people are used to remember email address but remembering username is bit hard.
What if someone forget their username (which is uses as login in sending reset password link) . Does django-rest-registration has any solution for this.

It is assumed that the user can access it's user ID anyway from the profile endpoint

How i can assume? whenever user signup for first time they will have the user id on their email. So basically user knows that what is their user id.

@apragacz
Copy link
Owner

apragacz commented May 6, 2019

... And i don't like to show my user how many users i have.

OK, that's fair point. One way to solve this would be to use UUID as your primary key, but I know some people may prefer to use integer IDs as internal primary keys and UUIDs (or whatever random key) as a external ID (with unique constraint in the DB).

I would still not use e-mail as user id (and this applies to username as well if it can be changed), reasons below.

I guess you convinced me. The problem I see is that the best solution I have in mind (which will not make DRR code messy and hard to read) includes changing user_id field in the signed data payload (this includes the verification link) to more general user to avoid confusion. This would make the code

I guess the initial solution could be leaving the name user_id but allow the user identification field to be changed, and then make the change user_id -> user in next major version (0.5.0?, 1.0.0?). Probably user_id should be deprecated first, and then removed altogether.

So theoretically using a e-mail in reset password link could cause a possibility to reset a password for a different user

I can't understand this.

Example (in case of username used for user identification):

  1. User with user_id=1 has username alice
  2. User with user_id=2 has username bob
  3. alice requests password reset (but does not click the link)
  4. alice changes username to aliceinwonderland
  5. bob changes username to alice
  6. now aliceinwonderland (originally alice, user_id=1) can change alice (originally bob, user_id=2) pasword using the reset password link

This is somewhat improbable scenario, but as you can see it's not impossible. In case of using e-mail as user identification this scenario alice would need to delete her e-mail account, and bob would need to create the e-mail account with the same address, which makes this case less probable, but still not impossible.

So again, I would argue that if you don't want to expose your user primary keys you should NOT use any field which may change for given user (e-mail, username if changeable). You should rather have another field which would be some random sequence (UUID or string of chars) which would never change.

@itzmanish
Copy link
Contributor Author

I would argue that if you don't want to expose your user primary keys you should NOT use any field which may change for given user

Well you are absolutely right. I have a simple solution for this in my mind. Why don't we ask for password when confirming email. DRR simply integrate username in verification hash and when someone confirm their email it will decode verification hash for username then it will verify with decoded username and password.

@apragacz apragacz self-assigned this May 13, 2019
apragacz added a commit that referenced this issue May 15, 2019
Added 'USER_VERIFICATION_ID_FIELD' setting key
apragacz added a commit that referenced this issue May 15, 2019
Added 'USER_VERIFICATION_ID_FIELD' setting key
apragacz added a commit that referenced this issue May 17, 2019
Added 'USER_VERIFICATION_ID_FIELD' setting key
apragacz added a commit that referenced this issue May 18, 2019
* Added 'USER_VERIFICATION_ID_FIELD' setting key
* Added tests
@apragacz
Copy link
Owner

Added 'USER_VERIFICATION_ID_FIELD' setting key to customize verification.
This feature will be released in next version (0.4.5).

apragacz added a commit that referenced this issue May 18, 2019
Changes:

* Resolved issue #48: Allowing to use custom user field for verification
* Resolved issue #46: added
  SEND_RESET_PASSWORD_LINK_SERIALIZER_USE_EMAIL setting
* Resolved issue #50: customizable send reset password link serializer
* Fixed reset password in case user is unverified and one-time use is enabled
@lock
Copy link

lock bot commented May 20, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants