Add GET challenge expired check

netzbegruenung · Jan 30, 2024 · 73fbd3b · 73fbd3b
1 parent 49c5cf5
commit 73fbd3b
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 10 deletions.
diff --git a/app-authenticator/src/main/java/netzbegruenung/keycloak/app/AppAuthenticator.java b/app-authenticator/src/main/java/netzbegruenung/keycloak/app/AppAuthenticator.java
@@ -75,17 +75,18 @@ private void createAppChallenge(AuthenticationFlowContext context, CredentialMod
 
 		Map<String, String> authConfig = context.getAuthenticatorConfig() != null ? context.getAuthenticatorConfig().getConfig() : Collections.emptyMap();
 
-		Integer tokenExpiration = 60;
+		long tokenExpiration = 60;
 
 		try {
-			tokenExpiration = Integer.valueOf(authConfig.getOrDefault("appAuthActionTokenExpiration", "60"));
+			tokenExpiration = Long.parseLong(authConfig.getOrDefault("appAuthActionTokenExpiration", "60"));
 		} catch (NumberFormatException e) {
 			logger.warn("Invalid config for app auth action token expiration, falling back to default");
 		}
 
+		long expiresAt = Time.currentTime() + tokenExpiration;
 		AppAuthActionToken token = new AppAuthActionToken(
 			context.getUser().getId(),
-			Time.currentTime() + tokenExpiration,
+			(int) expiresAt,
 			AuthenticationSessionCompoundId.fromAuthSession(authSession).getEncodedId(),
 			authSession.getClient().getClientId()
 		);
@@ -108,7 +109,8 @@ private void createAppChallenge(AuthenticationFlowContext context, CredentialMod
 				builder.build(context.getRealm().getName()),
 				deviceRepresentation,
 				appCredentialData.getDeviceId(),
-				secret
+				secret,
+				expiresAt
 			);
 
 			authSession.setAuthNote("credentialId", appCredentialModel.getId());
@@ -144,7 +146,7 @@ private void createAppChallenge(AuthenticationFlowContext context, CredentialMod
 		}
 	}
 
-	private Challenge upsertAppChallengeEntity(AuthenticationFlowContext context, URI actionTokenUri, DeviceRepresentation deviceRepresentation, String deviceId, String encryptedSecret) throws NonUniqueResultException {
+	private Challenge upsertAppChallengeEntity(AuthenticationFlowContext context, URI actionTokenUri, DeviceRepresentation deviceRepresentation, String deviceId, String encryptedSecret, long expiresAt) throws NonUniqueResultException {
 		Challenge challenge;
 		EntityManager em = getEntityManager(context.getSession());
 		RealmEntity realm = em.getReference(RealmEntity.class, context.getRealm().getId());
@@ -182,6 +184,7 @@ private Challenge upsertAppChallengeEntity(AuthenticationFlowContext context, UR
 		challenge.setIpAddress(deviceRepresentation.getIpAddress());
 		challenge.setUpdatedTimestamp(Time.currentTimeMillis());
 		challenge.setClient(client);
+		challenge.setExpiresAt(expiresAt);
 
 		em.persist(challenge);
 		em.flush();

diff --git a/app-authenticator/src/main/java/netzbegruenung/keycloak/app/jpa/Challenge.java b/app-authenticator/src/main/java/netzbegruenung/keycloak/app/jpa/Challenge.java
@@ -68,6 +68,9 @@ public class Challenge {
 	@Column(name = "os_version", length = 63)
 	private String osVersion;
 
+	@Column(name = "expires_at", nullable = false)
+	private Long expiresAt;
+
 	public UUID getId() {
 		return id;
 	}
@@ -167,4 +170,12 @@ public ClientEntity getClient() {
 	public void setClient(ClientEntity client) {
 		this.client = client;
 	}
+
+	public Long getExpiresAt() {
+		return expiresAt;
+	}
+
+	public void setExpiresAt(Long expiresAt) {
+		this.expiresAt = expiresAt;
+	}
 }
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
@@ -136,10 +136,8 @@ public Response getChallenges(@HeaderParam("Signature") List<String> signatureHe
 				.build();
 		}
 
-		Long actionTokenLifespan = (long) session.getContext().getRealm().getActionTokenGeneratedByUserLifespan() * 1000L;
-
-		if (Time.currentTimeMillis() > challenge.getUpdatedTimestamp() + actionTokenLifespan
-				|| Time.currentTimeMillis() > Long.parseLong(signatureMap.get("created")) + actionTokenLifespan) {
+		if (Time.currentTime() > challenge.getExpiresAt()
+				|| Long.parseLong(signatureMap.get("created")) < challenge.getUpdatedTimestamp() - 1000) {
 			return Response
 				.status(Response.Status.FORBIDDEN)
 				.entity(new Message(CHALLENGE_REJECTED, "Challenge expired"))

diff --git a/app-authenticator/src/main/resources/META-INF/challenge-changelog.xml b/app-authenticator/src/main/resources/META-INF/challenge-changelog.xml
@@ -52,4 +52,17 @@
 								 referencedTableName="client" referencedColumnNames="ID" constraintName="FK_APP_AUTH_CHALLENGE_ON_CLIENT"/>
 	</changeSet>
 
+	<changeSet id="app-auth-challenge-23.0.4" author="giuliano.mele@verdigado.com">
+		<delete tableName="app_auth_challenge">
+		</delete>
+		<addColumn tableName="app_auth_challenge">
+			<column name="expires_at" type="BIGINT" defaultValueNumeric="0">
+				<constraints nullable="false"/>
+			</column>
+		</addColumn>
+		<createIndex tableName="app_auth_challenge" indexName="idx_challenge_device_id">
+			<column name="device_id"></column>
+		</createIndex>
+	</changeSet>
+
 </databaseChangeLog>
diff --git a/app-authenticator/src/test/resources/import-test-data.sql b/app-authenticator/src/test/resources/import-test-data.sql
@@ -12,7 +12,8 @@ VALUES (
         'browser',
         'os',
         'os_version',
-		'12eebf0b-a3eb-49f8-9ecf-173cf8a00145'
+		'12eebf0b-a3eb-49f8-9ecf-173cf8a00145',
+        0
 		);
 
 INSERT INTO CREDENTIAL