Skip to content
This repository has been archived by the owner on Jul 16, 2019. It is now read-only.

Commit

Permalink
RAS-36 Add peer code review changes.
Browse files Browse the repository at this point in the history
  • Loading branch information
christian.rijke committed Sep 25, 2015
1 parent a4f4224 commit b57dc34
Show file tree
Hide file tree
Showing 48 changed files with 138 additions and 164 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,11 @@
import org.echocat.marquardt.authority.persistence.SessionStore;
import org.echocat.marquardt.authority.persistence.UserStore;
import org.echocat.marquardt.common.Signer;
import org.echocat.marquardt.common.domain.Certificate;
import org.echocat.marquardt.common.domain.certificate.Certificate;
import org.echocat.marquardt.common.domain.Credentials;
import org.echocat.marquardt.common.domain.JsonWrappedCertificate;
import org.echocat.marquardt.common.domain.KeyPairProvider;
import org.echocat.marquardt.common.keyprovisioning.KeyPairProvider;
import org.echocat.marquardt.common.domain.PublicKeyWithMechanism;
import org.echocat.marquardt.common.domain.Role;
import org.echocat.marquardt.common.domain.certificate.Role;
import org.echocat.marquardt.common.domain.Signable;
import org.echocat.marquardt.common.domain.Signature;
import org.echocat.marquardt.common.exceptions.AlreadyLoggedInException;
Expand Down Expand Up @@ -81,13 +80,13 @@ public Authority(final UserStore<USER, SIGNABLE> userStore, final SessionStore<S
* Implements signup. Creates a new User and a new Session.
*
* @param credentials Credentials of the user that should be signed up.
* @return Ready-to-send JSON wrapped certificate for the client.
* @return certificate for the client.
* @throws UserExistsException If a user with the same identifier already exists.
* @throws CertificateCreationException If there were problems creating the certificate.
*/
public JsonWrappedCertificate signUp(final Credentials credentials) {
if (!_userStore.findUserByCredentials(credentials).isPresent()) {
final USER user = _userStore.createUserFromCredentials(credentials);
public byte[] signUp(final Credentials credentials) {
if (!_userStore.findByCredentials(credentials).isPresent()) {
final USER user = _userStore.createFromCredentials(credentials);
return createCertificateAndSession(credentials, user);
} else {
throw new UserExistsException("User with identifier " + credentials.getIdentifier() + " already exists.");
Expand All @@ -97,17 +96,17 @@ public JsonWrappedCertificate signUp(final Credentials credentials) {
/**
* Implements signin. Creates a new Session for a known user.
* @param credentials Credentials of the user with the client's public key.
* @return Ready-to-send JSON wrapped certificate for the client.
* @return certificate for the client.
* @throws LoginFailedException If user does not exist or password does not match.
* @throws AlreadyLoggedInException If the user already has an active session for this client's public key.
* @throws CertificateCreationException If there were problems creating the certificate.
*/
public JsonWrappedCertificate signIn(final Credentials credentials) {
final USER user = _userStore.findUserByCredentials(credentials).orElseThrow(() -> new LoginFailedException("Login failed"));
public byte[] signIn(final Credentials credentials) {
final USER user = _userStore.findByCredentials(credentials).orElseThrow(() -> new LoginFailedException("Login failed"));
if (user.passwordMatches(credentials.getPassword())) {
// create new session
final PublicKeyWithMechanism publicKeyWithMechanism = new PublicKeyWithMechanism(credentials.getPublicKey());
if (_sessionStore.activeSessionExists(user.getUserId(), publicKeyWithMechanism.getValue(), _dateProvider.now())) {
if (_sessionStore.existsActiveSession(user.getUserId(), publicKeyWithMechanism.getValue(), _dateProvider.now())) {
throw new AlreadyLoggedInException("User with id " + user.getUserId() + " is already logged in for current client.");
} else {
return createCertificateAndSession(credentials, user);
Expand All @@ -122,22 +121,22 @@ public JsonWrappedCertificate signIn(final Credentials credentials) {
* @param certificate Certificate to replace (may be an expired one - but must be the last one created for this session).
* @param signedBytes byte sequence signed by the client.
* @param signature signature of the byte sequence.
* @return Ready-to-send JSON wrapped certificate for the client.
* @return certificate for the client.
* @throws NoSessionFoundException If no session exists for this certificate. You must sign in again to handle this.
* @throws ExpiredSessionException When the session is already expired. You must sign in again to handle this.
* @throws IllegalStateException When the user id of the session is not found. Caused by a data inconsistency or a wrong UserStore implementation.
* @throws CertificateCreationException If there were problems creating the certificate.
*/
public JsonWrappedCertificate refresh(final byte[] certificate, final byte[] signedBytes, final Signature signature) {
public byte[] refresh(final byte[] certificate, final byte[] signedBytes, final Signature signature) {
final SESSION session = getValidSessionBasedOnCertificate(decodeBase64(certificate));
verifySignature(signedBytes, signature, session);
final USER user = _userStore.findUserByUuid(session.getUserId()).orElseThrow(() -> new IllegalStateException("Could not find user with userId " + session.getUserId()));
final USER user = _userStore.findByUuid(session.getUserId()).orElseThrow(() -> new IllegalStateException("Could not find user with userId " + session.getUserId()));
try {
final byte[] newCertificate = createCertificate(user, clientPublicKeyFrom(session));
session.setCertificate(newCertificate);
session.setExpiresAt(nowPlus60Days());
_sessionStore.save(session);
return createCertificateResponse(newCertificate);
return newCertificate;
} catch (final IOException e) {
throw new CertificateCreationException("failed to refresh certificate for certificate " + user.getUserId(), e);
}
Expand Down Expand Up @@ -172,21 +171,17 @@ private void verifySignature(final byte[] signedBytes, final Signature signature
}
}

private JsonWrappedCertificate createCertificateAndSession(final Credentials credentials, final USER user) {
private byte[] createCertificateAndSession(final Credentials credentials, final USER user) {
final byte[] certificate;
try {
certificate = createCertificate(user, credentials.getPublicKey());
createSession(credentials.getPublicKey(), user.getUserId(), certificate);
return createCertificateResponse(certificate);
return certificate;
} catch (final IOException e) {
throw new CertificateCreationException("failed to create certificate for user with id " + user.getUserId(), e);
}
}

private JsonWrappedCertificate createCertificateResponse(final byte[] certificate) {
return new JsonWrappedCertificate(certificate);
}

private byte[] createCertificate(final USER user, final PublicKey clientPublicKey) throws IOException {
final SIGNABLE signable = _userStore.createSignableFromUser(user);
final Certificate<SIGNABLE> certificate = Certificate.create(_issuerKeyProvider.getPublicKey(), clientPublicKey, user.getRoles(), signable);
Expand All @@ -211,7 +206,7 @@ private SESSION getSessionBasedOnCertificate(final byte[] certificateBytes) {

private void createSession(final PublicKey publicKey, final UUID userId, final byte[] certificate) {
final PublicKeyWithMechanism publicKeyWithMechanism = new PublicKeyWithMechanism(publicKey);
final SESSION session = _sessionStore.create();
final SESSION session = _sessionStore.createTransient();
session.setUserId(userId);
session.setExpiresAt(nowPlus60Days());
session.setPublicKey(publicKeyWithMechanism.getValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

package org.echocat.marquardt.authority.domain;

import org.echocat.marquardt.common.domain.Role;
import org.echocat.marquardt.common.domain.certificate.Role;

import java.util.Set;
import java.util.UUID;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

package org.echocat.marquardt.authority.domain;

import org.echocat.marquardt.common.domain.Certificate;
import org.echocat.marquardt.common.domain.certificate.Certificate;
import org.echocat.marquardt.common.domain.Signable;

/**
Expand All @@ -24,8 +24,8 @@ public interface UserToSignableMapper<USER extends User, SIGNABLE extends Signab

/**
* Creates a custom signable from a (persisted) user in the authority.
* @param USER User to create signable from.
* @param user User to create signable from.
* @return Signable (for example to put into Certificate)
*/
SIGNABLE createSignableFromUser(USER USER);
SIGNABLE createSignableFromUser(USER user);
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public interface SessionStore<T extends Session> {
* @param dateToCheck Date to check - in most cases NOW.
* @return True if user has an active session on the current client, false if not.
*/
boolean activeSessionExists(UUID userId, byte[] clientPublicKey, Date dateToCheck);
boolean existsActiveSession(UUID userId, byte[] clientPublicKey, Date dateToCheck);

/**
* Implement to persist a new or updated session.
Expand All @@ -50,7 +50,7 @@ public interface SessionStore<T extends Session> {
* Implement this to create a new transient session.
* @return Created session.
*/
T create();
T createTransient();

/**
* Implement this to delete a session. You may also implement a invalidation here.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,17 @@
import org.echocat.marquardt.authority.domain.User;
import org.echocat.marquardt.authority.domain.UserToSignableMapper;
import org.echocat.marquardt.common.domain.Credentials;
import org.echocat.marquardt.common.domain.Role;
import org.echocat.marquardt.common.domain.certificate.Role;
import org.echocat.marquardt.common.domain.Signable;

import java.util.Optional;
import java.util.UUID;

public interface UserStore<USER extends User<? extends Role>, SIGNABLE extends Signable> extends UserToSignableMapper<USER, SIGNABLE> {

Optional<USER> findUserByCredentials(Credentials credentials);
Optional<USER> findByCredentials(Credentials credentials);

Optional<USER> findUserByUuid(UUID userId);
Optional<USER> findByUuid(UUID userId);

USER createUserFromCredentials(Credentials credentials);
USER createFromCredentials(Credentials credentials);
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,17 @@
import org.echocat.marquardt.authority.exceptions.ExpiredSessionException;
import org.echocat.marquardt.authority.persistence.SessionStore;
import org.echocat.marquardt.authority.persistence.UserStore;
import org.echocat.marquardt.common.CertificateValidator;
import org.echocat.marquardt.common.domain.Credentials;
import org.echocat.marquardt.common.domain.JsonWrappedCertificate;
import org.echocat.marquardt.common.domain.KeyPairProvider;
import org.echocat.marquardt.common.domain.Role;
import org.echocat.marquardt.common.domain.Signable;
import org.echocat.marquardt.common.domain.Signature;
import org.echocat.marquardt.common.domain.certificate.Role;
import org.echocat.marquardt.common.exceptions.AlreadyLoggedInException;
import org.echocat.marquardt.common.exceptions.InvalidCertificateException;
import org.echocat.marquardt.common.exceptions.LoginFailedException;
import org.echocat.marquardt.common.exceptions.NoSessionFoundException;
import org.echocat.marquardt.common.exceptions.UserExistsException;
import org.echocat.marquardt.common.keyprovisioning.KeyPairProvider;
import org.echocat.marquardt.common.web.JsonWrappedCertificate;
import org.echocat.marquardt.common.web.RequestValidator;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -63,7 +62,6 @@ public abstract class SpringAuthorityController<USER extends User<ROLE>,
private UserStore<USER, SIGNABLE> _userStore;
private Authority<USER, SESSION, SIGNABLE> _authority;
private final RequestValidator _requestValidator = new RequestValidator();
private CertificateValidator<SIGNABLE, ROLE> _certificateValidator;

/**
* Wire this with your stores.
Expand All @@ -72,33 +70,33 @@ public abstract class SpringAuthorityController<USER extends User<ROLE>,
* @param sessionStore Your session store implementation.
* @param issuerKeyProvider Your KeyPairProvider. The public key from this must be trusted by clients and services.
*/
public SpringAuthorityController(UserStore<USER, SIGNABLE> userStore, final SessionStore<SESSION> sessionStore, final KeyPairProvider issuerKeyProvider, CertificateValidator<SIGNABLE, ROLE> certificateValidator) {
public SpringAuthorityController(UserStore<USER, SIGNABLE> userStore, final SessionStore<SESSION> sessionStore, final KeyPairProvider issuerKeyProvider) {
_sessionStore = sessionStore;
_issuerKeyProvider = issuerKeyProvider;
_userStore = userStore;
_certificateValidator = certificateValidator;
//noinspection unchecked
_authority = new Authority<>(_userStore, _sessionStore, _issuerKeyProvider);
}

@RequestMapping(value = "/signup", method = RequestMethod.POST)
@ResponseStatus(value = HttpStatus.CREATED)
@ResponseBody
public JsonWrappedCertificate signUp(@RequestBody final CREDENTIALS credentials) {
return _authority.signUp(credentials);
return createCertificateResponse(_authority.signUp(credentials));
}

@RequestMapping(value = "/signin", method = RequestMethod.POST)
@ResponseBody
public JsonWrappedCertificate signIn(@RequestBody final CREDENTIALS credentials) {
return _authority.signIn(credentials);
return createCertificateResponse(_authority.signIn(credentials));
}

@RequestMapping(value = "/refresh", method = RequestMethod.POST)
@ResponseBody
public JsonWrappedCertificate refresh(@RequestHeader("X-Certificate") final byte[] certificate, HttpServletRequest request) {
byte[] signedBytesFromRequest = _requestValidator.extractSignedBytesFromRequest(request);
Signature signature = _requestValidator.extractSignatureFromHeader(request);
return _authority.refresh(certificate, signedBytesFromRequest, signature);
return createCertificateResponse(_authority.refresh(certificate, signedBytesFromRequest, signature));
}

@RequestMapping(value = "/signout", method = RequestMethod.POST)
Expand Down Expand Up @@ -151,4 +149,7 @@ public void handleIOException(final IOException ex) {
LOGGER.error("Unhandled IOException catched.", ex);
}

private JsonWrappedCertificate createCertificateResponse(final byte[] certificate) {
return new JsonWrappedCertificate(certificate);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import com.google.common.io.CharStreams;
import org.apache.commons.codec.binary.Base64;
import org.apache.commons.io.Charsets;
import org.echocat.marquardt.common.domain.JsonWrappedCertificate;
import org.echocat.marquardt.common.web.JsonWrappedCertificate;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public abstract class AuthorityTest {

@Before
public void setup() throws Exception {
when(getSessionStore().create()).thenReturn(createTestSession());
when(getSessionStore().createTransient()).thenReturn(createTestSession());
setValidSession(createTestSession());
}

Expand All @@ -59,27 +59,27 @@ protected static TestSession createTestSession() {
}

protected void givenUserExists() {
when(getUserStore().findUserByUuid(USER_ID)).thenReturn(Optional.of(TEST_USER));
when(getUserStore().findByUuid(USER_ID)).thenReturn(Optional.of(TEST_USER));
when(getUserStore().createSignableFromUser(any(TestUser.class))).thenReturn(TEST_USER_INFO);
when(getUserStore().findUserByCredentials(any(Credentials.class))).thenReturn(Optional.of(TEST_USER));
when(getUserStore().findByCredentials(any(Credentials.class))).thenReturn(Optional.of(TEST_USER));
}

protected void givenExistingSession() {
//noinspection UseOfObsoleteDateTimeApi
when(getSessionStore().activeSessionExists(any(UUID.class), any(byte[].class), any(Date.class))).thenReturn(true);
when(getSessionStore().existsActiveSession(any(UUID.class), any(byte[].class), any(Date.class))).thenReturn(true);
when(getSessionStore().findByCertificate(any(byte[].class))).thenReturn(Optional.of(getValidSession()));
}

protected void givenNoExistingSession() {
//noinspection UseOfObsoleteDateTimeApi
when(getSessionStore().activeSessionExists(eq(USER_ID), any(byte[].class), any(Date.class))).thenReturn(false);
when(getSessionStore().existsActiveSession(eq(USER_ID), any(byte[].class), any(Date.class))).thenReturn(false);
when(getSessionStore().findByCertificate(any(byte[].class))).thenReturn(Optional.empty());
}

protected void givenUserDoesNotExist() {
when(getUserStore().findUserByCredentials(any(Credentials.class))).thenReturn(Optional.<TestUser>empty());
when(getUserStore().findUserByUuid(any(UUID.class))).thenReturn(Optional.<TestUser>empty());
when(getUserStore().createUserFromCredentials(any(Credentials.class))).thenReturn(TEST_USER);
when(getUserStore().findByCredentials(any(Credentials.class))).thenReturn(Optional.<TestUser>empty());
when(getUserStore().findByUuid(any(UUID.class))).thenReturn(Optional.<TestUser>empty());
when(getUserStore().createFromCredentials(any(Credentials.class))).thenReturn(TEST_USER);
when(getUserStore().createSignableFromUser(any(TestUser.class))).thenReturn(TEST_USER_INFO);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,13 @@
import org.echocat.marquardt.authority.testdomain.TestUser;
import org.echocat.marquardt.authority.testdomain.TestUserInfo;
import org.echocat.marquardt.common.TestKeyPairProvider;
import org.echocat.marquardt.common.domain.JsonWrappedCertificate;
import org.echocat.marquardt.common.domain.KeyPairProvider;
import org.echocat.marquardt.common.domain.Signature;
import org.echocat.marquardt.common.exceptions.AlreadyLoggedInException;
import org.echocat.marquardt.common.exceptions.LoginFailedException;
import org.echocat.marquardt.common.exceptions.NoSessionFoundException;
import org.echocat.marquardt.common.exceptions.SignatureValidationFailedException;
import org.echocat.marquardt.common.exceptions.UserExistsException;
import org.echocat.marquardt.common.keyprovisioning.KeyPairProvider;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -46,7 +45,7 @@ public class AuthorityUnitTest extends AuthorityTest {
@InjectMocks
private Authority<TestUser, TestSession, TestUserInfo> _authority;

private JsonWrappedCertificate _certificate;
private byte[] _certificate;
@Mock
private Signature _signature;

Expand Down Expand Up @@ -216,7 +215,7 @@ private void whenSigningOut() {
}

private void thenUserIsStored() {
verify(getUserStore()).createUserFromCredentials(TEST_USER_CREDENTIALS);
verify(getUserStore()).createFromCredentials(TEST_USER_CREDENTIALS);
}

private void thenSessionIsCreated() {
Expand Down
Loading

0 comments on commit b57dc34

Please sign in to comment.