-
Notifications
You must be signed in to change notification settings - Fork 82
user/edit/password: check haveibeenpwned.com #1744
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
base: master
Are you sure you want to change the base?
Changes from all commits
93e457d
96e14a5
866ce50
c8fc03f
df913cd
bb3ad0c
eec7ea1
ee681d1
226180d
46fe608
bc76850
edcff1b
7528a42
791bc69
fc03a69
02c613f
e20053c
c1775bb
398e3af
876c462
dc023cc
4722948
e7880c2
7443aa1
49f8009
1c705a5
bf37431
715514d
01d73fd
b54fe8f
9e1c738
ebeb632
41d6f30
aff5e42
248ff5f
5735388
fe8f4d0
e2844af
eaa65a8
7d2f6e4
a3d035a
c6e7413
0060c22
d70d8b3
1996282
34c9950
2fb8123
cad294e
d9e0ede
89499a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,20 +18,37 @@ except according to the terms contained in the LICENSE file. | |
| <p v-if="config.oidcEnabled">{{ $t('oidcBody') }}</p> | ||
| <form v-else-if="user.dataExists && user.id === currentUser.id" | ||
| @submit.prevent="submit"> | ||
| <input :value="currentUser.email" autocomplete="username"> | ||
| <form-group id="user-edit-password-old-password" v-model="oldPassword" | ||
| type="password" :placeholder="$t('field.oldPassword')" required | ||
| autocomplete="current-password"/> | ||
| <form-group id="user-edit-password-new-password" v-model="newPassword" | ||
| type="password" :placeholder="$t('field.newPassword')" required | ||
| :has-error="tooShort || mismatch" autocomplete="new-password"/> | ||
| <form-group id="user-edit-password-confirm" v-model="confirm" | ||
| type="password" :placeholder="$t('field.passwordConfirm')" required | ||
| :has-error="mismatch" autocomplete="new-password"/> | ||
| <button type="submit" class="btn btn-primary" | ||
| :aria-disabled="awaitingResponse"> | ||
| {{ $t('action.change') }} <spinner :state="awaitingResponse"/> | ||
| </button> | ||
| <fieldset :disabled="submitInProgress"> | ||
| <input :value="currentUser.email" autocomplete="username"> | ||
| <form-group id="user-edit-password-old-password" v-model="oldPassword" | ||
| type="password" :placeholder="$t('field.oldPassword')" required | ||
| autocomplete="current-password"/> | ||
| <form-group id="user-edit-password-new-password" v-model="newPassword" | ||
| type="password" :placeholder="$t('field.newPassword')" required | ||
| :has-error="tooShort || mismatch || pwned" autocomplete="new-password"> | ||
| <template #after> | ||
| <transition name="collapse"> | ||
| <div v-if="pwned" class="collapsible-error"> | ||
| <div class="collapsible-inner"> | ||
| <p>{{ $t('alert.includedInBreach') }}</p> | ||
| <i18n-t keypath="moreInfo.clickHere.full"> | ||
| <template #clickHere> | ||
| <a href="https://haveibeenpwned.com/Passwords" target="_blank" rel="noopener noreferrer">{{ $t('moreInfo.clickHere.clickHere') }}</a> | ||
| </template> | ||
| </i18n-t> | ||
| </div> | ||
| </div> | ||
| </transition> | ||
| </template> | ||
| </form-group> | ||
| <form-group id="user-edit-password-confirm" v-model="confirm" | ||
| type="password" :placeholder="$t('field.passwordConfirm')" required | ||
| :has-error="mismatch" autocomplete="new-password"/> | ||
| <button type="submit" class="btn btn-primary" | ||
| :aria-disabled="awaitingResponse"> | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is now functionally pointless. It should be removed, but this will require changes to the test for "normal button" behaviour. |
||
| {{ $t('action.change') }} <spinner :state="submitInProgress"/> | ||
| </button> | ||
| </fieldset> | ||
| </form> | ||
| <p v-else>{{ $t('cannotChange') }}</p> | ||
| </div> | ||
|
|
@@ -46,6 +63,7 @@ import useRequest from '../../../composables/request'; | |
| import { apiPaths } from '../../../util/request'; | ||
| import { noop } from '../../../util/util'; | ||
| import { useRequestData } from '../../../request-data'; | ||
| import { checkPasswordPwnage } from '../../../util/password'; | ||
|
|
||
| export default { | ||
| name: 'UserEditPassword', | ||
|
|
@@ -62,13 +80,21 @@ export default { | |
| newPassword: '', | ||
| tooShort: false, | ||
| confirm: '', | ||
| mismatch: false | ||
| mismatch: false, | ||
| pwned: false, | ||
| submitInProgress: false, | ||
| }; | ||
| }, | ||
| watch: { | ||
| newPassword() { | ||
| this.pwned = false; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It sounds reasonable to me to reset
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'd like this to change. Would that be OK? If so, would you prefer a prior to watch these values?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changing that sounds good to me. 👍 I'm happy for that to happen in a prior PR or a follow-up PR or this PR. Mostly I just want
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm thinking we only need one watcher, on
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops, looks like there's a word missing. Should have read:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A prior PR sounds good to me. 👍 But I'm also happy for it to happen in this PR or in a follow-up PR. Basically whatever's easiest as long as it's done in time for the release. |
||
| }, | ||
| }, | ||
| methods: { | ||
| validate() { | ||
| this.tooShort = false; | ||
| this.mismatch = false; | ||
| this.pwned = false; | ||
|
|
||
| if (this.newPassword.length < 10) { | ||
| this.alert.danger(this.$t('alert.passwordTooShort')); | ||
|
|
@@ -86,26 +112,53 @@ export default { | |
| }, | ||
| submit() { | ||
| if (!this.validate()) return; | ||
| const data = { old: this.oldPassword, new: this.newPassword }; | ||
| this.request({ | ||
| method: 'PUT', | ||
| url: apiPaths.password(this.user.id), | ||
| data | ||
| }) | ||
| .then(() => { | ||
| this.alert.success(this.$t('alert.success')); | ||
|
|
||
| // The Chrome password manager does not realize that the form was | ||
| // submitted. Should we navigate to a different page so that it does? | ||
| this.submitInProgress = true; | ||
|
|
||
| checkPasswordPwnage(this.request, this.newPassword) | ||
| .then(isPwned => { | ||
| if (isPwned) { | ||
| this.pwned = true; | ||
| return; | ||
| } | ||
|
|
||
| const data = { old: this.oldPassword, new: this.newPassword }; | ||
| this.request({ | ||
| method: 'PUT', | ||
| url: apiPaths.password(this.user.id), | ||
| data | ||
| }) | ||
| .then(() => { | ||
| this.alert.success(this.$t('alert.success')); | ||
|
|
||
| // The Chrome password manager does not realize that the form was | ||
| // submitted. Should we navigate to a different page so that it does? | ||
| }); | ||
| }) | ||
| .catch(noop); | ||
| .catch(noop) | ||
| .finally(() => { | ||
| this.submitInProgress = false; | ||
| }); | ||
| } | ||
| } | ||
| }; | ||
| </script> | ||
|
|
||
| <style lang="scss"> | ||
| @import '../../../assets/scss/variables'; | ||
|
|
||
| #user-edit-password input[autocomplete="username"] { display: none; } | ||
| .collapsible-error { | ||
| display: grid; | ||
| grid-template-rows: 1fr; | ||
| color: $color-danger; | ||
| font-size: 11px; | ||
| margin: 25px 12px -25px; | ||
|
|
||
| .collapsible-inner { overflow:hidden } | ||
| } | ||
| .collapse-enter-active, .collapse-leave-active { transition:grid-template-rows 0.3s ease, opacity 0.3s ease } | ||
| .collapse-enter-from, .collapse-leave-to { grid-template-rows:0fr; opacity:0 } | ||
| </style> | ||
|
|
||
| <i18n lang="json5"> | ||
|
|
@@ -119,6 +172,7 @@ export default { | |
| }, | ||
| "cannotChange": "Only the owner of the account may directly set their own password.", | ||
| "alert": { | ||
| "includedInBreach": "This password has previously been included in a breach.", | ||
| "mismatch": "Please check that your new passwords match.", | ||
| "success": "Success! Your password has been updated." | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| export async function checkPasswordPwnage(request, password) { // eslint-disable-line import/prefer-default-export | ||
| const hash = await sha1hash(password); // eslint-disable-line no-use-before-define | ||
|
|
||
| const hashPrefix = hash.substring(0, 5); | ||
| const hashSuffix = hash.substring(5); | ||
|
|
||
| const suffixes = await getSuffixesFor(request, hashPrefix); // eslint-disable-line no-use-before-define | ||
|
|
||
| return suffixes.includes(hashSuffix); | ||
| } | ||
|
|
||
| // from: https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/digest#converting_a_digest_to_a_hex_string | ||
| async function sha1hash(message) { | ||
| const msgUint8 = new TextEncoder().encode(message); | ||
| const hashBuffer = await crypto.subtle.digest('SHA-1', msgUint8); | ||
| const hashArray = Array.from(new Uint8Array(hashBuffer)); | ||
| return hashArray | ||
| .map(b => b.toString(16).padStart(2, '0')) | ||
| .join('') | ||
| .toUpperCase(); | ||
| } | ||
|
|
||
| async function getSuffixesFor(request, prefix) { | ||
| try { | ||
| const url = `https://api.pwnedpasswords.com/range/${prefix}`; | ||
| const res = await request({ url, alert: false }); | ||
| return res.data.split('\n').map(line => line.split(':')[0]); | ||
| } catch (err) { | ||
| console.log('pwned check failed:', err); // eslint-disable-line no-console | ||
| // if we can't check, just let them use it | ||
| return []; | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.