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

Backport fixes to master #3082

Closed
21 of 25 tasks
rjmackay opened this issue Jun 15, 2018 · 4 comments
Closed
21 of 25 tasks

Backport fixes to master #3082

rjmackay opened this issue Jun 15, 2018 · 4 comments
Assignees

Comments

@rjmackay
Copy link
Contributor

rjmackay commented Jun 15, 2018

Overview

Backport fixes from develop to master, before releasing audit report.

Requirements

Acceptance checklist

Roles:

  • With Roles feature enabled set to false
  • Attempt to create/update a new role via HTTP Post
  • You should receive a 422

Password reset

Using link

  • Open a deployment where you have an account and go to login
  • Select "forgot password"
  • Enter email and trigger a password reset
  • Receive password reset email
  • Click link in password reset email
  • Set password to 5 characters
  • Returns an error that password is too short
  • Try again with 8 characters
  • Password is set

Using copy/paste:

  • Open a deployment where you have an account and go to login
  • Select "forgot password"
  • Enter email and trigger a password reset
  • Receive password reset email
  • Copy the token from in password reset email into reset form
  • Set password to 5 characters
  • Returns an error that password is too short
  • Try again with 8 characters
  • Password is set

Registration:

  • You should not be able to make more than 3 registration attempts per minute
@rjmackay
Copy link
Contributor Author

@rowasc I've added an acceptance checklist.
Are you happy to approve this as is?
I'd suggest point score might be a 3 or 5 because of the complexity of backporting

@rowasc
Copy link
Contributor

rowasc commented Jun 25, 2018

@rjmackay looks good.
Small fix: " With Roles feature enabled set to falseWith Roles feat" => I think this should be " With Roles feature enabled set to false"

I'd also add that the user should be able to copy and paste the token to the screen where you have the option to paste the token and add your new password (right after the forgot password modal) . It's a bit obvious but since we had a bug with the reset token not being decoded and failing if you just clicked the link, it might make sense to add the opposite behavior (copy-pasting the token) in the test script here as well.


Other than those small comments, I'd approve this , but probably a 5 because it's still 3 issues that need to be backported and tested and released.

@rjmackay
Copy link
Contributor Author

Thanks @rowasc . Fixed those issues and moved to ready for dev :)

@rjmackay
Copy link
Contributor Author

  • Registration rate limit tests pass
  • password reset validation works, but not exactly as in the test script:
    • client validation prevent reset with the wrong thing
    • it looks like maybe you can't reuse the token after the first attempt to reset

@rowasc rowasc closed this as completed Jul 11, 2018
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

No branches or pull requests

3 participants