Skip to content

Commit

Permalink
Return 409 HTTP response for missing credential
Browse files Browse the repository at this point in the history
  • Loading branch information
@melegiul
melegiul committed Dec 5, 2023
1 parent 605fd06 commit 5ae5996
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 99 deletions.
151 changes: 79 additions & 72 deletions app-authenticator/src/main/java/netzbegruenung/keycloak/app/rest/ChallengeResource.java
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
package netzbegruenung.keycloak.app.rest;

import jakarta.persistence.criteria.CriteriaBuilder;
import jakarta.persistence.criteria.CriteriaQuery;
import jakarta.persistence.criteria.Root;
import netzbegruenung.keycloak.app.AuthenticationUtil;
import netzbegruenung.keycloak.app.credentials.AppCredentialModel;
import netzbegruenung.keycloak.app.dto.ChallengeConverter;
import netzbegruenung.keycloak.app.jpa.Challenge;
import org.jboss.logging.Logger;
import org.keycloak.common.util.Base64;
import org.keycloak.common.util.Time;
import org.keycloak.connections.jpa.JpaConnectionProvider;
import org.keycloak.credential.CredentialModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.UserModel;
import org.keycloak.models.jpa.entities.CredentialEntity;
import org.keycloak.models.jpa.entities.RealmEntity;

import jakarta.persistence.EntityManager;
Expand All @@ -19,21 +25,24 @@
import jakarta.ws.rs.core.*;

import java.util.*;
import java.util.stream.Collector;
import java.util.stream.Collectors;

public class ChallengeResource {

private final KeycloakSession session;

private final EntityManager em;

private final Logger logger = Logger.getLogger(ChallengeResource.class);

public final static String CHALLENGE_REJECTED = "challenge_rejected";

public final static String INTERNAL_ERROR = "internal_error";

public final static String NO_CREDENTIAL = "no_credential";

public ChallengeResource(KeycloakSession session) {
this.session = session;
this.em = session.getProvider(JpaConnectionProvider.class).getEntityManager();
}

@GET
Expand All @@ -49,9 +58,56 @@ public Response getChallenges(@HeaderParam("Signature") List<String> signatureHe

String deviceId = signatureMap.get("keyId");
EntityManager em = session.getProvider(JpaConnectionProvider.class).getEntityManager();
RealmEntity realm = em.getReference(RealmEntity.class, session.getContext().getRealm().getId());

CriteriaBuilder cb = em.getCriteriaBuilder();
CriteriaQuery<CredentialEntity> criteria = cb.createQuery(CredentialEntity.class);
Root<CredentialEntity> root = criteria.from(CredentialEntity.class);
criteria.where(
cb.equal(root.get("type"), AppCredentialModel.TYPE),
cb.like(root.get("credentialData"), String.format("%%\"deviceId\":\"%s\"%%", deviceId))
);

TypedQuery<CredentialEntity> criteriaQuery = em.createQuery(criteria);
CredentialEntity credentialEntity;

try {
credentialEntity = criteriaQuery.getSingleResult();
} catch (NoResultException e) {
return Response
.status(Response.Status.CONFLICT)
.entity(new Message(NO_CREDENTIAL, "App credential does not exist"))
.build();
} catch (NonUniqueResultException e) {
logger.error("Failed to get app credentials: duplicate credentials detected for device ID: " + deviceId, e);
return Response
.status(Response.Status.INTERNAL_SERVER_ERROR)
.entity(new Message(INTERNAL_ERROR, "Internal server error"))
.build();
}

AppCredentialModel appCredential = AppCredentialModel.createFromCredentialModel(toModel(credentialEntity));
UserModel user = session.users().getUserById(session.getContext().getRealm(), credentialEntity.getUser().getId());

Map<String, String> signatureStringMap = new LinkedHashMap<>();
signatureStringMap.put("created", signatureMap.get("created"));

boolean verified = AuthenticationUtil.verifyChallenge(
user,
appCredential.getAppCredentialData(),
AuthenticationUtil.getSignatureString(signatureStringMap),
signatureMap.get("signature")
);

if (!verified) {
return Response
.status(Response.Status.FORBIDDEN)
.entity(new Message(CHALLENGE_REJECTED, "Invalid signature"))
.build();
}


TypedQuery<Challenge> query = em.createNamedQuery("Challenge.findByRealmAndDeviceId", Challenge.class);
RealmEntity realm = em.getReference(RealmEntity.class, session.getContext().getRealm().getId());
query.setParameter("realm", realm);
query.setParameter("deviceId", deviceId);
Challenge challenge;
Expand All @@ -69,15 +125,13 @@ public Response getChallenges(@HeaderParam("Signature") List<String> signatureHe
logger.error("Failed to get app authenticator challenge: duplicate challenge detected for device ID: " + deviceId, e);
return Response
.status(Response.Status.INTERNAL_SERVER_ERROR)
.type(MediaType.APPLICATION_JSON_TYPE)
.entity(new Message(INTERNAL_ERROR, "Internal server error"))
.build();

} catch (Throwable e) {
logger.error("Failed to get app authenticator challenge for device ID: " + deviceId, e);
return Response
.status(Response.Status.INTERNAL_SERVER_ERROR)
.type(MediaType.APPLICATION_JSON_TYPE)
.entity(new Message(INTERNAL_ERROR, "Internal server error"))
.build();
}
Expand All @@ -92,75 +146,28 @@ public Response getChallenges(@HeaderParam("Signature") List<String> signatureHe
.build();
}

try {
UserModel user = session.users().getUserById(session.getContext().getRealm(), challenge.getUser().getId());
AppCredentialModel appCredential = user.credentialManager()
.getStoredCredentialsByTypeStream(AppCredentialModel.TYPE)
.map(AppCredentialModel::createFromCredentialModel)
.filter(c -> c.getAppCredentialData().getDeviceId().equals(deviceId))
.collect(toSingleton());

Map<String, String> signatureStringMap = new LinkedHashMap<>();
signatureStringMap.put("created", signatureMap.get("created"));

boolean verified = AuthenticationUtil.verifyChallenge(
user,
appCredential.getAppCredentialData(),
AuthenticationUtil.getSignatureString(signatureStringMap),
signatureMap.get("signature")
);

if (!verified) {
return Response
.status(Response.Status.FORBIDDEN)
.entity(new Message(CHALLENGE_REJECTED, "Invalid signature"))
.build();
}

return Response
.ok(Arrays.asList(ChallengeConverter.getChallengeDto(challenge, session)))
.build();
return Response
.ok(List.of(ChallengeConverter.getChallengeDto(challenge, session)))
.build();
}

} catch (IllegalStateException e) {
logger.error(
String.format(
"Failed to get app authenticator challenge: duplicate app credentials detected for device ID [%s] user [%s]",
deviceId,
challenge.getUser().getUsername()
),
e
);
return Response
.status(Response.Status.INTERNAL_SERVER_ERROR)
.type(MediaType.APPLICATION_JSON_TYPE)
.entity(new Message(INTERNAL_ERROR, "Internal server error"))
.build();
} catch (IndexOutOfBoundsException e) {
logger.error(
String.format(
"Failed to get app authenticator challenge: no app credentials found for device ID [%s] user [%s]",
deviceId,
challenge.getUser().getUsername()
),
e
);
return Response
.status(Response.Status.INTERNAL_SERVER_ERROR)
.type(MediaType.APPLICATION_JSON_TYPE)
.entity(new Message(INTERNAL_ERROR, "Internal server error"))
.build();
CredentialModel toModel(CredentialEntity entity) {
CredentialModel model = new CredentialModel();
model.setId(entity.getId());
model.setType(entity.getType());
model.setCreatedDate(entity.getCreatedDate());
model.setUserLabel(entity.getUserLabel());

// Backwards compatibility - users from previous version still have "salt" in the DB filled.
// We migrate it to new secretData format on-the-fly
if (entity.getSalt() != null) {
String newSecretData = entity.getSecretData().replace("__SALT__", Base64.encodeBytes(entity.getSalt()));
entity.setSecretData(newSecretData);
entity.setSalt(null);
}
}

private <T> Collector<T, ?, T> toSingleton() throws IllegalStateException, IndexOutOfBoundsException {
return Collectors.collectingAndThen(
Collectors.toList(),
list -> {
if (list.size() > 1) {
throw new IllegalStateException();
}
return list.get(0);
}
);
model.setSecretData(entity.getSecretData());
model.setCredentialData(entity.getCredentialData());
return model;
}
}
31 changes: 9 additions & 22 deletions app-authenticator/src/test/java/netzbegruenung/keycloak/app/rest/ChallengeResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@
@SpringBootTest(classes = {AuthorizationServerApp.class}, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@AutoConfigureTestEntityManager
@ActiveProfiles("test")
@Sql(scripts = {"/import-challenges.sql"},
config = @SqlConfig(transactionMode = ISOLATED))
@Sql(scripts = {"/import-test-data.sql"},
config = @SqlConfig(transactionMode = ISOLATED)
)
@Sql(
scripts = "/delete-challenges.sql",
scripts = "/delete-test-data.sql",
config = @SqlConfig(transactionMode = ISOLATED),
executionPhase = AFTER_TEST_METHOD
)
Expand All @@ -35,8 +36,6 @@ public class ChallengeResourceTest {

private final static String INVALID_SIGNATURE_HEADER_VALUE = "keyId:not_existing,created:%d,signature:base64encodedSignature";

private final static Long HALF_HOUR_MILLIS = 1800000L;

@Autowired
private WebTestClient webClient;

Expand All @@ -58,25 +57,13 @@ void testSignatureHeaderRejected() {
}

@Test
void testEmptyChallenges() {
webClient
.get().uri(CHALLENGE_URI)
.header(SIGNATURE_HEADER_NAME, String.format(INVALID_SIGNATURE_HEADER_VALUE, System.currentTimeMillis()))
.exchange()
.expectStatus().isOk()
.expectBody()
.json("[]");
}

@Test
void testChallengeExpired() {
Long expiredTimestamp = System.currentTimeMillis() - HALF_HOUR_MILLIS;
void testInvalidSignature() {
webClient
.get().uri(uriBuilder -> uriBuilder
.path(CHALLENGE_URI)
.build()
)
.header(SIGNATURE_HEADER_NAME, String.format(VALID_SIGNATURE_HEADER_VALUE, expiredTimestamp))
.header(SIGNATURE_HEADER_NAME, String.format(VALID_SIGNATURE_HEADER_VALUE, System.currentTimeMillis()))
.exchange()
.expectStatus().isForbidden()
.expectBody()
Expand All @@ -91,11 +78,11 @@ void testNoCredentialsFound() {
.path(CHALLENGE_URI)
.build()
)
.header(SIGNATURE_HEADER_NAME, String.format(VALID_SIGNATURE_HEADER_VALUE, System.currentTimeMillis()))
.header(SIGNATURE_HEADER_NAME, String.format(INVALID_SIGNATURE_HEADER_VALUE, System.currentTimeMillis()))
.exchange()
.expectStatus().is5xxServerError()
.expectStatus().is4xxClientError()
.expectBody()
.jsonPath("$['error']").isEqualTo(ChallengeResource.INTERNAL_ERROR)
.jsonPath("$['error']").isEqualTo(ChallengeResource.NO_CREDENTIAL)
.jsonPath("$['message']").hasJsonPath();
}

Expand Down
0 .../src/test/resources/delete-challenges.sql → ...r/src/test/resources/delete-test-data.sql
View file
File renamed without changes.
2 changes: 0 additions & 2 deletions app-authenticator/src/test/resources/import-challenges.sql
View file

This file was deleted.

29 changes: 29 additions & 0 deletions app-authenticator/src/test/resources/import-test-data.sql
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
INSERT INTO APP_AUTH_CHALLENGE
VALUES (
random_uuid(),
'baeldung',
'a5461470-33eb-4b2d-82d4-b0484e96ad7f',
'target_url',
'test_device_id',
'secret',
DATEDIFF('SECOND', DATE '1970-01-01', CURRENT_TIMESTAMP()) * 1000,
'ip_address',
'device',
'browser',
'os',
'os_version',
'12eebf0b-a3eb-49f8-9ecf-173cf8a00145'
);

INSERT INTO CREDENTIAL
VALUES (
random_uuid(),
null,
'APP_CREDENTIAL',
'a5461470-33eb-4b2d-82d4-b0484e96ad7f',
DATEDIFF('SECOND', DATE '1970-01-01', CURRENT_TIMESTAMP()) * 1000,
null,
null,
'{"publicKey":"MCowBQYDK2VwAyEAyGcPE5TNqGYHeGFVOx2skuo6imkIdoAUppmDp8ug7T0=","deviceId":"test_device_id","deviceOs":"device_os","keyAlgorithm":"Ed25519","signatureAlgorithm":"Ed25519"}',
20
);
2 changes: 0 additions & 2 deletions openapi.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,6 @@ paths:
description: Signature header is missing or incomplete
'403':
description: Signature is invalid, challenge expired
'404':
description: Challenge for requested device ID not found
'409':
description: Device ID is not registered as second factor
'500':
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<maven.compiler.version>3.11.0</maven.compiler.version>
<maven.shade.version>3.5.1</maven.shade.version>
<maven.surefire.version>3.2.1</maven.surefire.version>
<spring.boot.version>3.1.5</spring.boot.version>
<spring.boot.version>3.2.0</spring.boot.version>
<!-- these should be updated together with Keycloak -->
<!-- check keycloak-dependencies-server-all effective pom -->
<resteasy.version>6.2.4.Final</resteasy.version>
Expand Down

0 comments on commit 5ae5996

Please sign in to comment.