mirror of
https://github.com/keycloak/keycloak.git
synced 2026-02-03 20:39:33 -05:00
Refactor credential offer handling for improved replay protection
Signed-off-by: Awambeng Rodrick <awambengrodrick@gmail.com>
This commit is contained in:
parent
a4e926fac0
commit
b132acccd8
5 changed files with 29 additions and 59 deletions
|
|
@ -603,7 +603,7 @@ public class OID4VCIssuerEndpoint {
|
|||
CredentialOfferStorage offerStorage = session.getProvider(CredentialOfferStorage.class);
|
||||
CredentialOfferState offerState = offerStorage.findOfferStateByNonce(session, nonce);
|
||||
if (offerState == null) {
|
||||
var errorMessage = "No credential offer state for nonce: " + nonce;
|
||||
var errorMessage = "Credential offer not found or already consumed";
|
||||
eventBuilder.detail(Details.REASON, errorMessage).error(Errors.INVALID_REQUEST);
|
||||
throw new BadRequestException(getErrorResponse(INVALID_CREDENTIAL_OFFER_REQUEST, errorMessage));
|
||||
}
|
||||
|
|
@ -620,14 +620,17 @@ public class OID4VCIssuerEndpoint {
|
|||
throw new BadRequestException(getErrorResponse(INVALID_CREDENTIAL_OFFER_REQUEST, errorMessage));
|
||||
}
|
||||
|
||||
boolean isFirstConsumption = offerStorage.markAsConsumedIfNotConsumed(session, nonce);
|
||||
if (!isFirstConsumption) {
|
||||
var errorMessage = "Credential offer has already been consumed";
|
||||
LOGGER.debugf("Credential offer with nonce %s has already been consumed", nonce);
|
||||
// Remove the nonce entry atomically for replay protection
|
||||
// This prevents the same offer URL from being accessed multiple times
|
||||
// while keeping pre-authorized code and credential identifier entries available
|
||||
Map<String, String> removed = session.singleUseObjects().remove(nonce);
|
||||
if (removed == null) {
|
||||
var errorMessage = "Credential offer not found or already consumed";
|
||||
LOGGER.debugf("Credential offer with nonce %s not found or already consumed", nonce);
|
||||
eventBuilder.detail(Details.REASON, errorMessage).error(Errors.INVALID_REQUEST);
|
||||
throw new BadRequestException(getErrorResponse(INVALID_CREDENTIAL_OFFER_REQUEST, errorMessage));
|
||||
}
|
||||
LOGGER.debugf("Marked credential offer with nonce %s as consumed", nonce);
|
||||
LOGGER.debugf("Removed credential offer nonce %s for replay protection", nonce);
|
||||
|
||||
// Add event details
|
||||
if (offerState.getClientId() != null) {
|
||||
|
|
@ -794,6 +797,8 @@ public class OID4VCIssuerEndpoint {
|
|||
|
||||
CredentialScopeModel requestedCredential;
|
||||
OID4VCAuthorizationDetailResponse authDetails;
|
||||
CredentialOfferState offerState = null;
|
||||
CredentialOfferStorage offerStorage = null;
|
||||
|
||||
// When the CredentialRequest contains a credential identifier the caller must have gone through the
|
||||
// CredentialOffer process or otherwise have set up a valid CredentialOfferState
|
||||
|
|
@ -802,8 +807,8 @@ public class OID4VCIssuerEndpoint {
|
|||
|
||||
// First check if the credential identifier exists
|
||||
// This allows proper error reporting for unknown identifiers
|
||||
CredentialOfferStorage offerStorage = session.getProvider(CredentialOfferStorage.class);
|
||||
CredentialOfferState offerState = offerStorage.findOfferStateByCredentialId(session, credentialIdentifier);
|
||||
offerStorage = session.getProvider(CredentialOfferStorage.class);
|
||||
offerState = offerStorage.findOfferStateByCredentialId(session, credentialIdentifier);
|
||||
if (offerState == null) {
|
||||
var errorMessage = "No credential offer state for credential id: " + credentialIdentifier;
|
||||
eventBuilder.detail(Details.REASON, errorMessage).error(Errors.INVALID_REQUEST);
|
||||
|
|
@ -939,6 +944,13 @@ public class OID4VCIssuerEndpoint {
|
|||
.detail(Details.VERIFIABLE_CREDENTIALS_ISSUED, String.valueOf(responseVO.getCredentials().size()));
|
||||
eventBuilder.success();
|
||||
|
||||
// Clean up offer state after successful credential issuance
|
||||
// This prevents memory leaks while ensuring the state remains available during the request
|
||||
if (offerState != null) {
|
||||
offerStorage.removeOfferState(session, offerState);
|
||||
LOGGER.debugf("Removed credential offer state after successful issuance for credential identifier: %s", credentialIdentifier);
|
||||
}
|
||||
|
||||
return response;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -42,10 +42,6 @@ public interface CredentialOfferStorage extends Provider {
|
|||
private String nonce;
|
||||
private int expiration;
|
||||
private OID4VCAuthorizationDetailResponse authorizationDetails;
|
||||
/**
|
||||
* Flag indicating whether this credential offer has been consumed (accessed via the credential offer URL).
|
||||
*/
|
||||
private boolean consumed;
|
||||
|
||||
public CredentialOfferState(CredentialsOffer credOffer, String clientId, String userId, int expiration) {
|
||||
this.credentialsOffer = credOffer;
|
||||
|
|
@ -53,7 +49,6 @@ public interface CredentialOfferStorage extends Provider {
|
|||
this.userId = userId;
|
||||
this.expiration = expiration;
|
||||
this.nonce = Base64Url.encode(RandomSecret.createRandomSecret(64));
|
||||
this.consumed = false;
|
||||
}
|
||||
|
||||
// For json serialization
|
||||
|
|
@ -120,14 +115,6 @@ public interface CredentialOfferStorage extends Provider {
|
|||
void setExpiration(int expiration) {
|
||||
this.expiration = expiration;
|
||||
}
|
||||
|
||||
public boolean isConsumed() {
|
||||
return consumed;
|
||||
}
|
||||
|
||||
public void setConsumed(boolean consumed) {
|
||||
this.consumed = consumed;
|
||||
}
|
||||
}
|
||||
|
||||
void putOfferState(KeycloakSession session, CredentialOfferState entry);
|
||||
|
|
@ -140,19 +127,6 @@ public interface CredentialOfferStorage extends Provider {
|
|||
|
||||
void replaceOfferState(KeycloakSession session, CredentialOfferState entry);
|
||||
|
||||
/**
|
||||
* Atomically marks a credential offer as consumed if it is not already consumed.
|
||||
* This method provides thread-safe replay protection by ensuring only one thread
|
||||
* can successfully mark an offer as consumed.
|
||||
*
|
||||
* @param session the Keycloak session
|
||||
* @param nonce the nonce identifying the credential offer
|
||||
* @return true if the offer was successfully marked as consumed (was not consumed before),
|
||||
* false if the offer was already consumed (replay attempt) or does not exist.
|
||||
* The caller should verify the offer exists before calling this method.
|
||||
*/
|
||||
boolean markAsConsumedIfNotConsumed(KeycloakSession session, String nonce);
|
||||
|
||||
void removeOfferState(KeycloakSession session, CredentialOfferState entry);
|
||||
|
||||
@Override
|
||||
|
|
|
|||
|
|
@ -122,25 +122,6 @@ class DefaultCredentialOfferStorage implements CredentialOfferStorage {
|
|||
});
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean markAsConsumedIfNotConsumed(KeycloakSession session, String nonce) {
|
||||
CredentialOfferState currentState = findOfferStateByNonce(session, nonce);
|
||||
if (currentState == null || currentState.isConsumed()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
currentState.setConsumed(true);
|
||||
String consumedStateJson = JsonSerialization.valueAsString(currentState);
|
||||
boolean replaced = session.singleUseObjects().replace(nonce, Map.of(ENTRY_KEY, consumedStateJson));
|
||||
|
||||
if (replaced) {
|
||||
replaceOfferState(session, currentState);
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void removeOfferState(KeycloakSession session, CredentialOfferState entry) {
|
||||
session.singleUseObjects().remove(entry.getNonce());
|
||||
|
|
|
|||
|
|
@ -382,8 +382,11 @@ public class OID4VCCredentialOfferCorsTest extends OID4VCIssuerEndpointTest {
|
|||
.user((String) null)
|
||||
.session((String) null)
|
||||
// Storage prunes expired single-use entries before lookup; lookup failure yields INVALID_REQUEST
|
||||
// The error message indicates the offer was not found (pruned due to expiration) or already consumed
|
||||
.error(Errors.INVALID_REQUEST)
|
||||
.detail(Details.REASON, Matchers.containsString("No credential offer state"))
|
||||
.detail(Details.REASON, Matchers.anyOf(
|
||||
Matchers.containsString("not found"),
|
||||
Matchers.containsString("already consumed")))
|
||||
.assertEvent();
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1123,8 +1123,8 @@ public class OID4VCJWTIssuerEndpointTest extends OID4VCIssuerEndpointTest {
|
|||
assertEquals("Error type should be INVALID_CREDENTIAL_OFFER_REQUEST",
|
||||
ErrorType.INVALID_CREDENTIAL_OFFER_REQUEST.name(),
|
||||
response.getError());
|
||||
assertTrue("Error description should mention that offer has been consumed",
|
||||
response.getErrorDescription().contains("already been consumed"));
|
||||
assertTrue("Error description should mention that offer is not found or already consumed",
|
||||
response.getErrorDescription().contains("not found") || response.getErrorDescription().contains("already consumed"));
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -1206,7 +1206,7 @@ public class OID4VCJWTIssuerEndpointTest extends OID4VCIssuerEndpointTest {
|
|||
}
|
||||
|
||||
/**
|
||||
* Test that consuming the credential offer (setting consumed=true) does not invalidate
|
||||
* Test that removing the nonce entry (for replay protection) does not invalidate
|
||||
* the Pre-Authorized Code. This verifies that the replay protection mechanism doesn't
|
||||
* interfere with the normal token request flow using the pre-authorized code.
|
||||
*/
|
||||
|
|
@ -1229,7 +1229,7 @@ public class OID4VCJWTIssuerEndpointTest extends OID4VCIssuerEndpointTest {
|
|||
String nonce = credentialOfferURI.getNonce();
|
||||
assertNotNull("Nonce should not be null", nonce);
|
||||
|
||||
// 2. Fetch the Offer JSON (this triggers the consumed flag)
|
||||
// 2. Fetch the Offer JSON (this removes the nonce entry for replay protection)
|
||||
CredentialsOffer credentialsOffer = oauth.oid4vc()
|
||||
.credentialOfferRequest(nonce)
|
||||
.bearerToken(token)
|
||||
|
|
@ -1256,7 +1256,7 @@ public class OID4VCJWTIssuerEndpointTest extends OID4VCIssuerEndpointTest {
|
|||
AccessTokenResponse accessTokenResponse = oauth.oid4vc()
|
||||
.preAuthorizedCodeGrantRequest(preAuthorizedCode)
|
||||
.send();
|
||||
assertEquals("Token request should succeed even after offer is consumed",
|
||||
assertEquals("Token request should succeed even after nonce is removed for replay protection",
|
||||
HttpStatus.SC_OK,
|
||||
accessTokenResponse.getStatusCode());
|
||||
assertNotNull("Access token should be present", accessTokenResponse.getAccessToken());
|
||||
|
|
|
|||
Loading…
Reference in a new issue