Skip to content

Commit

Permalink
Add a secret to the AccessToken
Browse files Browse the repository at this point in the history
  • Loading branch information
arteymix committed Aug 26, 2023
1 parent a978653 commit 3e6ba6f
Show file tree
Hide file tree
Showing 12 changed files with 94 additions and 35 deletions.
14 changes: 9 additions & 5 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,16 @@ the [International data](customization.md#international-data)
customization section.

For external services, authentication should instead be performed with using a [service account](service-accounts.md)
with a secret access token.
with an access token and a secret.

```http
GET /api/** HTTP/1.1
Authorization: Bearer {accessToken}
Authorization: Bearer {accessToken}:{secret}
```

Tokens created prior to the 1.6 release may omit the `:{secret}` part. We highly recommend that you revoke and
regenerate new tokens with secrets.

Passing the authorization token via `auth` query parameter is deprecated as of 1.4.0.

```http
Expand Down Expand Up @@ -160,7 +163,6 @@ GET /api/ontologies/{ontologyName}/terms HTTP/1.1

- `page` the page to query starting from zero to `totalPages`


## List specific terms in a category/ontology

!!! note
Expand All @@ -171,8 +173,10 @@ GET /api/ontologies/{ontologyName}/terms HTTP/1.1
GET /api/ontologies/{ontologyName}/terms?ontologyTermIds HTTP/1.1
```

To retrieve specific terms, you may use `ontologyTermIds` query parameter and pass it as many time as you want. The output is
not paginated and the `page` parameter from [List all terms in a category/ontology](#list-all-terms-in-a-categoryontology) is ignored.
To retrieve specific terms, you may use `ontologyTermIds` query parameter and pass it as many time as you want. The
output is
not paginated and the `page` parameter
from [List all terms in a category/ontology](#list-all-terms-in-a-categoryontology) is ignored.

## Retrieve a single category/ontology term

Expand Down
14 changes: 9 additions & 5 deletions src/main/java/ubc/pavlab/rdp/controllers/AdminController.java
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public ModelAndView viewCreateServiceAccount() {
}

@PostMapping(value = "/admin/create-service-account")
public Object createServiceAccount( @Validated(User.ValidationServiceAccount.class) User user, BindingResult bindingResult ) {
public Object createServiceAccount( @Validated(User.ValidationServiceAccount.class) User user, BindingResult bindingResult, RedirectAttributes redirectAttributes ) {
String serviceEmail = user.getEmail() + '@' + siteSettings.getHostUrl().getHost();

if ( userService.findUserByEmailNoAuth( serviceEmail ) != null ) {
Expand All @@ -136,9 +136,13 @@ public Object createServiceAccount( @Validated(User.ValidationServiceAccount.cla
profile.setContactEmailVerified( false );
user.setProfile( profile );

user = userService.createServiceAccount( user );
UserService.ServiceAccountAndAccessToken userAndAccessToken = userService.createServiceAccount( user );

return "redirect:/admin/users/" + user.getId();
redirectAttributes.addFlashAttribute( "message", String.format( "Created service account with token %s and secret %s.",
userAndAccessToken.getAccessTokenWithRawSecret().getAccessToken().getToken(),
userAndAccessToken.getAccessTokenWithRawSecret().getRawSecret() ) );

return "redirect:/admin/users/" + userAndAccessToken.getUser().getId();
}

@PostMapping(value = "/admin/users/{user}/roles")
Expand Down Expand Up @@ -184,8 +188,8 @@ public Object createAccessTokenForUser( @PathVariable(required = false) User use
return new ModelAndView( "error/404", HttpStatus.NOT_FOUND )
.addObject( "message", messageSource.getMessage( "AdminController.userNotFoundById", null, locale ) );
}
AccessToken accessToken = userService.createAccessTokenForUser( user );
redirectAttributes.addFlashAttribute( "message", MessageFormat.format( "Successfully created an access token {0}.", accessToken.getToken() ) );
UserService.AccessTokenWithRawSecret accessToken = userService.createAccessTokenForUser( user );
redirectAttributes.addFlashAttribute( "message", MessageFormat.format( "Successfully created an access token {0} with secret {1}.", accessToken.getAccessToken().getToken(), accessToken.getRawSecret() ) );
return "redirect:/admin/users/" + user.getId();
}

Expand Down
2 changes: 2 additions & 0 deletions src/main/java/ubc/pavlab/rdp/model/AccessToken.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ public class AccessToken extends Token implements UserContent {
@JoinColumn(name = "user_id", nullable = false)
private User user;

private String secret;

@CreatedDate
private Instant createdAt;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,21 @@
public class TokenBasedAuthentication extends AbstractAuthenticationToken {

private final String token;
private final String secret;

public TokenBasedAuthentication( String token ) {
public TokenBasedAuthentication( String token, String secret ) {
super( null );
this.token = token;
this.secret = secret;
}

@Override
public Object getCredentials() {
return token;
return secret;
}

@Override
public Object getPrincipal() {
return null;
return token;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class TokenBasedAuthenticationConverter implements AuthenticationConverter {
public final TokenBasedAuthentication convert( HttpServletRequest request ) throws IllegalArgumentException {
String authorizationHeader = request.getHeader( "Authorization" );
String authToken = request.getParameter( "auth" );
String secret;
if ( authToken == null && authorizationHeader != null ) {
String[] pieces = authorizationHeader.split( " ", 2 );
if ( pieces[0].equalsIgnoreCase( "Bearer" ) ) {
Expand All @@ -28,6 +29,13 @@ public final TokenBasedAuthentication convert( HttpServletRequest request ) thro
return null; /* unsupported authentication scheme */
}
}
return authToken != null ? new TokenBasedAuthentication( authToken ) : null;
if ( authToken != null && authToken.contains( ":" ) ) {
String[] pieces = authToken.split( ":", 2 );
authToken = pieces[0];
secret = pieces[1];
} else {
secret = null;
}
return authToken != null ? new TokenBasedAuthentication( authToken, secret ) : null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ public TokenBasedAuthenticationManager( UserService userService, ApplicationSett

@Override
public Authentication authenticate( Authentication authentication ) throws AuthenticationException {
String authToken = (String) authentication.getCredentials();
String authToken = (String) authentication.getPrincipal();
String secret = (String) authentication.getCredentials();
User u;
if ( applicationSettings.getIsearch().getAuthTokens().contains( authToken ) ) {
// remote admin authentication
Expand All @@ -42,7 +43,7 @@ public Authentication authenticate( Authentication authentication ) throws Authe
} else {
// authentication via access token
try {
u = userService.findUserByAccessTokenNoAuth( authToken );
u = userService.findUserByAccessTokenNoAuth( authToken, secret );
} catch ( ExpiredTokenException e ) {
throw new CredentialsExpiredException( "API token is expired.", e );
} catch ( TokenException e ) {
Expand Down
19 changes: 16 additions & 3 deletions src/main/java/ubc/pavlab/rdp/services/UserService.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package ubc.pavlab.rdp.services;

import lombok.Value;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.security.authentication.BadCredentialsException;
Expand All @@ -26,7 +27,13 @@ public interface UserService {

User createAdmin( User admin );

User createServiceAccount( User user );
ServiceAccountAndAccessToken createServiceAccount( User user );

@Value
class ServiceAccountAndAccessToken {
User user;
AccessTokenWithRawSecret accessTokenWithRawSecret;
}

List<Role> findAllRoles();

Expand Down Expand Up @@ -61,7 +68,7 @@ public interface UserService {

User findUserByEmailNoAuth( String email );

User findUserByAccessTokenNoAuth( String accessToken ) throws TokenException;
User findUserByAccessTokenNoAuth( String accessToken, String secret ) throws TokenException;

/**
* Anonymize the given user.
Expand Down Expand Up @@ -113,7 +120,13 @@ public interface UserService {

void revokeAccessToken( AccessToken accessToken );

AccessToken createAccessTokenForUser( User user );
AccessTokenWithRawSecret createAccessTokenForUser( User user );

@Value
class AccessTokenWithRawSecret {
AccessToken accessToken;
String rawSecret;
}

Optional<User> getRemoteSearchUser();

Expand Down
17 changes: 11 additions & 6 deletions src/main/java/ubc/pavlab/rdp/services/UserServiceImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,13 @@ public User createAdmin( User admin ) {
@Override
@Secured("ROLE_ADMIN")
@Transactional
public User createServiceAccount( User user ) {
public ServiceAccountAndAccessToken createServiceAccount( User user ) {
user.setPassword( bCryptPasswordEncoder.encode( createSecureRandomToken() ) );
Role serviceAccountRole = roleRepository.findByRole( "ROLE_SERVICE_ACCOUNT" );
user.getRoles().add( serviceAccountRole );
user = userRepository.save( user );
createAccessTokenForUser( user );
return user;
AccessTokenWithRawSecret accessToken = createAccessTokenForUser( user );
return new ServiceAccountAndAccessToken( user, accessToken );
}

@Override
Expand Down Expand Up @@ -320,7 +320,7 @@ public User findUserByEmailNoAuth( String email ) {
}

@Override
public User findUserByAccessTokenNoAuth( String accessToken ) throws TokenException {
public User findUserByAccessTokenNoAuth( String accessToken, String secret ) throws TokenException {
AccessToken token = accessTokenRepository.findByToken( accessToken );
if ( token == null ) {
return null;
Expand All @@ -329,6 +329,9 @@ public User findUserByAccessTokenNoAuth( String accessToken ) throws TokenExcept
// token is expired
throw new ExpiredTokenException( "Token is expired." );
}
if ( token.getSecret() != null && !bCryptPasswordEncoder.matches( secret, token.getSecret() ) ) {
throw new TokenException( "Provided secret does not match the token." );
}
return token.getUser();
}

Expand Down Expand Up @@ -386,11 +389,13 @@ public void revokeAccessToken( AccessToken accessToken ) {
}

@Override
public AccessToken createAccessTokenForUser( User user ) {
public AccessTokenWithRawSecret createAccessTokenForUser( User user ) {
AccessToken token = new AccessToken();
token.updateToken( createSecureRandomToken() );
token.setUser( user );
return accessTokenRepository.save( token );
String secret = createSecureRandomToken();
token.setSecret( bCryptPasswordEncoder.encode( secret ) );
return new AccessTokenWithRawSecret( accessTokenRepository.save( token ), secret );
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
alter table access_token
add column secret varchar(255); -- may be null for pre-1.6 tokens
16 changes: 11 additions & 5 deletions src/test/java/ubc/pavlab/rdp/controllers/AdminControllerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
import org.springframework.security.test.context.support.WithMockUser;
import org.springframework.test.context.TestPropertySource;
import org.springframework.test.context.junit4.SpringRunner;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.MvcResult;
import org.springframework.util.LinkedMultiValueMap;
Expand Down Expand Up @@ -181,13 +180,17 @@ public void givenLoggedIn_whenCreateServiceAccount_thenRedirect3xx() throws Exce
when( userService.createServiceAccount( any() ) ).thenAnswer( answer -> {
User createdUser = answer.getArgument( 0, User.class );
createdUser.setId( 1 );
return createdUser;
AccessToken accessToken = new AccessToken();
accessToken.setToken( "1234" );
return new UserService.ServiceAccountAndAccessToken( createdUser, new UserService.AccessTokenWithRawSecret( accessToken, "5678" ) );
} );
mvc.perform( post( "/admin/create-service-account" )
.param( "profile.name", "Service Account" )
.param( "email", "service-account" ) )
.andExpect( status().is3xxRedirection() )
.andExpect( redirectedUrl( "/admin/users/1" ) );
.andExpect( redirectedUrl( "/admin/users/1" ) )
.andExpect( flash().attribute( "message", containsString( "1234" ) ) )
.andExpect( flash().attribute( "message", containsString( "5678" ) ) );
ArgumentCaptor<User> captor = ArgumentCaptor.forClass( User.class );
verify( userService ).createServiceAccount( captor.capture() );
assertThat( captor.getValue() )
Expand All @@ -200,12 +203,15 @@ public void givenLoggedIn_whenCreateServiceAccount_thenRedirect3xx() throws Exce
public void givenLoggedIn_whenCreateAccessToken_thenRedirect3xx() throws Exception {
User user = createUser( 1 );
AccessToken accessToken = TestUtils.createAccessToken( 1, user, "1234" );
UserService.AccessTokenWithRawSecret accessTokenWithSecret = new UserService.AccessTokenWithRawSecret( accessToken, "5678" );
when( userService.findUserById( 1 ) ).thenReturn( user );
when( userService.createAccessTokenForUser( user ) ).thenReturn( accessToken );
when( userService.createAccessTokenForUser( user ) ).thenReturn( accessTokenWithSecret );
when( roleRepository.findByRole( "ROLE_USER" ) ).thenReturn( createRole( 1, "ROLE_USER" ) );
mvc.perform( post( "/admin/users/{user}/create-access-token", user.getId() ) )
.andExpect( status().is3xxRedirection() )
.andExpect( redirectedUrl( "/admin/users/1" ) );
.andExpect( redirectedUrl( "/admin/users/1" ) )
.andExpect( flash().attribute( "message", containsString( "1234" ) ) )
.andExpect( flash().attribute( "message", containsString( "5678" ) ) );
verify( userService ).createAccessTokenForUser( user );
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,30 @@ public class TokenBasedAuthenticationConverterTest {

@Test
public void convert_withAuthorizationHeader() {
MockHttpServletRequest request = request( HttpMethod.GET, "/" )
.header( "Authorization", "Bearer 1234:5678" )
.buildRequest( servletContext );
assertThat( new TokenBasedAuthenticationConverter().convert( request ) )
.isNotNull()
.hasFieldOrPropertyWithValue( "principal", "1234" )
.hasFieldOrPropertyWithValue( "credentials", "5678" );
}

@Test
public void convert_withAuthorizationHeaderWithoutSecret() {
MockHttpServletRequest request = request( HttpMethod.GET, "/" )
.header( "Authorization", "Bearer 1234" )
.buildRequest( servletContext );
assertThat( new TokenBasedAuthenticationConverter().convert( request ) )
.isNotNull()
.hasFieldOrPropertyWithValue( "credentials", "1234" );
.hasFieldOrPropertyWithValue( "principal", "1234" )
.hasFieldOrPropertyWithValue( "credentials", null );
}

@Test
public void convert_withUnsupportedAuthenticationScheme_thenIgnoreAndReturnNull() {
MockHttpServletRequest request = request( HttpMethod.GET, "/" )
.header( "Authorization", "Basic 1234" )
.header( "Authorization", "Basic 1234:5678" )
.buildRequest( servletContext );
assertThat( new TokenBasedAuthenticationConverter().convert( request ) )
.isNull();
Expand All @@ -40,11 +52,11 @@ public void convert_withAuthRequestParam() {
.buildRequest( servletContext );
assertThat( new TokenBasedAuthenticationConverter().convert( request ) )
.isNotNull()
.hasFieldOrPropertyWithValue( "credentials", "1234" );
.hasFieldOrPropertyWithValue( "principal", "1234" );
}

@Test
public void convert_whenCredentialsAreMissing_thenReturnNull() {
public void convert_whenPrincipalIsMissing_thenReturnNull() {
MockHttpServletRequest request = request( HttpMethod.GET, "/" )
.buildRequest( servletContext );
assertThat( new TokenBasedAuthenticationConverter().convert( request ) )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void authenticate_whenRemoteUserIsRemovedAtRuntime() {
when( userService.getRemoteSearchUser() ).thenReturn( Optional.empty() );

Authentication auth = mock( Authentication.class );
when( auth.getCredentials() ).thenReturn( "test" );
when( auth.getPrincipal() ).thenReturn( "test" );
assertThatThrownBy( () -> manager.authenticate( auth ) )
.isInstanceOf( InternalAuthenticationServiceException.class );
}
Expand Down

0 comments on commit 3e6ba6f

Please sign in to comment.