Skip to content

Commit

Permalink
Add a User-Agent challenge when validating tokens
Browse files Browse the repository at this point in the history
  • Loading branch information
arteymix committed Aug 23, 2023
1 parent f85d5f5 commit ae21210
Show file tree
Hide file tree
Showing 14 changed files with 205 additions and 38 deletions.
7 changes: 7 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,13 @@
<version>1.10.0</version>
</dependency>

<!-- User-Agent header parsing -->
<dependency>
<groupId>nl.basjes.parse.useragent</groupId>
<artifactId>yauaa</artifactId>
<version>7.22.0</version>
</dependency>

<!-- Testing Support -->

<dependency>
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/ubc/pavlab/rdp/SecureTokenConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import ubc.pavlab.rdp.security.SecureTokenChallenge;
import ubc.pavlab.rdp.security.UserAgentChallenge;

import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
Expand All @@ -18,4 +20,9 @@ public class SecureTokenConfig {
public SecureRandom secureRandom() throws NoSuchAlgorithmException {
return SecureRandom.getInstance( "SHA1PRNG" );
}

@Bean
public SecureTokenChallenge secureTokenChallenge() {
return new UserAgentChallenge();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import ubc.pavlab.rdp.services.UserService;
import ubc.pavlab.rdp.settings.ApplicationSettings;

import javax.servlet.http.HttpServletRequest;
import java.util.Locale;

/**
Expand Down Expand Up @@ -132,9 +133,10 @@ public ModelAndView resendConfirmation( @RequestParam("email") String email, Loc

@GetMapping(value = "/registrationConfirm")
public ModelAndView confirmRegistration( @RequestParam("token") String token,
HttpServletRequest request,
RedirectAttributes redirectAttributes ) {
try {
userService.confirmVerificationToken( token );
userService.confirmVerificationToken( token, request );
redirectAttributes.addFlashAttribute( "message", "Your account has been enabled successfully, and you can now proceed to login." );
return new ModelAndView( "redirect:/login" );
} catch ( TokenException e ) {
Expand Down
10 changes: 6 additions & 4 deletions src/main/java/ubc/pavlab/rdp/controllers/PasswordController.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import ubc.pavlab.rdp.model.UserPrinciple;
import ubc.pavlab.rdp.services.UserService;

import javax.servlet.http.HttpServletRequest;
import javax.validation.Valid;
import java.util.Locale;

Expand Down Expand Up @@ -59,11 +60,11 @@ public ModelAndView resetPassword( @RequestParam("email") String userEmail, Loca
}

@GetMapping(value = "/updatePassword")
public ModelAndView changePassword( @RequestParam("id") int id, @RequestParam("token") String token ) {
public ModelAndView changePassword( @RequestParam("id") int id, @RequestParam("token") String token, HttpServletRequest request ) {
ModelAndView modelAndView = new ModelAndView( "updatePassword" );

try {
userService.verifyPasswordResetToken( id, token );
userService.verifyPasswordResetToken( id, token, request );
} catch ( TokenException e ) {
modelAndView.setStatus( HttpStatus.BAD_REQUEST );
modelAndView.addObject( "error", e.getMessage() );
Expand All @@ -78,7 +79,8 @@ public ModelAndView changePassword( @RequestParam("id") int id, @RequestParam("t

@PostMapping(value = "/updatePassword")
public ModelAndView showChangePasswordPage( @RequestParam("id") int id, @RequestParam("token") String token,
@Valid PasswordReset passwordReset, BindingResult bindingResult ) {
@Valid PasswordReset passwordReset, BindingResult bindingResult,
HttpServletRequest request ) {
ModelAndView modelAndView = new ModelAndView( "updatePassword" );

if ( !passwordReset.isValid() ) {
Expand All @@ -93,7 +95,7 @@ public ModelAndView showChangePasswordPage( @RequestParam("id") int id, @Request
}

try {
userService.changePasswordByResetToken( id, token, passwordReset );
userService.changePasswordByResetToken( id, token, passwordReset, request );
} catch ( TokenException e ) {
modelAndView.setStatus( HttpStatus.BAD_REQUEST );
modelAndView.addObject( "message", e.getMessage() );
Expand Down
15 changes: 10 additions & 5 deletions src/main/java/ubc/pavlab/rdp/controllers/UserController.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package ubc.pavlab.rdp.controllers;

import lombok.*;
import lombok.Builder;
import lombok.Data;
import lombok.EqualsAndHashCode;
import lombok.extern.apachecommons.CommonsLog;
import org.apache.commons.lang3.StringUtils;
import lombok.extern.jackson.Jacksonized;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.MessageSource;
import org.springframework.http.HttpStatus;
Expand All @@ -13,7 +14,10 @@
import org.springframework.security.access.annotation.Secured;
import org.springframework.security.authentication.BadCredentialsException;
import org.springframework.stereotype.Controller;
import org.springframework.validation.*;
import org.springframework.validation.BindingResult;
import org.springframework.validation.Errors;
import org.springframework.validation.FieldError;
import org.springframework.validation.Validator;
import org.springframework.web.bind.WebDataBinder;
import org.springframework.web.bind.annotation.*;
import org.springframework.web.multipart.MultipartFile;
Expand All @@ -28,6 +32,7 @@
import ubc.pavlab.rdp.settings.ApplicationSettings;

import javax.mail.MessagingException;
import javax.servlet.http.HttpServletRequest;
import javax.validation.Valid;
import javax.validation.constraints.NotNull;
import javax.validation.constraints.Size;
Expand Down Expand Up @@ -311,9 +316,9 @@ public Object resendContactEmailVerification( RedirectAttributes redirectAttribu
}

@GetMapping("/user/verify-contact-email")
public String verifyContactEmail( @RequestParam String token, RedirectAttributes redirectAttributes ) {
public String verifyContactEmail( @RequestParam String token, RedirectAttributes redirectAttributes, HttpServletRequest request ) {
try {
userService.confirmVerificationToken( token );
userService.confirmVerificationToken( token, request );
redirectAttributes.addFlashAttribute( "message", "Your contact email has been successfully verified." );
} catch ( TokenException e ) {
log.warn( String.format( "%s attempt to confirm verification token failed: %s.", userService.findCurrentUser(), e.getMessage() ) );
Expand Down
18 changes: 18 additions & 0 deletions src/main/java/ubc/pavlab/rdp/security/SecureTokenChallenge.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package ubc.pavlab.rdp.security;

import ubc.pavlab.rdp.exception.TokenException;
import ubc.pavlab.rdp.model.Token;

public interface SecureTokenChallenge<T> {

/**
* @param token the token being challenged
* @throws TokenException
*/
void challenge( Token token, T object ) throws TokenException;

/**
* Indicate if the challenge supports the given class.
*/
boolean supports( Class<?> clazz );
}
56 changes: 56 additions & 0 deletions src/main/java/ubc/pavlab/rdp/security/UserAgentChallenge.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package ubc.pavlab.rdp.security;

import nl.basjes.parse.useragent.UserAgent;
import nl.basjes.parse.useragent.UserAgentAnalyzer;
import org.apache.commons.lang3.StringUtils;
import org.springframework.stereotype.Component;
import ubc.pavlab.rdp.exception.TokenException;
import ubc.pavlab.rdp.model.Token;

import javax.servlet.http.HttpServletRequest;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;

/**
* Challenge the token by checking the User-Agent header.
*
* @author poirigui
*/
@Component
public class UserAgentChallenge implements SecureTokenChallenge<HttpServletRequest> {

private final static Set<String> ACCEPTED_AGENT_CLASSES = new HashSet<>( Arrays.asList( "Browser", "Browser Webview" ) );

private final UserAgentAnalyzer userAgentAnalyzer;

public UserAgentChallenge() {
this.userAgentAnalyzer = UserAgentAnalyzer
.newBuilder()
.withField( "AgentClass" )
.useJava8CompatibleCaching()
.withCache( 10000 )
.build();
}

@Override
public void challenge( Token token, HttpServletRequest request ) throws TokenException {
String userAgent = request.getHeader( "User-Agent" );
if ( StringUtils.isBlank( userAgent ) ) {
throw new TokenException( "The User-Agent header is missing or blank." );
}
// unfortunately, Yauaa is not thread safe
UserAgent.ImmutableUserAgent ua;
synchronized ( userAgentAnalyzer ) {
ua = userAgentAnalyzer.parse( userAgent );
}
if ( !ACCEPTED_AGENT_CLASSES.contains( ua.get( "AgentClass" ).getValue() ) ) {
throw new TokenException( "Unacceptable User-Agent header." );
}
}

@Override
public boolean supports( Class<?> clazz ) {
return HttpServletRequest.class.isAssignableFrom( clazz );
}
}
7 changes: 4 additions & 3 deletions src/main/java/ubc/pavlab/rdp/services/UserService.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import ubc.pavlab.rdp.model.ontology.Ontology;
import ubc.pavlab.rdp.model.ontology.OntologyTermInfo;

import javax.servlet.http.HttpServletRequest;
import javax.validation.ValidationException;
import java.util.*;
import java.util.function.Predicate;
Expand Down Expand Up @@ -182,15 +183,15 @@ User updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( User user,

PasswordResetToken createPasswordResetTokenForUser( User user, Locale locale );

PasswordResetToken verifyPasswordResetToken( int userId, String token ) throws TokenException;
PasswordResetToken verifyPasswordResetToken( int userId, String token, HttpServletRequest request ) throws TokenException;

User changePasswordByResetToken( int userId, String token, PasswordReset passwordReset ) throws TokenException;
User changePasswordByResetToken( int userId, String token, PasswordReset passwordReset, HttpServletRequest request ) throws TokenException;

VerificationToken createVerificationTokenForUser( User user, Locale locale );

VerificationToken createContactEmailVerificationTokenForUser( User user, Locale locale );

User confirmVerificationToken( String token ) throws TokenException;
User confirmVerificationToken( String token, HttpServletRequest request ) throws TokenException;

SortedSet<String> getLastNamesFirstChar();

Expand Down
17 changes: 13 additions & 4 deletions src/main/java/ubc/pavlab/rdp/services/UserServiceImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,13 @@
import ubc.pavlab.rdp.model.ontology.OntologyTermInfo;
import ubc.pavlab.rdp.model.ontology.UserOntologyTerm;
import ubc.pavlab.rdp.repositories.*;
import ubc.pavlab.rdp.security.SecureTokenChallenge;
import ubc.pavlab.rdp.settings.ApplicationSettings;
import ubc.pavlab.rdp.util.CacheUtils;
import ubc.pavlab.rdp.util.CollectionUtils;
import ubc.pavlab.rdp.util.Messages;

import javax.servlet.http.HttpServletRequest;
import javax.validation.ValidationException;
import java.security.SecureRandom;
import java.text.MessageFormat;
Expand Down Expand Up @@ -100,6 +102,8 @@ public class UserServiceImpl implements UserService, InitializingBean {
private SecureRandom secureRandom;
@Autowired
private OntologyService ontologyService;
@Autowired
private SecureTokenChallenge<HttpServletRequest> secureTokenChallenge;

private Cache usersByAnonymousIdCache;
private Cache userGenesByAnonymousIdCache;
Expand Down Expand Up @@ -804,13 +808,15 @@ public PasswordResetToken createPasswordResetTokenForUser( User user, Locale loc
}

@Override
public PasswordResetToken verifyPasswordResetToken( int userId, String token ) throws TokenException {
public PasswordResetToken verifyPasswordResetToken( int userId, String token, HttpServletRequest request ) throws TokenException {
PasswordResetToken passToken = passwordResetTokenRepository.findByToken( token );

if ( passToken == null ) {
throw new TokenException( "Password reset token is invalid." );
}

secureTokenChallenge.challenge( passToken, request );

if ( !passToken.getUser().getId().equals( userId ) ) {
throw new TokenException( "Password reset token is invalid." );
}
Expand All @@ -824,9 +830,9 @@ public PasswordResetToken verifyPasswordResetToken( int userId, String token ) t

@Transactional(rollbackFor = { TokenException.class })
@Override
public User changePasswordByResetToken( int userId, String token, PasswordReset passwordReset ) throws TokenException, ValidationException {
public User changePasswordByResetToken( int userId, String token, PasswordReset passwordReset, HttpServletRequest request ) throws TokenException, ValidationException {

PasswordResetToken passToken = verifyPasswordResetToken( userId, token );
PasswordResetToken passToken = verifyPasswordResetToken( userId, token, request );

// Preauthorize might cause trouble here if implemented, fix by setting manual authentication
User user = findUserByIdNoAuth( userId );
Expand Down Expand Up @@ -864,12 +870,15 @@ public VerificationToken createContactEmailVerificationTokenForUser( User user,

@Override
@Transactional(rollbackFor = { TokenException.class })
public User confirmVerificationToken( String token ) throws TokenException {
public User confirmVerificationToken( String token, HttpServletRequest request ) throws TokenException {
VerificationToken verificationToken = tokenRepository.findByToken( token );

if ( verificationToken == null ) {
throw new TokenNotFoundException( "Verification token is invalid." );
}

secureTokenChallenge.challenge( verificationToken, request );

if ( Instant.now().isAfter( verificationToken.getExpiryDate() ) ) {
throw new ExpiredTokenException( "Verification token is expired." );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

import static org.hamcrest.Matchers.containsString;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
Expand Down Expand Up @@ -163,7 +164,7 @@ public void resendConfirmation_thenReturnSuccess() throws Exception {
@Test
public void registrationConfirm_thenReturnSuccess() throws Exception {
User user = createUser( 1 );
when( userService.confirmVerificationToken( "1234" ) ).thenReturn( user );
when( userService.confirmVerificationToken( eq( "1234" ), any() ) ).thenReturn( user );
mvc.perform( get( "/registrationConfirm" )
.param( "token", "1234" ) )
.andExpect( status().is3xxRedirection() )
Expand All @@ -174,7 +175,7 @@ public void registrationConfirm_thenReturnSuccess() throws Exception {

@Test
public void registrationConfirm_whenTokenDoesNotExist_thenRedirectToLoginWithMessage() throws Exception {
when( userService.confirmVerificationToken( "1234" ) ).thenThrow( TokenNotFoundException.class );
when( userService.confirmVerificationToken( eq( "1234" ), any() ) ).thenThrow( TokenNotFoundException.class );
mvc.perform( get( "/registrationConfirm" )
.param( "token", "1234" ) )
.andExpect( status().is3xxRedirection() )
Expand All @@ -185,7 +186,7 @@ public void registrationConfirm_whenTokenDoesNotExist_thenRedirectToLoginWithMes

@Test
public void registrationConfirm_whenTokenIsExpired_thenRedirectToResendConfirmation() throws Exception {
when( userService.confirmVerificationToken( "1234" ) ).thenThrow( ExpiredTokenException.class );
when( userService.confirmVerificationToken( eq( "1234" ), any() ) ).thenThrow( ExpiredTokenException.class );
mvc.perform( get( "/registrationConfirm" )
.param( "token", "1234" ) )
.andExpect( status().is3xxRedirection() )
Expand All @@ -196,7 +197,7 @@ public void registrationConfirm_whenTokenIsExpired_thenRedirectToResendConfirmat

@Test
public void registrationConfirm_whenTokenIsInvalid_thenReturn400() throws Exception {
when( userService.confirmVerificationToken( "1234" ) ).thenThrow( TokenDoesNotMatchEmailException.class );
when( userService.confirmVerificationToken( eq( "1234" ), any() ) ).thenThrow( TokenDoesNotMatchEmailException.class );
mvc.perform( get( "/registrationConfirm" )
.param( "token", "1234" ) )
.andExpect( status().isBadRequest() )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@
import ubc.pavlab.rdp.model.PasswordReset;
import ubc.pavlab.rdp.model.User;
import ubc.pavlab.rdp.repositories.UserRepository;
import ubc.pavlab.rdp.security.SecureTokenChallenge;
import ubc.pavlab.rdp.services.EmailService;
import ubc.pavlab.rdp.services.UserDetailsServiceImpl;
import ubc.pavlab.rdp.services.UserService;
import ubc.pavlab.rdp.settings.ApplicationSettings;
import ubc.pavlab.rdp.settings.SiteSettings;

import javax.servlet.http.HttpServletRequest;
import java.util.Locale;

import static org.mockito.Mockito.*;
Expand Down Expand Up @@ -67,6 +69,9 @@ public BCryptPasswordEncoder bCryptPasswordEncoder() {
@MockBean(name = "ontologyMessageSource")
private MessageSource ontologyMessageSource;

@MockBean
private SecureTokenChallenge<HttpServletRequest> secureTokenChallenge;

@Test
public void forgotPassword_thenReturnSuccess() throws Exception {
mvc.perform( get( "/forgotPassword" ) )
Expand Down Expand Up @@ -105,7 +110,7 @@ public void updatePassword_thenReturnSuccess() throws Exception {
.andExpect( status().isOk() )
.andExpect( view().name( "updatePassword" ) );

verify( userService ).verifyPasswordResetToken( 1, "1234" );
verify( userService ).verifyPasswordResetToken( eq( 1 ), eq( "1234" ), any() );

mvc.perform( post( "/updatePassword" )
.param( "id", "1" )
Expand All @@ -115,7 +120,7 @@ public void updatePassword_thenReturnSuccess() throws Exception {
.andExpect( status().is3xxRedirection() )
.andExpect( redirectedUrl( "/user/home" ) );

verify( userService ).changePasswordByResetToken( 1, "1234", new PasswordReset( "123456", "123456" ) );
verify( userService ).changePasswordByResetToken( eq( 1 ), eq( "1234" ), eq( new PasswordReset( "123456", "123456" ) ), any() );
}

@Test
Expand Down
Loading

0 comments on commit ae21210

Please sign in to comment.