Skip to content

Make password minimum requirements stronger#197

Merged
tdonohue merged 2 commits intoDSpace:mainfrom
4Science:CST-6108
Sep 22, 2022
Merged

Make password minimum requirements stronger#197
tdonohue merged 2 commits intoDSpace:mainfrom
4Science:CST-6108

Conversation

@Micheleboychuk
Copy link
Copy Markdown
Contributor

@Micheleboychuk Micheleboychuk commented Jul 14, 2022

Copy link
Copy Markdown
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Micheleboychuk : Gave this a review today alongside the implementation. Overall it looks good, but minor suggestions inline below.

Comment thread epersons.md Outdated
* 201 Created - if the operation succeed
* 400 Bad Request - if the email address didn't match the token or already exists. If the token doesn't exist or is expired
* 401 Unauthorized - if the token doesn't allow you to create this account
* 422 Unprocessable Entity - If password validation is enabled and the password of respects the rules configured in the regular expression
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the text of this could be improved. Maybe say "If password validation is enabled, and the new password doesn't validate based on those rules. See [configuration name]" (Please replace the [configuration name] with the final name of the configuration... I didn't fill it out because I suggested it be renamed in my review here: DSpace/DSpace#8404 (review))

I think we also need to add this 422 exception to the Patch -> Add section in this same epersons.md as it's also possible this will be thrown when changing your password.

@tdonohue tdonohue added this to the 7.4 milestone Aug 17, 2022
@tdonohue
Copy link
Copy Markdown
Member

@Micheleboychuk : This PR still needs updates to align with the implementation PR DSpace/DSpace#8404

Copy link
Copy Markdown
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks @Micheleboychuk . This looks good enough to me. However, I'll wait to merge it until the implementation PR is ready to merge (it just needs one more approval)

@tdonohue tdonohue merged commit b6fc9c2 into DSpace:main Sep 22, 2022
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.

Make password minimum requirements stronger

2 participants