Skip to content

Commit

Permalink
JAMES-1644 rework exceptions handling to return different status when…
Browse files Browse the repository at this point in the history
… a bad continuation token is given

git-svn-id: https://svn.apache.org/repos/asf/james/project/trunk@1719321 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
mbaechler committed Dec 11, 2015
1 parent 54e7866 commit 9350d8d
Show file tree
Hide file tree
Showing 9 changed files with 173 additions and 61 deletions.
Expand Up @@ -139,15 +139,17 @@ private void handleContinuationTokenRequest(ContinuationTokenRequest request, Ht
}

private void handleAccessTokenRequest(AccessTokenRequest request, HttpServletResponse resp) throws IOException {
try {
if (!continuationTokenManager.isValid(request.getToken())) {
LOG.warn("Use of an invalid ContinuationToken : " + request.getToken().serialize());
returnUnauthorizedResponse(resp);
} else {
manageAuthenticationResponse(request, resp);
}
} catch(Exception e) {
throw new InternalErrorException("Internal error while managing access token request", e);
switch (continuationTokenManager.getValidity(request.getToken())) {
case EXPIRED:
returnRestartAuthentication(resp);
break;
case INVALID:
LOG.warn("Use of an invalid ContinuationToken : " + request.getToken().serialize());
returnUnauthorizedResponse(resp);
break;
case OK:
manageAuthenticationResponse(request, resp);
break;
}
}

Expand Down Expand Up @@ -197,4 +199,7 @@ private void returnUnauthorizedResponse(HttpServletResponse resp) throws IOExcep
resp.sendError(HttpServletResponse.SC_UNAUTHORIZED);
}

private void returnRestartAuthentication(HttpServletResponse resp) throws IOException {
resp.sendError(HttpServletResponse.SC_FORBIDDEN);
}
}
Expand Up @@ -22,9 +22,16 @@
import org.apache.james.jmap.model.ContinuationToken;

public interface ContinuationTokenManager {
public static enum ContinuationTokenStatus {
OK,
INVALID,
EXPIRED
}

ContinuationToken generateToken(String username) throws Exception;
ContinuationToken generateToken(String username);

ContinuationTokenStatus getValidity(ContinuationToken token);

boolean isValid(ContinuationToken token) throws Exception;
boolean isValid(ContinuationToken token);

}
Expand Up @@ -20,22 +20,30 @@
package org.apache.james.jmap.crypto;

import java.io.InputStream;
import java.security.InvalidKeyException;
import java.security.Key;
import java.security.KeyStore;
import java.security.NoSuchAlgorithmException;
import java.security.PrivateKey;
import java.security.PublicKey;
import java.security.Signature;
import java.security.SignatureException;

import org.apache.commons.codec.binary.Base64;
import org.apache.commons.configuration.ConfigurationException;
import org.apache.commons.configuration.HierarchicalConfiguration;
import org.apache.james.filesystem.api.FileSystem;
import org.apache.james.lifecycle.api.Configurable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.base.Preconditions;
import com.google.common.base.Throwables;

public class JamesSignatureHandler implements SignatureHandler, Configurable {

private static final Logger LOGGER = LoggerFactory.getLogger(JamesSignatureHandler.class);

public static final String ALIAS = "james";
public static final String ALGORITHM = "SHA1withRSA";
public static final String JKS = "JKS";
Expand Down Expand Up @@ -68,21 +76,32 @@ public void init() throws Exception {
}

@Override
public String sign(String source) throws Exception {
public String sign(String source) {
Preconditions.checkNotNull(source);
Signature javaSignature = Signature.getInstance(ALGORITHM);
javaSignature.initSign(privateKey);
javaSignature.update(source.getBytes());
return new Base64().encodeAsString(javaSignature.sign());
try {
Signature javaSignature = Signature.getInstance(ALGORITHM);
javaSignature.initSign(privateKey);
javaSignature.update(source.getBytes());
return new Base64().encodeAsString(javaSignature.sign());
} catch (NoSuchAlgorithmException | InvalidKeyException | SignatureException e) {
throw Throwables.propagate(e);
}
}

@Override
public boolean verify(String source, String signature) throws Exception {
public boolean verify(String source, String signature) {
Preconditions.checkNotNull(source);
Preconditions.checkNotNull(signature);
Signature javaSignature = Signature.getInstance(ALGORITHM);
javaSignature.initVerify(publicKey);
javaSignature.update(source.getBytes());
return javaSignature.verify(new Base64().decode(signature));
try {
Signature javaSignature = Signature.getInstance(ALGORITHM);
javaSignature.initVerify(publicKey);
javaSignature.update(source.getBytes());
return javaSignature.verify(new Base64().decode(signature));
} catch (NoSuchAlgorithmException | InvalidKeyException e) {
throw Throwables.propagate(e);
} catch (SignatureException e) {
LOGGER.warn("Attempt to use a malformed signature '"+ signature + "' for source '" + source + "'", e);
return false;
}
}
}
Expand Up @@ -21,8 +21,8 @@

public interface SignatureHandler {

String sign(String source) throws Exception;
String sign(String source);

boolean verify(String source, String signature) throws Exception;
boolean verify(String source, String signature);

}
Expand Up @@ -19,21 +19,17 @@

package org.apache.james.jmap.crypto;

import com.google.common.base.Preconditions;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;

import org.apache.james.jmap.api.ContinuationTokenManager;
import org.apache.james.jmap.model.ContinuationToken;
import org.apache.james.jmap.utils.ZonedDateTimeProvider;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.security.SignatureException;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import com.google.common.base.Preconditions;

public class SignedContinuationTokenManager implements ContinuationTokenManager {

private static final Logger LOGGER = LoggerFactory.getLogger(SignedContinuationTokenManager.class);

private final SignatureHandler signatureHandler;
private final ZonedDateTimeProvider zonedDateTimeProvider;

Expand All @@ -43,7 +39,7 @@ public SignedContinuationTokenManager(SignatureHandler signatureHandler, ZonedDa
}

@Override
public ContinuationToken generateToken(String username) throws Exception {
public ContinuationToken generateToken(String username) {
Preconditions.checkNotNull(username);
ZonedDateTime expirationTime = zonedDateTimeProvider.provide().plusMinutes(15);
return new ContinuationToken(username,
Expand All @@ -52,25 +48,28 @@ public ContinuationToken generateToken(String username) throws Exception {
}

@Override
public boolean isValid(ContinuationToken token) throws Exception {
public ContinuationTokenStatus getValidity(ContinuationToken token) {
Preconditions.checkNotNull(token);
try {
return ! isTokenOutdated(token)
&& isCorrectlySigned(token);
} catch (SignatureException e) {
LOGGER.warn("Attempt to use a malformed signature for user " + token.getUsername(), e);
return false;
if (! isCorrectlySigned(token)) {
return ContinuationTokenStatus.INVALID;
}
if (isExpired(token)) {
return ContinuationTokenStatus.EXPIRED;
}
return ContinuationTokenStatus.OK;
}

@Override
public boolean isValid(ContinuationToken token) {
Preconditions.checkNotNull(token);
return ContinuationTokenStatus.OK.equals(getValidity(token));
}

private boolean isCorrectlySigned(ContinuationToken token) throws Exception {
return signatureHandler.verify(token.getUsername()
+ ContinuationToken.SEPARATOR
+ DateTimeFormatter.ISO_OFFSET_DATE_TIME.format(token.getExpirationDate()),
token.getSignature());
private boolean isCorrectlySigned(ContinuationToken token) {
return signatureHandler.verify(token.getContent(), token.getSignature());
}

private boolean isTokenOutdated(ContinuationToken token) {
private boolean isExpired(ContinuationToken token) {
return token.getExpirationDate().isBefore(zonedDateTimeProvider.provide());
}
}
Expand Up @@ -112,12 +112,16 @@ public String getSignature() {
}

public String serialize() {
return username
+ SEPARATOR
+ DateTimeFormatter.ISO_OFFSET_DATE_TIME.format(expirationDate)
return getContent()
+ SEPARATOR
+ signature;
}

public String getContent() {
return username
+ SEPARATOR
+ DateTimeFormatter.ISO_OFFSET_DATE_TIME.format(expirationDate);
}

@Override
public boolean equals(Object other) {
Expand Down
Expand Up @@ -44,6 +44,7 @@
import org.apache.james.jmap.crypto.JamesSignatureHandlerProvider;
import org.apache.james.jmap.crypto.SignedContinuationTokenManager;
import org.apache.james.jmap.memory.access.MemoryAccessTokenRepository;
import org.apache.james.jmap.model.ContinuationToken;
import org.apache.james.jmap.utils.ZonedDateTimeProvider;
import org.apache.james.user.api.UsersRepository;
import org.apache.james.user.api.UsersRepositoryException;
Expand Down Expand Up @@ -244,6 +245,25 @@ public void mustReturnAuthenticationFailedWhenBadPassword() throws Exception {

@Test
public void mustReturnAuthenticationFailedWhenContinuationTokenIsRejectedByTheContinuationTokenManager() throws Exception {
ContinuationToken badContinuationToken = new ContinuationToken("user@domain.tld", newDate, "badSignature");

when(mockedUsersRepository.test("user@domain.tld", "password"))
.thenReturn(true);
when(mockedZonedDateTimeProvider.provide())
.thenReturn(oldDate);

given()
.contentType(ContentType.JSON)
.accept(ContentType.JSON)
.body("{\"token\": \"" + badContinuationToken.serialize() + "\", \"method\": \"password\", \"password\": \"password\"}")
.when()
.post("/authentication")
.then()
.statusCode(401);
}

@Test
public void mustReturnRestartAuthenticationWhenContinuationTokenIsExpired() throws Exception {
when(mockedZonedDateTimeProvider.provide())
.thenReturn(oldDate);

Expand All @@ -261,7 +281,7 @@ public void mustReturnAuthenticationFailedWhenContinuationTokenIsRejectedByTheCo
.when()
.post("/authentication")
.then()
.statusCode(401);
.statusCode(403);
}

@Test
Expand Down
Expand Up @@ -24,8 +24,6 @@
import org.junit.Before;
import org.junit.Test;

import java.security.SignatureException;

public class JamesSignatureHandlerTest {

public static final String SIGNATURE = "NeIFNei4p6vn085wCEw0pbEwJ+Oak5yEIRLZsDcRVzT9rWWOcLvDFUA3S6awi/bxPiFxqJFreVz6xqzehnUI4tUBupk3sIsqeXShhFWBpaV+m58mC41lT/A0RJa3GgCvg6kmweCRf3tOo0+gvwOQJdwCL2B21GjDCKqBHaiK+OHcsSjrQW0xuew5z84EAz3ErdH4MMNjITksxK5FG/cGQ9V6LQgwcPk0RrprVC4eY7FFHw/sQNlJpZKsSFLnn5igPQkQtjiQ4ay1/xoB7FU7aJLakxRhYOnTKgper/Ur7UWOZJaE+4EjcLwCFLF9GaCILwp9W+mf/f7j92PVEU50Vg==";
Expand All @@ -49,9 +47,9 @@ public void invalidSignatureShouldNotBeRecognised() throws Exception {
assertThat(signatureHandler.verify(SOURCE, signatureHandler.sign(FAKE_SIGNATURE))).isFalse();
}

@Test(expected = SignatureException.class)
public void incorrectLengthSignatureShouldThrow() throws Exception {
signatureHandler.verify(SOURCE, "signature");
@Test
public void incorrectLengthSignatureShouldReturnFalse() throws Exception {
assertThat(signatureHandler.verify(SOURCE, "signature")).isFalse();
}

@Test(expected = NullPointerException.class)
Expand Down

0 comments on commit 9350d8d

Please sign in to comment.