diff --git a/app-authenticator/src/main/java/netzbegruenung/keycloak/app/rest/ChallengeResource.java b/app-authenticator/src/main/java/netzbegruenung/keycloak/app/rest/ChallengeResource.java index 4ad1ce9..1becd25 100644 --- a/app-authenticator/src/main/java/netzbegruenung/keycloak/app/rest/ChallengeResource.java +++ b/app-authenticator/src/main/java/netzbegruenung/keycloak/app/rest/ChallengeResource.java @@ -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; @@ -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 @@ -49,9 +58,56 @@ public Response getChallenges(@HeaderParam("Signature") List 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 criteria = cb.createQuery(CredentialEntity.class); + Root 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 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 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 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; @@ -69,7 +125,6 @@ public Response getChallenges(@HeaderParam("Signature") List 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(); @@ -77,7 +132,6 @@ public Response getChallenges(@HeaderParam("Signature") List signatureHe 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(); } @@ -92,75 +146,28 @@ public Response getChallenges(@HeaderParam("Signature") List 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 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 Collector 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; } } diff --git a/app-authenticator/src/test/java/netzbegruenung/keycloak/app/rest/ChallengeResourceTest.java b/app-authenticator/src/test/java/netzbegruenung/keycloak/app/rest/ChallengeResourceTest.java index b2b0f79..aa4b52c 100644 --- a/app-authenticator/src/test/java/netzbegruenung/keycloak/app/rest/ChallengeResourceTest.java +++ b/app-authenticator/src/test/java/netzbegruenung/keycloak/app/rest/ChallengeResourceTest.java @@ -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 ) @@ -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; @@ -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() @@ -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(); } diff --git a/app-authenticator/src/test/resources/delete-challenges.sql b/app-authenticator/src/test/resources/delete-test-data.sql similarity index 100% rename from app-authenticator/src/test/resources/delete-challenges.sql rename to app-authenticator/src/test/resources/delete-test-data.sql diff --git a/app-authenticator/src/test/resources/import-challenges.sql b/app-authenticator/src/test/resources/import-challenges.sql deleted file mode 100644 index 81f73e1..0000000 --- a/app-authenticator/src/test/resources/import-challenges.sql +++ /dev/null @@ -1,2 +0,0 @@ -INSERT INTO APP_AUTH_CHALLENGE (id, realm_id, user_id, client_id, target_url, device_id, secret, updated_timestamp, ip_address, device, browser, os, os_version) -VALUES (random_uuid(), 'baeldung', 'a5461470-33eb-4b2d-82d4-b0484e96ad7f', '12eebf0b-a3eb-49f8-9ecf-173cf8a00145', 'target_url', 'test_device_id', 'secret', DATEDIFF('SECOND', DATE '1970-01-01', CURRENT_TIMESTAMP()) * 1000, 'ip_address', 'device', 'browser', 'os', 'os_version'); diff --git a/app-authenticator/src/test/resources/import-test-data.sql b/app-authenticator/src/test/resources/import-test-data.sql new file mode 100644 index 0000000..3e829a2 --- /dev/null +++ b/app-authenticator/src/test/resources/import-test-data.sql @@ -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 + ); diff --git a/openapi.yaml b/openapi.yaml index 0f5f33a..b217bc1 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -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': diff --git a/pom.xml b/pom.xml index 7482165..965617f 100644 --- a/pom.xml +++ b/pom.xml @@ -25,7 +25,7 @@ 3.11.0 3.5.1 3.2.1 - 3.1.5 + 3.2.0 6.2.4.Final