Skip to content

Add credential details to AdminEvent in UserResource.removeCredential (#48338)#48339

Open
thomasdarimont wants to merge 1 commit intokeycloak:mainfrom
thomasdarimont:issue/gh-48338-add-credential-details-to-remove-credential-admin-event
Open

Add credential details to AdminEvent in UserResource.removeCredential (#48338)#48339
thomasdarimont wants to merge 1 commit intokeycloak:mainfrom
thomasdarimont:issue/gh-48338-add-credential-details-to-remove-credential-admin-event

Conversation

@thomasdarimont
Copy link
Copy Markdown
Contributor

@thomasdarimont thomasdarimont commented Apr 21, 2026

  • Add credentialId, credentialType, credentialUserLabel to the AdminEvent details
  • Add testDeleteOtpCredentialEmitsAdminEventWithCredentialDetails test

Fixes #48338

@thomasdarimont thomasdarimont marked this pull request as ready for review April 21, 2026 18:51
@thomasdarimont thomasdarimont requested a review from a team as a code owner April 21, 2026 18:51
@thomasdarimont thomasdarimont changed the title Add details to AdminEvent in UserResource.removeCredential (#48338) Add credential details to AdminEvent in UserResource.removeCredential (#48338) Apr 21, 2026
@keycloak-github-bot keycloak-github-bot Bot mentioned this pull request Apr 21, 2026
Open
ahus1
ahus1 requested changes Apr 22, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@ahus1 ahus1 left a comment

Choose a reason for hiding this comment

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

Thank you for the PR.

I think the changes to the code are good, still the test failures here are related to your changes.

In your test, you're deleting a credential from a shared user that other tests depend on. Please update the test to create a user on-the-fly within your test.

@thomasdarimont thomasdarimont force-pushed the issue/gh-48338-add-credential-details-to-remove-credential-admin-event branch from 29cfa31 to bfe20d0 Compare April 22, 2026 08:13
@thomasdarimont
Copy link
Copy Markdown
Contributor Author

thomasdarimont commented Apr 22, 2026

Thanks @ahus1 just rebased and updated the test to use a dedicated test user.

@ahus1 ahus1 self-assigned this Apr 22, 2026
@thomasdarimont thomasdarimont force-pushed the issue/gh-48338-add-credential-details-to-remove-credential-admin-event branch from bfe20d0 to 8d861ea Compare April 23, 2026 11:25
@ahus1
Copy link
Copy Markdown
Member

ahus1 commented Apr 23, 2026

@thomasdarimont - unforatunately there are now compilation failures due to upstream changes. Please rebase your PR, and check that it compiles.

@thomasdarimont
Add details to AdminEvent in UserResource.removeCredential (keycloak#…
02f5a8f 
…48338)

- Add credentialId, credentialType, credentialUserLabel to the AdminEvent details
- Add testDeleteOtpCredentialEmitsAdminEventWithCredentialDetails test

Fixes keycloak#48338

Signed-off-by: Thomas Darimont <[email protected]>
@thomasdarimont thomasdarimont force-pushed the issue/gh-48338-add-credential-details-to-remove-credential-admin-event branch from 8d861ea to 02f5a8f Compare April 23, 2026 16:12
@thomasdarimont
Copy link
Copy Markdown
Contributor Author

thomasdarimont commented Apr 23, 2026

I rebased on current main and fixed the failing test.

keycloak-github-bot[bot]
keycloak-github-bot Bot reviewed Apr 23, 2026
View reviewed changes
Copy link
Copy Markdown

@keycloak-github-bot keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@keycloak-github-bot
Copy link
Copy Markdown

keycloak-github-bot Bot commented Apr 23, 2026

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.authz.PolicyEvaluationTest#

Keycloak CI - Base IT (3)

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

Report flaky test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ahus1 ahus1 ahus1 requested changes

+1 more reviewer

@keycloak-github-bot keycloak-github-bot[bot] keycloak-github-bot[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees

@ahus1 ahus1

Labels

flaky-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add details to AdminEvent in UserResource.removeCredential

2 participants

@thomasdarimont @ahus1