Make invalid password message clearer by derek-ho · Pull Request #3057 · opensearch-project/security

derek-ho · 2023-07-26T21:10:33Z

Description

Returns the error message instead of the validation message because validation message should already have been shown in above case statements.

Issues Resolved

Fix: #3055

Is this a backport? If so, please add backport PR # and/or commits #

Testing

[Please provide details of testing done: unit testing, integration testing and manual testing]

Check List

New functionality includes testing
New functionality has been documented
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

codecov · 2023-07-26T21:14:14Z

Codecov Report

Merging #3057 (16be66f) into main (bd084c8) will decrease coverage by 0.02%.
The diff coverage is 85.71%.

@@             Coverage Diff              @@
##               main    #3057      +/-   ##
============================================
- Coverage     62.46%   62.44%   -0.02%     
+ Complexity     3354     3352       -2     
============================================
  Files           254      254              
  Lines         19749    19747       -2     
  Branches       3334     3334              
============================================
- Hits          12337    12332       -5     
+ Misses         5784     5783       -1     
- Partials       1628     1632       +4

Files Changed	Coverage Δ
...curity/dlic/rest/validation/PasswordValidator.java	`82.53% <66.66%> (ø)`
.../dlic/rest/validation/RequestContentValidator.java	`91.61% <100.00%> (-0.10%)`	⬇️

... and 6 files with indirect coverage changes

src/main/java/org/opensearch/security/dlic/rest/validation/AbstractConfigurationValidator.java

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Signed-off-by: Derek Ho <dxho@amazon.com>

src/main/java/org/opensearch/security/dlic/rest/validation/RequestContentValidator.java

willyborankin · 2023-07-29T21:52:12Z

Checkstyle fails :-). gradle checkstyleMain checkstyleTest should help

Signed-off-by: Derek Ho <dxho@amazon.com>

willyborankin · 2023-07-31T15:12:49Z

I think test failures are not related to this change.

peternied

I think the previous behavior was better for users to act on, why remove the distinct message?

src/main/java/org/opensearch/security/dlic/rest/validation/RequestContentValidator.java

…message

peternied

CI failures due to broken tests

…ange-password-security-message

Signed-off-by: Derek Ho <dxho@amazon.com>

peternied

Thanks for cleaning up that code, this makes the documentation align better to how the product behaves.

Signed-off-by: Derek Ho <dxho@amazon.com>

opensearch-trigger-bot · 2023-08-17T13:44:43Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/security/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/security/backport-2.x
# Create a new branch
git switch --create backport/backport-3057-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 847f91108a4a10c8368fde26a8f79266edf423c3
# Push it to GitHub
git push --set-upstream origin backport/backport-3057-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/security/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-3057-to-2.x.

DarshitChanpura · 2023-08-17T14:15:53Z

@derek-ho Mind creating the backport manually?

Returns the error message instead of the validation message because validation message should already have been shown in above case statements. Fix: opensearch-project#3055 Is this a backport? If so, please add backport PR # and/or commits # [Please provide details of testing done: unit testing, integration testing and manual testing] - [ ] New functionality includes testing - [ ] New functionality has been documented - [ ] Commits are signed per the DCO using --signoff By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check [here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin). --------- Signed-off-by: Craig Perkins <cwperx@amazon.com> Signed-off-by: Derek Ho <dxho@amazon.com> Co-authored-by: Craig Perkins <cwperx@amazon.com> (cherry picked from commit 847f911)

Backports 847f911 from #3057 Returns the error message instead of the validation message because validation message should already have been shown in above case statements. Signed-off-by: Craig Perkins <cwperx@amazon.com> Signed-off-by: Derek Ho <dxho@amazon.com> Co-authored-by: Craig Perkins <cwperx@amazon.com> (cherry picked from commit 847f911) --------- Signed-off-by: Derek Ho <dxho@amazon.com>

cwperks reviewed Jul 27, 2023

View reviewed changes

src/main/java/org/opensearch/security/dlic/rest/validation/AbstractConfigurationValidator.java Outdated Show resolved Hide resolved

cwperks and others added 2 commits July 29, 2023 16:45

Fix build with Lucene upgrade

015d5fc

Signed-off-by: Craig Perkins <cwperx@amazon.com>

fix message and add test

6479513

Signed-off-by: Derek Ho <dxho@amazon.com>

derek-ho force-pushed the change-password-security-message branch from 36a6c4e to 6479513 Compare July 29, 2023 21:26

derek-ho marked this pull request as ready for review July 29, 2023 21:26

derek-ho requested review from DarshitChanpura, RyanL1997, cliu123, davidlago, peternied, reta, stephen-crawford and willyborankin as code owners July 29, 2023 21:26

derek-ho commented Jul 29, 2023

View reviewed changes

src/main/java/org/opensearch/security/dlic/rest/validation/RequestContentValidator.java Show resolved Hide resolved

fix start import

2db63a7

Signed-off-by: Derek Ho <dxho@amazon.com>

derek-ho force-pushed the change-password-security-message branch 2 times, most recently from 36a6c4e to 2db63a7 Compare July 31, 2023 13:26

Merge branch 'main' of github.com:opensearch-project/security

fb3f37c

willyborankin previously approved these changes Jul 31, 2023

View reviewed changes

cwperks previously approved these changes Jul 31, 2023

View reviewed changes

Merge branch 'main' of github.com:opensearch-project/security

239c4f6

peternied suggested changes Aug 5, 2023

View reviewed changes

src/main/java/org/opensearch/security/dlic/rest/validation/RequestContentValidator.java Show resolved Hide resolved

Merge branch 'opensearch-project:main' into change-password-security-…

3d7f82b

…message

peternied approved these changes Aug 16, 2023

View reviewed changes

peternied suggested changes Aug 16, 2023

View reviewed changes

derek-ho added 2 commits August 16, 2023 13:08

Merge branch 'main' of github.com:opensearch-project/security into ch…

d612464

…ange-password-security-message

fix test

19f34c7

Signed-off-by: Derek Ho <dxho@amazon.com>

derek-ho dismissed stale reviews from cwperks and willyborankin via 19f34c7 August 16, 2023 17:18

derek-ho added 2 commits August 16, 2023 14:22

update enums to include too short too long and invalid regex

d733e0a

Signed-off-by: Derek Ho <dxho@amazon.com>

spotlessApply

d67745a

Signed-off-by: Derek Ho <dxho@amazon.com>

peternied previously approved these changes Aug 16, 2023

View reviewed changes

cwperks previously approved these changes Aug 16, 2023

View reviewed changes

derek-ho added 2 commits August 16, 2023 14:55

update missed test

839d036

Signed-off-by: Derek Ho <dxho@amazon.com>

spotless

16be66f

Signed-off-by: Derek Ho <dxho@amazon.com>

derek-ho dismissed stale reviews from cwperks and peternied via 16be66f August 16, 2023 18:59

peternied approved these changes Aug 16, 2023

View reviewed changes

cwperks added the backport 2.x label Aug 17, 2023

cwperks approved these changes Aug 17, 2023

View reviewed changes

cwperks merged commit 847f911 into opensearch-project:main Aug 17, 2023

opensearch-trigger-bot bot added the backport-failed label Aug 17, 2023

peternied mentioned this pull request Aug 18, 2023

[Backport 2.x] Change password security message (#3057) #3203

Merged

peternied changed the title ~~Change password security message~~ Make invalid password message clearer Aug 31, 2023

peternied removed the backport-failed label Aug 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make invalid password message clearer#3057

Make invalid password message clearer#3057
cwperks merged 12 commits intoopensearch-project:mainfrom
derek-ho:change-password-security-message

derek-ho commented Jul 26, 2023 •

edited

Loading

Uh oh!

codecov bot commented Jul 26, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

willyborankin commented Jul 29, 2023 •

edited

Loading

Uh oh!

willyborankin commented Jul 31, 2023

Uh oh!

peternied left a comment

Uh oh!

Uh oh!

peternied left a comment

Uh oh!

peternied left a comment

Uh oh!

opensearch-trigger-bot bot commented Aug 17, 2023

Uh oh!

DarshitChanpura commented Aug 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

derek-ho commented Jul 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues Resolved

Testing

Check List

Uh oh!

codecov bot commented Jul 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

willyborankin commented Jul 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

willyborankin commented Jul 31, 2023

Uh oh!

peternied left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

peternied left a comment

Choose a reason for hiding this comment

Uh oh!

peternied left a comment

Choose a reason for hiding this comment

Uh oh!

opensearch-trigger-bot bot commented Aug 17, 2023

Uh oh!

DarshitChanpura commented Aug 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

derek-ho commented Jul 26, 2023 •

edited

Loading

codecov bot commented Jul 26, 2023 •

edited

Loading

willyborankin commented Jul 29, 2023 •

edited

Loading