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.

Sign up for GitHub

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

Jump to bottom

BCryptPasswordEncoder handle null rawPassword #7981

Closed
wants to merge 4 commits into from
Closed

BCryptPasswordEncoder handle null rawPassword #7981

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b454ebd
detect whether rawPassword is empty
ge1mina023 Feb 13, 2020
e84016a
Detecting NullPointerException from rawPassword
ge1mina023 Feb 13, 2020
df9a3b3
use lf
ge1mina023 Feb 13, 2020
e6bc8ed
Detecting the NullPointerException of rawPassword
ge1mina023 Feb 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2011 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -119,6 +119,11 @@ public boolean matches(CharSequence rawPassword, String encodedPassword) {
return false;
}

if(rawPassword.toString() == null || rawPassword.toString.length() == 0){
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's have this match the earlier check:

Suggested change
if(rawPassword.toString() == null || rawPassword.toString.length() == 0){
if (rawPassword == null || rawPassword.length() == 0) {

Also, for readability, we should probably place this at the beginning of the method, so that the method parameters are testing in the same order that they are defined.

logger.warn("Empty raw password");
return false;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you also add a couple of tests to BCryptPasswordEncoderTests? I think that doesntMatchNullEncodedValue and doesntMatchEmptyEncodedValue are probably good examples to follow.

return BCrypt.checkpw(rawPassword.toString(), encodedPassword);
}

Expand Down