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

incorporate haveibeenpwned.com #186

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

incorporate haveibeenpwned.com #186

wants to merge 1 commit into from

Conversation

lambdaTotoro
Copy link
Collaborator

This would close #71 if we decide to go forward with this. I'd like to hear your thoughts on this, @consideRatio. The code itself is simple enough, just a few lines and a library call. But of course we're introducing a dependency not only on the library, but also on haveibeenpwned.com for password validation.

On the other hand, I do think it will give users more secure passwords because this catches a lot of passwords that our static text file can't and doesn't.

What do you think?

@lambdaTotoro lambdaTotoro added the enhancement New feature or request label Nov 3, 2021
@lambdaTotoro lambdaTotoro marked this pull request as draft November 3, 2021 12:20
@manics
Copy link
Member

manics commented Nov 3, 2021

How does this behave if used in a restricted (limited outbound network) environment?

@consideRatio
Copy link
Member

I think this is a cool feature, but since its sending out password to via a library to a third party, I'm not feeling very comfortable. I'd like to have quite a few voices to weigh in about this feature in general.

  1. Is it okay for the JupyterHub admin to declare all passwords should be checked like this against haveibeenpwned?
    If an admin has done this, will we need to notify the user before they type in their password that the password will be hashed and checked against the haveibeenpwned database?
  2. What kind of UX do we expose?
    • Admin's sets a boolean flag to enable/disable an automatic and required successful a check against haveibeenpwned?
    • Admin's sets a boolean flag to enable/disable an automatic and not required successful a check against haveibeenpwned, which just provides the user a warning?
    • Users opt in to check their password before locking into it with some button?
  3. What do we do if this feature is enabled, but haveibeenpwned is down?

@lambdaTotoro
Copy link
Collaborator Author

@consideRatio: [...] its sending out password to via a library to a third party [...]

I don't feel that this description does justice to what is actually happening. If you take a look at the library, it makes use of haveibeenpwned.com's k-anonymity. In other words, neither the password, nor even the complete hash ever get transmitted anywhere. Only prefixes of the first 5 playes of the SHA-1 hash do. The reply is then a bucket of suffixes with an amount attached to each of them, to be locally checked. The amount is then returned. We'd error on anything > 0.

@consideRatio: Is it okay for the JupyterHub admin to declare all passwords should be checked like this against haveibeenpwned?

I think as an opt-in option, that would be okay.

@consideRatio: If an admin has done this, will we need to notify the user before they type in their password that the password will be hashed and checked against the haveibeenpwned database?

I wouldn't think that necessary. We already compare against a subset of that now, without explicitly making the user aware that the password will be checked.

@consideRatio: Admin's sets a boolean flag to enable/disable an automatic and required successful a check against haveibeenpwned?

This is what I had in mind, yes. I am also open to other approaches, though.

@manics: How does this behave if used in a restricted (limited outbound network) environment?
@consideRatio: What do we do if this feature is enabled, but haveibeenpwned is down?

My intuition would be that if the feature is enabled, but the service can't be reached, it defaults to the usual check against common passwords we already have.

@consideRatio
Copy link
Member

I don't feel that this description does justice to what is actually happening. If you take a look at the library, it makes use of haveibeenpwned.com's k-anonymity. In other words, neither the password, nor even the complete hash ever get transmitted anywhere. Only prefixes of the first 5 playes of the SHA-1 hash do.

Wieeeeeeeee! Then yes yes yes! :D

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

Successfully merging this pull request may close these issues.

integrate with https://haveibeenpwned.com/
3 participants