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

Hide hash and salt fields of user in register() #226

Merged
merged 4 commits into from
Aug 18, 2017

Conversation

guoyunhe
Copy link

Usually, in register() callback, you do not need salt and hash anymore. They should be hidden to avoid exposing to API.

Usually, in `register()` callback, you do not need salt and hash anymore. They should be hidden to avoid exposing to API.
Guo Yunhe added 3 commits August 18, 2017 10:43
After authentication, salt and hash are usually not used anymore. It is better to drop them to avoid exposing in `req.user`
@saintedlama saintedlama merged commit f928443 into saintedlama:master Aug 18, 2017
@resteinbock
Copy link

I think this breaks things...

@roblingle
Copy link

Was this reverted? Doesn't seem to be in master, and I'm still getting hash and salt in my session.

@davejm
Copy link

davejm commented Apr 4, 2018

@saintedlama authenticate and register both seem to expose the sensitive fields on the user object

@coldfire84
Copy link

Old thread...

I have a scenario where exposing hash/ salt is needed on register, change password and through another channel - I'm using the mqtt-auth-plug which requires passwords to be stored as outlined here: https://github.com/jpmens/mosquitto-auth-plug#passwords

At present, the register() and authenticate() functions return the hash, which is means I'm able to use this to generate the required hash format for MQTT auth to work.

If nullifying these fields becomes the default, can we have an option to expose them for scenarios such as the one outlined above? Or is there another way to get salt/ hash? findOne() for example does not return these either.

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.

6 participants