Skip to content

Commit

Permalink
feat: The client is now a bit more secure against attacks and authent…
Browse files Browse the repository at this point in the history
…ication token (JWT) stealing. For this, the JWT is now transferred and processed in HTTP-only cookies. In this context, XSRF protection with XSRF-TOKEN cookies has also been enabled.

Beside the `security.jwt.sharedSecret` the following values are configurable now:
- `security.jwt.expirationTime` (expiration time of the JWT)
- `security.jwt.cookieName` (name of the HTTP-Only cookie)
- `security.jwt.setSecure` (the boolean value for the cookie parameter `Secure` )
- `security.jwt.same-site` (`Strict` or `Lax` as value for the cookie parameter `SameSite`)

FE: Mock authentication flow and user role definition without using a token in the request header (mockAPIServer).

We have tried to fix CORS errors with the e2e tests, however there are still issues that need to be investigated and fixed later:
- Changes NGINX template to avoid network errors
- makes sameSite parameter of the cookies configurable

Co-authored-by: andreas.kausler <[email protected]>
PR #802
  • Loading branch information
jekutzsche authored Jun 8, 2022
1 parent 863a24c commit ae25da8
Show file tree
Hide file tree
Showing 24 changed files with 356 additions and 139 deletions.
6 changes: 3 additions & 3 deletions infrastructure/dev/nginx/iris-client.conf.template
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ server {
set $cors_header "";
set $cors_method "";
set $cors_maxage "";
if ( $http_origin ~* ((https:\/\/iris.staging.iris-gateway\.de|http:\/\/localhost:8080)$)) {
if ( $http_origin ~* ((https:\/\/iris.staging.iris-gateway\.de|https:\/\/client.dev.iris-gateway\.de|http:\/\/localhost:8080|http:\/\/localhost:8081)$)) {
set $cors_origin $http_origin;
set $cors_cred true;
set $cors_header "*";
set $cors_method "*";
set $cors_header $http_access_control_request_headers;
set $cors_method $http_access_control_request_method;
set $cors_maxage "86400";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.springframework.context.annotation.Configuration;
import org.springframework.core.convert.converter.Converter;
import org.springframework.core.env.Environment;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.security.authentication.AbstractAuthenticationToken;
Expand All @@ -32,11 +33,11 @@
import org.springframework.security.oauth2.jwt.Jwt;
import org.springframework.security.oauth2.jwt.JwtDecoder;
import org.springframework.security.oauth2.server.resource.web.BearerTokenResolver;
import org.springframework.security.oauth2.server.resource.web.DefaultBearerTokenResolver;
import org.springframework.security.web.AuthenticationEntryPoint;
import org.springframework.security.web.access.AccessDeniedHandler;
import org.springframework.security.web.authentication.logout.HttpStatusReturningLogoutSuccessHandler;
import org.springframework.security.web.authentication.logout.LogoutHandler;
import org.springframework.security.web.csrf.CookieCsrfTokenRepository;

@Configuration
@ConditionalOnProperty(
Expand All @@ -46,6 +47,8 @@
@RequiredArgsConstructor
public class DbAuthSecurityConfig extends WebSecurityConfigurerAdapter {

private static final String LOGIN = "/login";

private static final String USER_NOT_FOUND = "User: %s, not found";

private static final String[] SWAGGER_WHITELIST = {
Expand Down Expand Up @@ -73,7 +76,9 @@ protected void configure(HttpSecurity http) throws Exception {
}

http.cors().and()
.csrf().disable()
.csrf(it -> it
.csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse())
.ignoringAntMatchers(LOGIN, DATA_SUBMISSION_ENDPOINT, DATA_SUBMISSION_ENDPOINT_WITH_SLASH))
.oauth2ResourceServer(it -> it
.bearerTokenResolver(bearerTokenResolver())
.authenticationEntryPoint(authenticationEntryPoint)
Expand All @@ -87,7 +92,7 @@ protected void configure(HttpSecurity http) throws Exception {
.requestMatchers(EndpointRequest.toAnyEndpoint()).hasAuthority(UserRole.ADMIN.name())
.antMatchers(HttpMethod.POST, DATA_SUBMISSION_ENDPOINT).permitAll()
.antMatchers(HttpMethod.POST, DATA_SUBMISSION_ENDPOINT_WITH_SLASH).permitAll()
.antMatchers("/login", "/error").permitAll()
.antMatchers(LOGIN, "/error").permitAll()
.anyRequest().authenticated())
.logout(it -> it
.logoutUrl("/user/logout")
Expand Down Expand Up @@ -121,7 +126,7 @@ public UserDetailsService userDetailsServiceBean() {

@Bean
BearerTokenResolver bearerTokenResolver() {
return new DefaultBearerTokenResolver();
return jwtService.createTokenResolver();
}

@Bean
Expand All @@ -136,11 +141,12 @@ Converter<Jwt, AbstractAuthenticationToken> authenticationConverter() {

LogoutHandler logoutHandler() {

return (req, __, ___) -> Try.success(req)
return (req, res, ___) -> Try.success(req)
.map(bearerTokenResolver()::resolve)
.map(jwtDecoder()::decode)
.map(Jwt::getSubject)
.onSuccess(jwtService::invalidateTokensOfUser);
.onSuccess(jwtService::invalidateTokensOfUser)
.onSuccess(__ -> res.addHeader(HttpHeaders.SET_COOKIE, jwtService.createCleanJwtCookie().toString()));
}

private UserAccount findUser(String username) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,28 @@
package iris.client_bff.auth.db.jwt;

import static iris.client_bff.auth.db.jwt.JwtConstants.*;
import static java.util.Optional.*;

import lombok.RequiredArgsConstructor;
import lombok.Value;
import lombok.extern.slf4j.Slf4j;

import java.nio.charset.StandardCharsets;
import java.time.Duration;
import java.time.Instant;
import java.util.Date;
import java.util.List;

import javax.crypto.spec.SecretKeySpec;
import javax.servlet.http.Cookie;
import javax.validation.constraints.NotBlank;
import javax.validation.constraints.NotNull;

import org.apache.commons.codec.digest.DigestUtils;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.ConstructorBinding;
import org.springframework.http.ResponseCookie;
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.oauth2.core.DelegatingOAuth2TokenValidator;
import org.springframework.security.oauth2.core.OAuth2Error;
Expand All @@ -29,8 +34,10 @@
import org.springframework.security.oauth2.jwt.JwtDecoder;
import org.springframework.security.oauth2.jwt.JwtValidators;
import org.springframework.security.oauth2.jwt.NimbusJwtDecoder;
import org.springframework.security.oauth2.server.resource.web.BearerTokenResolver;
import org.springframework.stereotype.Service;
import org.springframework.validation.annotation.Validated;
import org.springframework.web.util.WebUtils;

import com.auth0.jwt.JWT;
import com.auth0.jwt.JWTCreator.Builder;
Expand All @@ -48,6 +55,13 @@ public class JWTService {

private final JWTService.Properties jwtProperties;

public BearerTokenResolver createTokenResolver() {

return request -> ofNullable(WebUtils.getCookie(request, jwtProperties.getCookieName()))
.map(Cookie::getValue)
.orElse(null);
}

public JwtDecoder createJwtDecoder() {

var algorithmName = getAlgorithm().getName();
Expand All @@ -62,25 +76,22 @@ public JwtDecoder createJwtDecoder() {
return decoder;
}

public String createToken(UserDetails user) {

var username = user.getUsername();

// By convention we expect that there exists only one authority and it represents the role
var role = user.getAuthorities().iterator().next().getAuthority();
public ResponseCookie createJwtCookie(UserDetails user) {

var issuedAt = Instant.now();
var expirationTime = issuedAt.plus(EXPIRATION_TIME);
var jwt = createToken(user);

var token = sign(JWT.create()
.withSubject(username)
.withClaim(JWT_CLAIM_USER_ROLE, role)
.withIssuedAt(Date.from(issuedAt))
.withExpiresAt(Date.from(expirationTime)));

saveToken(token, username, expirationTime, issuedAt);
return ResponseCookie.from(jwtProperties.getCookieName(), jwt)
.maxAge(jwtProperties.getExpirationTime().toSeconds())
.secure(jwtProperties.isSetSecure())
.httpOnly(true)
.sameSite(jwtProperties.getSameSiteStr())
.build();
}

return token;
public ResponseCookie createCleanJwtCookie() {
// Without setting maxAge >= 0, we would create a session cookie.
// Setting it to 0 will delete the cookie.
return ResponseCookie.from(jwtProperties.getCookieName(), null).maxAge(0).build();
}

public boolean isTokenWhitelisted(String token) {
Expand Down Expand Up @@ -118,6 +129,27 @@ private OAuth2TokenValidatorResult isTokenWhitelisted(Jwt jwt) {
return OAuth2TokenValidatorResult.failure(error);
}

private String createToken(UserDetails user) {

var username = user.getUsername();

// By convention we expect that there exists only one authority and it represents the role
var role = user.getAuthorities().iterator().next().getAuthority();

var issuedAt = Instant.now();
var expirationTime = issuedAt.plus(jwtProperties.getExpirationTime());

var token = sign(JWT.create()
.withSubject(username)
.withClaim(JWT_CLAIM_USER_ROLE, role)
.withIssuedAt(Date.from(issuedAt))
.withExpiresAt(Date.from(expirationTime)));

saveToken(token, username, expirationTime, issuedAt);

return token;
}

private String sign(Builder builder) {
return builder.sign(getAlgorithm());
}
Expand All @@ -143,6 +175,25 @@ private String hashToken(String jwt) {
@Value
static class Properties {

private @NotBlank String sharedSecret;
@NotBlank
String sharedSecret;

@NotNull
Duration expirationTime;

@NotBlank
String cookieName;

boolean setSecure;

SameSite sameSite;

String getSameSiteStr() {
return getSameSite().name();
}

enum SameSite {
Strict, Lax
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,8 @@
import lombok.AccessLevel;
import lombok.NoArgsConstructor;

import java.time.Duration;

@NoArgsConstructor(access = AccessLevel.PRIVATE)
class JwtConstants {

public static final Duration EXPIRATION_TIME = Duration.ofHours(1);
public static final String JWT_CLAIM_USER_ROLE = "role";
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

import com.fasterxml.jackson.core.exc.StreamReadException;
import com.fasterxml.jackson.databind.DatabindException;

/**
* @author Jens Kutzsche
*/
Expand All @@ -39,16 +36,12 @@
@Slf4j
public class LoginController {

public static final String AUTHENTICATION_INFO = "Authentication-Info";
public static final String BEARER_TOKEN_PREFIX = "Bearer ";

private final AuthenticationManager authManager;
private final LoginAttemptsService loginAttempts;
private final JWTService jwtService;

@PostMapping("/login")
public ResponseEntity<?> login(@RequestBody LoginRequestDto login, HttpServletRequest req)
throws StreamReadException, DatabindException, IOException {
public ResponseEntity<?> login(@RequestBody LoginRequestDto login, HttpServletRequest req) throws IOException {

log.debug("Login request from remote address: " + LogHelper.obfuscateLastThree(req.getRemoteAddr()));

Expand All @@ -66,20 +59,11 @@ public ResponseEntity<?> login(@RequestBody LoginRequestDto login, HttpServletRe

var user = (UserDetails) authManager.authenticate(authToken).getPrincipal();

var token = jwtService.createToken(user);
var jwtCookie = jwtService.createJwtCookie(user);

var headers = new HttpHeaders();
headers.add(AUTHENTICATION_INFO, BEARER_TOKEN_PREFIX + token);
headers.add(HttpHeaders.ACCESS_CONTROL_EXPOSE_HEADERS, AUTHENTICATION_INFO);

return ResponseEntity.ok().headers(headers).body(new LoginResponseDto(token));
return ResponseEntity.ok().header(HttpHeaders.SET_COOKIE, jwtCookie.toString()).build();
}

// @ExceptionHandler
// ResponseEntity<?> heandleLockedException(LockedException e) {
// return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(e.getMessage());
// }

static record LoginRequestDto(@NotBlank String userName, @NotBlank String password) {}

static record LoginResponseDto(String token) {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,10 @@ public WebMvcConfigurer configureCorsForLocalDev() {
public void addCorsMappings(CorsRegistry registry) {
registry
.addMapping("/**")
.allowedOrigins("*")
.allowedOriginPatterns("*")
.allowedMethods("*")
.allowedHeaders("*");
.allowedHeaders("*")
.allowCredentials(true);
}
};
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package iris.client_bff.core.web.error;

import iris.client_bff.auth.db.jwt.JWTService;
import lombok.RequiredArgsConstructor;

import java.io.IOException;
Expand All @@ -8,6 +9,7 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.springframework.http.HttpHeaders;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.core.AuthenticationException;
import org.springframework.security.oauth2.server.resource.web.BearerTokenAuthenticationEntryPoint;
Expand All @@ -25,6 +27,8 @@ class GlobalFilterExceptionHandler implements AuthenticationEntryPoint, AccessDe

private final IrisErrorAttributes errorAttributes;

private final JWTService jwtService;

/**
* handle authentication error
*/
Expand All @@ -33,6 +37,10 @@ public void commence(HttpServletRequest request, HttpServletResponse response, A
throws IOException, ServletException {

authenticationEntryPoint.commence(request, response, authException);

// If there is an existing http only cookie containing an invalid token, it has to be deleted.
// Otherwise, the user won't be able to login, because the invalid token gets rejected with every login request.
response.addHeader(HttpHeaders.SET_COOKIE, jwtService.createCleanJwtCookie().toString());
response.sendError(response.getStatus());

errorAttributes.resolveException(request, response, authenticationEntryPoint, authException);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
security.auth=db
security.auth.db.admin-user-name=admin
security.auth.db.admin-user-password=admin

security.jwt.set-secure=false
4 changes: 4 additions & 0 deletions iris-client-bff/src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ spring.jackson.default-property-inclusion=NON_ABSENT
springdoc.api-docs.enabled=false

security.jwt.shared-secret=${random.value}${random.value}
security.jwt.expiration-time=1h
security.jwt.cookie-name=IRIS_JWT
security.jwt.set-secure=true
security.jwt.same-site=Strict
# Durations >0 like descripted in https://docs.spring.io/spring-boot/docs/current/reference/html/features.html#features.external-config.typesafe-configuration-properties.conversion
security.login.attempts.first-waiting-time=3s
security.login.attempts.ignore-old-attempts-after=1h
Expand Down
Loading

0 comments on commit ae25da8

Please sign in to comment.