Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
goekay committed Aug 15, 2024
1 parent 163de00 commit 0992d5c
Show file tree
Hide file tree
Showing 4 changed files with 224 additions and 146 deletions.
22 changes: 0 additions & 22 deletions src/main/java/de/rwth/idsg/steve/config/SecurityConfiguration.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@

import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.base.Strings;
import de.rwth.idsg.steve.SteveConfiguration;
import de.rwth.idsg.steve.repository.WebUserRepository;
import de.rwth.idsg.steve.web.api.ApiControllerAdvice;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
Expand All @@ -37,15 +35,13 @@
import org.springframework.security.config.http.SessionCreationPolicy;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.AuthenticationException;
import org.springframework.security.core.userdetails.User;
import org.springframework.security.crypto.factory.PasswordEncoderFactories;
import org.springframework.security.crypto.password.DelegatingPasswordEncoder;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.security.web.AuthenticationEntryPoint;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.security.web.authentication.preauth.AbstractPreAuthenticatedProcessingFilter;

import jakarta.annotation.PostConstruct;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
Expand All @@ -64,24 +60,6 @@
@EnableWebSecurity
public class SecurityConfiguration {

private final WebUserRepository webUserRepository;

@PostConstruct
public void postConstruct() {
if (webUserRepository.hasUserWithAuthority("ADMIN")) {
return;
}

var user = User
.withUsername(SteveConfiguration.CONFIG.getAuth().getUserName())
.password(SteveConfiguration.CONFIG.getAuth().getEncodedPassword())
.disabled(false)
.authorities("ADMIN")
.build();

webUserRepository.createUser(user);
}

/**
* Password encoding changed with spring-security 5.0.0. We either have to use a prefix before the password to
* indicate which actual encoder {@link DelegatingPasswordEncoder} should use [1, 2] or specify the encoder as we do.
Expand Down
18 changes: 15 additions & 3 deletions src/main/java/de/rwth/idsg/steve/repository/WebUserRepository.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,25 @@
*/
package de.rwth.idsg.steve.repository;

import org.springframework.security.provisioning.UserDetailsManager;
import jooq.steve.db.tables.records.WebUserRecord;

public interface WebUserRepository extends UserDetailsManager {
public interface WebUserRepository {

void createUser(WebUserRecord user);

void updateUser(WebUserRecord user);

void deleteUser(String username);

void deleteUser(int webUserPk);

void changeStatusOfUser(String username, boolean enabled);

boolean hasUserWithAuthority(String authority);
Integer getUserCountWithAuthority(String authority);

void changePassword(String username, String newPassword);

boolean userExists(String username);

WebUserRecord loadUserByUsername(String username);
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,38 +18,19 @@
*/
package de.rwth.idsg.steve.repository.impl;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import de.rwth.idsg.steve.repository.WebUserRepository;
import jooq.steve.db.tables.records.WebUserRecord;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.jooq.DSLContext;
import org.jooq.JSON;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextHolderStrategy;
import org.springframework.security.core.userdetails.User;
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.core.userdetails.UsernameNotFoundException;
import org.springframework.security.provisioning.JdbcUserDetailsManager;
import org.springframework.stereotype.Repository;
import org.springframework.util.Assert;

import java.util.Collection;
import java.util.LinkedHashSet;
import java.util.stream.Collectors;

import static jooq.steve.db.Tables.WEB_USER;
import static org.jooq.impl.DSL.condition;
import static org.springframework.security.authentication.UsernamePasswordAuthenticationToken.authenticated;
import static org.springframework.security.core.context.SecurityContextHolder.getContextHolderStrategy;
import static org.jooq.impl.DSL.count;

/**
* Inspired by {@link org.springframework.security.provisioning.JdbcUserDetailsManager}
*
* @author Sevket Goekay <sevketgokay@gmail.com>
* @since 10.08.2024
*/
Expand All @@ -59,29 +40,23 @@
public class WebUserRepositoryImpl implements WebUserRepository {

private final DSLContext ctx;
private final ObjectMapper jacksonObjectMapper;
private final SecurityContextHolderStrategy securityContextHolderStrategy = getContextHolderStrategy();

@Override
public void createUser(UserDetails user) {
validateUserDetails(user);

public void createUser(WebUserRecord user) {
ctx.insertInto(WEB_USER)
.set(WEB_USER.USERNAME, user.getUsername())
.set(WEB_USER.PASSWORD, user.getPassword())
.set(WEB_USER.ENABLED, user.isEnabled())
.set(WEB_USER.AUTHORITIES, toJson(user.getAuthorities()))
.set(WEB_USER.ENABLED, user.getEnabled())
.set(WEB_USER.AUTHORITIES, user.getAuthorities())
.execute();
}

@Override
public void updateUser(UserDetails user) {
validateUserDetails(user);

public void updateUser(WebUserRecord user) {
ctx.update(WEB_USER)
.set(WEB_USER.PASSWORD, user.getPassword())
.set(WEB_USER.ENABLED, user.isEnabled())
.set(WEB_USER.AUTHORITIES, toJson(user.getAuthorities()))
.set(WEB_USER.ENABLED, user.getEnabled())
.set(WEB_USER.AUTHORITIES, user.getAuthorities())
.where(WEB_USER.USERNAME.eq(user.getUsername()))
.execute();
}
Expand Down Expand Up @@ -109,40 +84,20 @@ public void changeStatusOfUser(String username, boolean enabled) {
}

@Override
public boolean hasUserWithAuthority(String authority) {
public Integer getUserCountWithAuthority(String authority) {
JSON authValue = JSON.json("\"" + authority + "\"");
return ctx.selectOne()
return ctx.selectCount()
.from(WEB_USER)
.where(condition("json_contains({0}, {1})", WEB_USER.AUTHORITIES, authValue))
.fetchOptional()
.isPresent();
.fetchOne(count());
}

/**
* Not only just an SQL Update.
* The flow is inspired by {@link JdbcUserDetailsManager#changePassword(String, String)}
*/
@Override
public void changePassword(String oldPassword, String newPassword) {
Authentication currentUser = this.securityContextHolderStrategy.getContext().getAuthentication();
if (currentUser == null) {
// This would indicate bad coding somewhere
throw new AccessDeniedException(
"Can't change password as no Authentication object found in context for current user."
);
}

String username = currentUser.getName();

public void changePassword(String username, String newPassword) {
ctx.update(WEB_USER)
.set(WEB_USER.PASSWORD, newPassword)
.where(WEB_USER.USERNAME.eq(username))
.execute();

Authentication authentication = createNewAuthentication(currentUser, newPassword);
SecurityContext context = this.securityContextHolderStrategy.createEmptyContext();
context.setAuthentication(authentication);
this.securityContextHolderStrategy.setContext(context);
}

@Override
Expand All @@ -155,72 +110,9 @@ public boolean userExists(String username) {
}

@Override
public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
WebUserRecord record = ctx.selectFrom(WEB_USER)
public WebUserRecord loadUserByUsername(String username) {
return ctx.selectFrom(WEB_USER)
.where(WEB_USER.USERNAME.eq(username))
.fetchOne();

if (record == null) {
throw new UsernameNotFoundException(username);
}

return User
.withUsername(record.getUsername())
.password(record.getPassword())
.disabled(!record.getEnabled())
.authorities(fromJson(record.getAuthorities()))
.build();
}

private String[] fromJson(JSON jsonArray) {
try {
return jacksonObjectMapper.readValue(jsonArray.data(), String[].class);
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
}
}

private JSON toJson(Collection<? extends GrantedAuthority> authorities) {
Collection<String> auths = authorities.stream()
.map(GrantedAuthority::getAuthority)
.sorted() // keep a stable order of entries
.collect(Collectors.toCollection(LinkedHashSet::new)); // prevent duplicates

try {
String str = jacksonObjectMapper.writeValueAsString(auths);
return JSON.jsonOrNull(str);
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
}
}

/**
* Lifted from {@link JdbcUserDetailsManager#validateUserDetails(UserDetails)}
*/
private static void validateUserDetails(UserDetails user) {
Assert.hasText(user.getUsername(), "Username may not be empty or null");
validateAuthorities(user.getAuthorities());
}

/**
* Lifted from {@link JdbcUserDetailsManager#validateAuthorities(Collection)}
*/
private static void validateAuthorities(Collection<? extends GrantedAuthority> authorities) {
Assert.notNull(authorities, "Authorities list must not be null");
for (GrantedAuthority authority : authorities) {
Assert.notNull(authority, "Authorities list contains a null entry");
Assert.hasText(authority.getAuthority(), "getAuthority() method must return a non-empty string");
}
}

/**
* Lifted from {@link JdbcUserDetailsManager#createNewAuthentication(Authentication, String)}
*/
private Authentication createNewAuthentication(Authentication currentAuth, String newPassword) {
var user = this.loadUserByUsername(currentAuth.getName());
var newAuthentication = authenticated(user, null, user.getAuthorities());
newAuthentication.setDetails(currentAuth.getDetails());
return newAuthentication;
}

}
Loading

0 comments on commit 0992d5c

Please sign in to comment.