Skip to content

Commit

Permalink
Make logins for locally defined users no longer case sensitive (#220)
Browse files Browse the repository at this point in the history
* Make logins for locally defined users no longer case sensitive

* update changelog
  • Loading branch information
Crim authored Jun 21, 2020
1 parent 5df60bf commit 68316a7
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
## 2.6.0 (UNRELEASED)
- [ISSUE-144](https://github.com/SourceLabOrg/kafka-webview/issues/144) Make providing a TrustStore file when setting up a SSL enabled cluster optional. You might not want/need this option if your JVM is already configured to accept the SSL certificate served by the cluster, or if the cluster's certificate can be validated by a publically accessible CA.
- [PR-215](https://github.com/SourceLabOrg/kafka-webview/pull/215) Improve errors displayed when using the `test cluster` functionality.
- [PR-220](https://github.com/SourceLabOrg/kafka-webview/pull/220) Usernames/email addresses for locally defined users are no longer case-insensitive.

## 2.5.1 (05/19/2020)
- [ISSUE-209](https://github.com/SourceLabOrg/kafka-webview/issues/209) Expose HealthCheck and App Info endpoints without requiring authentication.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public String update(
model.addAttribute("isAdmin", isAdmin);

// Validate email doesn't already exist!
final User existingUser = userRepository.findByEmail(userForm.getEmail());
final User existingUser = userRepository.findByEmailIgnoreCase(userForm.getEmail());
if ((userForm.exists() && existingUser != null && existingUser.getId() != userForm.getId())
|| (!userForm.exists() && existingUser != null)) {
bindingResult.addError(new FieldError(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public String lostPasswordFormSubmit(
redirectAttributes.addFlashAttribute("FlashMessage", flashMessage);

// Retrieve User by Email
final User user = userRepository.findByEmail(lostPasswordForm.getEmail());
final User user = userRepository.findByEmailIgnoreCase(lostPasswordForm.getEmail());
if (user != null) {
// Do email reset request.
//resetUserPasswordManager.requestPasswordReset(user);
Expand Down Expand Up @@ -158,7 +158,7 @@ public String resetPasswordFormSubmit(
}

// Retrieve User by Email
final User user = userRepository.findByEmail(resetPasswordForm.getEmail());
final User user = userRepository.findByEmailIgnoreCase(resetPasswordForm.getEmail());
boolean result = false;
if (user != null) {
// Attempt reset
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public CustomUserDetailsService(final UserRepository userRepository) {

@Override
public UserDetails loadUserByUsername(final String email) throws UsernameNotFoundException {
final User user = userRepository.findByEmail(email);
final User user = userRepository.findByEmailIgnoreCase(email);
if (user == null) {
throw new UsernameNotFoundException("User not found.");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,18 @@ public interface UserRepository extends CrudRepository<User, Long> {

/**
* Find user by email address.
* NOTE: Case sensitive!
* @param email Email to lookup user by
* @return User or null if none found.
* @deprecated probably want to use findByEmailIgnoreCase()
*/
User findByEmail(String email);

/**
* Find user by email address.
* NOTE: Case insensitive!
* @param email Email to lookup user by
* @return User or null if none found.
*/
User findByEmailIgnoreCase(String email);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/**
* MIT License
*
* Copyright (c) 2017, 2018, 2019 SourceLab.org (https://github.com/SourceLabOrg/kafka-webview/)
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/

package org.sourcelab.kafka.webview.ui.manager.user;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.sourcelab.kafka.webview.ui.model.User;
import org.sourcelab.kafka.webview.ui.tools.UserTestTools;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.core.userdetails.UsernameNotFoundException;
import org.springframework.test.context.junit4.SpringRunner;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

@SpringBootTest
@RunWith(SpringRunner.class)
public class CustomUserDetailsServiceTest {

@Autowired
private UserTestTools userTestTools;

/**
* Instance under test.
*/
@Autowired
private CustomUserDetailsService customUserDetailsService;

/**
* Verify what happens if you attempt to load a user which does not exist.
*/
@Test(expected = UsernameNotFoundException.class)
public void smokeTest_invalidUser() {
final String email = "[email protected]";
customUserDetailsService.loadUserByUsername(email);
}

/**
* Test loading using the same case.
*/
@Test
public void smokeTest_loadValidUser_sameCase() {
// Setup user.
final String userEmail = "test" + System.currentTimeMillis() + "@example.com";
final User user = userTestTools.createUser();
user.setEmail(userEmail);
userTestTools.save(user);

// Attempt to load
final UserDetails userDetails = customUserDetailsService.loadUserByUsername(userEmail);

// Verify
assertNotNull("Result should be non-null", userDetails);
assertTrue("Should be a CustomUserDetails instance", userDetails instanceof CustomUserDetails);
assertEquals("Should have correct email", userEmail, userDetails.getUsername());
assertEquals("Should have correct id", user.getId(), ((CustomUserDetails) userDetails).getUserId());
assertNotNull("Should have a user model", ((CustomUserDetails) userDetails).getUserModel());
}

/**
* Test loading using insensitive case.
*/
@Test
public void smokeTest_loadValidUser_differentCasing() {
// Setup user.
final String userEmail = "test" + System.currentTimeMillis() + "@example.com";
final User user = userTestTools.createUser();
user.setEmail(userEmail);
userTestTools.save(user);

// Setup lookup email to have a different case.
final String lookupEmail = userEmail.toUpperCase();

// Attempt to load using different case.
final UserDetails userDetails = customUserDetailsService.loadUserByUsername(lookupEmail);

// Verify
assertNotNull("Result should be non-null", userDetails);
assertTrue("Should be a CustomUserDetails instance", userDetails instanceof CustomUserDetails);
assertEquals("Should have correct email", userEmail, userDetails.getUsername());
assertEquals("Should have correct id", user.getId(), ((CustomUserDetails) userDetails).getUserId());
assertNotNull("Should have a user model", ((CustomUserDetails) userDetails).getUserModel());
}
}

0 comments on commit 68316a7

Please sign in to comment.