8000 Fix oid4vc mappers by wistefan · Pull Request #29806 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

Fix oid4vc mappers #29806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ahus1 merged 5 commits into from
May 27, 2024
Merged

Fix oid4vc mappers #29806

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d008c87
fix credential type check for protocol mappers
wistefan May 22, 2024
70b0acc
fix the test
wistefan May 22, 2024
f046bfd
fix the test
wistefan May 22, 2024
c863197
fix the test
wistefan May 23, 2024
bf62374
solve review remarks
wistefan May 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions services/src/main/java/org/keycloak/protocol/oid4vc/issuance/OID4VCIssuerEndpoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,11 +373,15 @@ private VerifiableCredential getVCToSign(List<OID4VCMapper> protocolMappers, Str

Map<String, Object> subjectClaims = new HashMap<>();
protocolMappers
.stream()
.filter(mapper -> mapper.isTypeSupported(vcType))
.forEach(mapper -> mapper.setClaimsForSubject(subjectClaims, userSessionModel));

subjectClaims.forEach((key, value) -> vc.getCredentialSubject().setClaims(key, value));

protocolMappers
.stream()
.filter(mapper -> mapper.isTypeSupported(vcType))
.forEach(mapper -> mapper.setClaimsForCredential(vc, userSessionModel));

LOGGER.debugf("The credential to sign is: %s", vc);
Expand Down
10 changes: 8 additions & 2 deletions ...rc/test/java/org/keycloak/testsuite/oid4vc/issuance/signing/OID4VCIssuerEndpointTest.java
View file
Open in desktop
@tnorimat tnorimat May 24, 2024
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

L79 import is not used.

@wistefan wistefan May 24, 2024
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in bf62374

@tnorimat tnorimat May 24, 2024
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed the fix. Thank you.

Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

Expand Down Expand Up @@ -335,6 +334,7 @@ public void testRequestCredentialUnsupportedCredential() throws Throwable {
@Test
public void testRequestCredential() {
@tnorimat tnorimat May 24, 2024
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about add these additional check in requestOffer ?

@wistefan wistefan May 24, 2024
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, thank you:) Fixed in bf62374

@tnorimat tnorimat May 24, 2024
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed the fix. Thank you.

String token = getBearerToken(oauth);
ObjectMapper objectMapper = new ObjectMapper();
testingClient
.server(TEST_REALM_NAME)
.run((session -> {
Expand All @@ -349,8 +349,12 @@ public void testRequestCredential() {
assertNotNull("A credential should be responded.", credentialResponse.getEntity());
CredentialResponse credentialResponseVO = OBJECT_MAPPER.convertValue(credentialResponse.getEntity(), CredentialResponse.class);
JsonWebToken jsonWebToken = TokenVerifier.create((String) credentialResponseVO.getCredential(), JsonWebToken.class).getToken();
// correct signing and contents are verified in the JwtSigningServiceTest, thus we only check that it is a JWT

assertNotNull("A valid credential string should have been responded", jsonWebToken);
assertNotNull("The credentials should be included at the vc-claim.", jsonWebToken.getOtherClaims().get("vc"));
VerifiableCredential credential = objectMapper.convertValue(jsonWebToken.getOtherClaims().get("vc"), VerifiableCredential.class);
assertTrue("The static claim should be set.", credential.getCredentialSubject().getClaims().containsKey("VerifiableCredential"));
assertFalse("Only mappers supported for the requested type should have been evaluated.", credential.getCredentialSubject().getClaims().containsKey("AnotherCredentialType"));
}));
}

Expand Down Expand Up @@ -482,6 +486,8 @@ private void requestOffer(String token, String credentialEndpoint, SupportedCred
assertEquals(List.of("VerifiableCredential"), credential.getType());
assertEquals(URI.create("did:web:test.org"), credential.getIssuer());
assertEquals("john@email.cz", credential.getCredentialSubject().getClaims().get("email"));
assertTrue("The static claim should be set.", credential.getCredentialSubject().getClaims().containsKey("VerifiableCredential"));
assertFalse("Only mappers supported for the requested type should have been evaluated.", credential.getCredentialSubject().getClaims().containsKey("AnotherCredentialType"));
}

@Override
Expand Down
13 changes: 7 additions & 6 deletions ...n/tests/base/src/test/java/org/keycloak/testsuite/oid4vc/issuance/signing/OID4VCTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,8 @@ public static ClientRepresentation getTestClient(String clientId) {
getRoleMapper(clientId),
getEmailMapper(),
getIdMapper(),
getStaticClaimMapper()
getStaticClaimMapper("VerifiableCredential"),
getStaticClaimMapper("AnotherCredentialType")
)
);
return clientRepresentation;
Expand Down Expand Up @@ -253,17 +254,17 @@ public static ProtocolMapperRepresentation getIdMapper() {
return protocolMapperRepresentation;
}

public static ProtocolMapperRepresentation getStaticClaimMapper() {
public static ProtocolMapperRepresentation getStaticClaimMapper(String supportedType) {
ProtocolMapperRepresentation protocolMapperRepresentation = new ProtocolMapperRepresentation();
protocolMapperRepresentation.setName("static-mapper");
protocolMapperRepresentation.setName(UUID.randomUUID().toString());
protocolMapperRepresentation.setProtocol("oid4vc");
protocolMapperRepresentation.setId(UUID.randomUUID().toString());
protocolMapperRepresentation.setProtocolMapper("oid4vc-static-claim-mapper");
protocolMapperRepresentation.setConfig(
Map.of(
"subjectProperty", "static",
"subjectValue", "Value",
"supportedCredentialTypes", "VerifiableCredential")
"subjectProperty", supportedType,
"staticValue", "true",
"supportedCredentialTypes", supportedType)
);
return protocolMapperRepresentation;
}
Expand Down
0