Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a secret to the AccessToken #389

Draft
wants to merge 1 commit into
base: development
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading