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

Merged
ahus1 merged 5 commits into
keycloak:mainfrom
wistefan:fix-oid4vc-mappers
May 27, 2024
Merged

ahus1 merged 5 commits into
keycloak:mainfrom
wistefan:fix-oid4vc-mappers

Conversation

@wistefan
Copy link
Copy Markdown
Contributor

Evaluates the supportedTypes of the OID4VC mapper config.
Fixes #29805

wistefan added 4 commits May 23, 2024 09:11
Signed-off-by: Stefan Wiedemann <wistefan@googlemail.com>
Signed-off-by: Stefan Wiedemann <wistefan@googlemail.com>
Signed-off-by: Stefan Wiedemann <wistefan@googlemail.com>
Signed-off-by: Stefan Wiedemann <wistefan@googlemail.com>
@wistefan
wistefan force-pushed the fix-oid4vc-mappers branch from 4c87bb8 to c863197 Compare May 23, 2024 07:11
@wistefan
wistefan marked this pull request as ready for review May 23, 2024 09:27
@wistefan
wistefan requested review from a team as code owners May 23, 2024 09:27
@mposolda mposolda self-assigned this May 23, 2024
@mposolda
Copy link
Copy Markdown
Contributor

@francis-pouatcha @tnorimat @pskopek Anyone of you available to review this please?

@tnorimat
Copy link
Copy Markdown
Contributor

@mposolda Hello, I will review the PR.

@tnorimat tnorimat left a comment
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.

@wistefan Thank you. Could you check review comments I added?

@@ -335,6 +335,7 @@ public void testRequestCredentialUnsupportedCredential() throws Throwable {
@Test
public void testRequestCredential() {
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 ?

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

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.

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.

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

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.

Signed-off-by: Stefan Wiedemann <wistefan@googlemail.com>
@tnorimat tnorimat left a comment
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.

@wistefan Thank you very much.
@mposolda I approved the PR.

@@ -335,6 +335,7 @@ public void testRequestCredentialUnsupportedCredential() throws Throwable {
@Test
public void testRequestCredential() {
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.

@ahus1 ahus1 left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving based on @tnorimat's review.

@ahus1
ahus1 merged commit 5a68056 into keycloak:main May 27, 2024
@wistefan
wistefan deleted the fix-oid4vc-mappers branch June 4, 2024 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Supported Credential Type is not evaluated when applying the Protocol Mapper in OID4VCI

4 participants

0