Reputation: 107
this is my first project with Spring and I have just started to create the login with Spring Security. I want some pages to be accessible only for the admin and not for the players. I've found some examples on the web and this mechanism works pretty well, I have this secured page that is protected by the login and it's forbidden when the user has no ROLE_ADMIN.
@PreAuthorize("hasAuthority('ROLE_ADMIN')")
@GetMapping("/secured/all")
public String securedHello() {
return "Secured Hello";
}
The problem is that testing my code I found out that Spring authenticates the admin (and the user as well) only checking the username. If I put the wrong password it allows me to enter anyway. I don't understand how this is possible, shouldn't Spring Security do all the authentication work by itself? I've seen somebody suggested to implement an authentication manager or something like that, but I don't understand why and how to insert it in my code. I'm stuck on this since two days, please any advice wuold be really appreciated. These are my classes:
package model;
import java.io.IOException;
import javax.naming.AuthenticationException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.ComponentScan;
import org.springframework.context.annotation.Configuration;
import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
import org.springframework.http.HttpStatus;
import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
import org.springframework.security.config.annotation.method.configuration.EnableGlobalMethodSecurity;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
import org.springframework.security.core.Authentication;
import org.springframework.security.crypto.password.PasswordEncoder;
import com.fasterxml.jackson.databind.ObjectMapper;
@EnableGlobalMethodSecurity(prePostEnabled = true)
@EnableWebSecurity
@EnableJpaRepositories(basePackageClasses = PlayersRepository.class)
@ComponentScan(basePackageClasses= CustomUserDetailsService.class)
@Configuration
public class SecurityConfiguration extends WebSecurityConfigurerAdapter {
@Autowired
private CustomUserDetailsService userDetailsService;
@Override
protected void configure(AuthenticationManagerBuilder auth) throws Exception {
auth.userDetailsService(userDetailsService)
.passwordEncoder(getPasswordEncoder());
}
@Override
protected void configure(HttpSecurity http) throws Exception {
//http.csrf().disable();
http.authorizeRequests()
.antMatchers("**/secured/**").access("hasAuthority('ROLE_ADMIN')")
.anyRequest().permitAll()
.and()
.formLogin().permitAll();
}
private PasswordEncoder getPasswordEncoder() {
return new PasswordEncoder() {
@Override
public String encode(CharSequence charSequence) {
return charSequence.toString();
}
@Override
public boolean matches(CharSequence charSequence, String s) {
return true;
}
};
}
}
package model;
import java.util.ArrayList;
import java.util.Optional;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.security.core.userdetails.User;
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.core.userdetails.UserDetailsService;
import org.springframework.security.core.userdetails.UsernameNotFoundException;
import org.springframework.stereotype.Service;
@Service
public class CustomUserDetailsService implements UserDetailsService {
@Autowired
private PlayersRepository usersRepository;
@Autowired
private RoleRepository rolesRepository;
public CustomUserDetailsService(PlayersRepository usersRepository, RoleRepository rolesRepository) {
this.usersRepository=usersRepository;
this.rolesRepository=rolesRepository;
}
@Override
public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
Optional<Player> optionalUser = usersRepository.findByUsername(username);
optionalUser
.orElseThrow(() -> new UsernameNotFoundException("Username not found"));
Player user= optionalUser.get();
System.out.println(user);
return toUserDetails(new UserObject(user.getUsername(),user.getPassword(),user.getRole()));
}
private UserDetails toUserDetails(UserObject userObject) {
return User.withUsername(userObject.name)
.password(userObject.password)
.roles(userObject.role).build();
}
private static class UserObject {
private String name;
private String password;
private String role;
public UserObject(String name, String password, String role) {
this.name = name;
this.password = password;
this.role = role;
}
}
}
package model;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.core.userdetails.UserDetails;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;
public class CustomUserDetails extends Player implements UserDetails {
String role;
public CustomUserDetails(final Player user) {
super(user);
}
public CustomUserDetails(Optional<Player> user, String role) {
super(user);
this.role=role;
}
@Override
public Collection<? extends GrantedAuthority> getAuthorities() {
List<GrantedAuthority> list = new ArrayList<GrantedAuthority>();
list.add(new SimpleGrantedAuthority("ROLE_"+ role));
System.out.println(list);
return list;
}
@Override
public String getPassword() {
return super.getPassword();
}
@Override
public String getUsername() {
return super.getUsername();
}
@Override
public boolean isAccountNonExpired() {
return true;
}
@Override
public boolean isAccountNonLocked() {
return true;
}
@Override
public boolean isCredentialsNonExpired() {
return true;
}
@Override
public boolean isEnabled() {
return true;
}
}
Upvotes: 4
Views: 8617
Reputation: 44675
Shouldn't Spring Security do all the authentication work by itself?
Yes, Spring Security does that for you using an AuthenticationManager
.
I've seen somebody suggested to implement an authentication manager or something like that, but I don't understand why and how to insert it in my code.
You actually already have an AuthenticationManager
, since you built one within the configure()
method:
@Override
protected void configure(AuthenticationManagerBuilder auth) throws Exception {
auth.userDetailsService(userDetailsService).passwordEncoder(getPasswordEncoder());
}
So, what is exactly the reason this isn't working you may ask. Well, the AuthenticationManager
you provided contains two parts:
CustomUserDetailsService
)getPasswordEncoder()
).What happens behind the screens is that Spring calls your CustomUserDetailsService
to fetch your user information, including your (hashed) password. After fetching that information, it calls your PasswordEncoder.matches()
function to verify if the raw entered password matches your hashed password provided by the CustomUserDetailsService
.
In your case, your PasswordEncoder.matches()
function looks like this:
@Override
public boolean matches(CharSequence charSequence, String s) {
return true;
}
This means that regardless of what password you provide, it will return true
. This is exactly what you're experiencing since any password will work.
So, how do you solve this? Well, your PasswordEncoder
should actually hash your raw password and compare it to the hashed password that is being passed, for example:
@Override
public boolean matches(CharSequence rawPassword, String hashedPassword) {
String hashedPassword2 = null; // hash your rawPassword here
return hashedPassword2.equals(hashedPassword);
}
The implementation of this method depends on how you store your password in your database. Spring Security already comes with a few implementation including BcryptPasswordEncoder
, StandardPasswordEncoder
, MessageDigestPasswordEncoder
, ... . Some of these implementations are deprecated, mostly to indicate that the hashing mechanisms used by those encoders are considered unsafe. There are no plans at the moment of writing to remove those encoders, as mentioned by the Javadoc:
Digest based password encoding is not considered secure. Instead use an adaptive one way function like
BCryptPasswordEncoder
,Pbkdf2PasswordEncoder
, orSCryptPasswordEncoder
. Even better useDelegatingPasswordEncoder
which supports password upgrades. There are no plans to remove this support. It is deprecated to indicate that this is a legacy implementation and using it is considered insecure.
(Emphasis is my own)
If you are free to choose which implementation you pick, then Spring recommends using BCryptPasswordEncoder
as mentioned by the Javadoc:
Service interface for encoding passwords. The preferred implementation is
BCryptPasswordEncoder
.
Upvotes: 7
Reputation: 7624
I just had a Quick scan I found this
private PasswordEncoder getPasswordEncoder() {
return new PasswordEncoder() {
@Override
public String encode(CharSequence charSequence) {
return charSequence.toString();
}
@Override
public boolean matches(CharSequence charSequence, String s) {
return true;
}
};
}
In your matches
you are returning always true.
I guess here you should put logic for checking password for equality something like this
@Override
public boolean matches(CharSequence charSequence, String s) {
return charSequence.toString.equals(s);
}
I would suggest you use something like this
@Bean
public PasswordEncoder passwordEncoder() {
return new BCryptPasswordEncoder();
}
Upvotes: 3