Migrate account REST service tests to tests/base - #51678
Conversation
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
There was a problem hiding this comment.
Pull request overview
Migrates five account REST integration tests to the JUnit 5 test framework under tests/base.
Changes:
- Replaces legacy utilities with
ManagedRealm,OAuthClient, andSimpleHttp. - Adds migrated account, resource, linked-account, CORS, and session tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
AccountRestServiceCorsTest.java |
Migrates CORS tests. |
AccountRestServiceReadOnlyAttributesTest.java |
Migrates read-only attribute tests. |
LinkedAccountsRestServiceTest.java |
Migrates linked-account tests. |
ResourcesRestServiceTest.java |
Migrates UMA resource tests. |
SessionRestServiceTest.java |
Migrates session and device tests. |
Suppressed comments (1)
tests/base/src/test/java/org/keycloak/tests/account/ResourcesRestServiceTest.java:439
- The attacker and Alice mutations persist in the class-scoped realm; in particular, a later
testUserLookupReturnsMinimalDatawill receivealice@test.cominstead of its expectedalice@localhost. Register both changes throughManagedRealmcleanup helpers.
// Create an attacker user whose username matches the email of a legitimate user.
UserRepresentation attacker = createUser("alice@test.com", "password", "Attacker", "X", "attacker@test.com");
managedRealm.admin().users().create(attacker);
alice.setEmail("alice@test.com");
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| int status = simpleHttp.doPost(getProtectionPermissionUrl()) | ||
| .auth(getToken()) | ||
| .json(ticket) | ||
| .asStatus(); |
| @InjectOAuthClient | ||
| OAuthClient oauth; |
| @KeycloakIntegrationTest | ||
| public class AccountRestServiceCorsTest { |
| @KeycloakIntegrationTest | ||
| public class AccountRestServiceReadOnlyAttributesTest extends AbstractRestServiceTest { |
| @KeycloakIntegrationTest | ||
| public class LinkedAccountsRestServiceTest { |
| @KeycloakIntegrationTest | ||
| public class ResourcesRestServiceTest extends AbstractRestServiceTest { |
| @KeycloakIntegrationTest | ||
| public class SessionRestServiceTest extends AbstractRestServiceTest { |
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (7)
tests/base/src/test/java/org/keycloak/tests/account/AccountRestServiceCorsTest.java:188
LegacyRealmConfigstarts from an empty realm, so this configuration creates neithertest-user@localhostnor thedirect-grantclient used bygetToken(); every CORS test will fail before issuing its XHR. Extend the account REST realm configuration and apply only the username override here.
tests/base/src/test/java/org/keycloak/tests/account/AccountRestServiceCorsTest.java:54- The new test framework serves Keycloak on port 8080 and its injected test HTTP server on 8500; nothing in
tests/baselistens on the legacy 8180 port. Browser navigation to these origins will therefore fail before the CORS request runs; use a reachable origin and whitelist that same origin ondirect-grant.
tests/base/src/test/java/org/keycloak/tests/account/LinkedAccountsRestServiceTest.java:104 - After switching to
managedRealm.getBaseUrl(), generated broker links use/realms/test/..., buttestBuildLinkedAccountUristill asserts the legacy/auth/realms/test/...path. Update that assertion to the test-framework URL layout or derive it from the managed realm URL.
tests/base/src/test/java/org/keycloak/tests/account/SessionRestServiceTest.java:438 - Unlike the migrated
TokenUtil, this helper performs a fresh password grant on every call. IntestLogout, the calls at lines 301 and 304 create two test-user sessions, so the finalassertEquals(1, sessions.size())fails; cache one access token per test instance.
tests/base/src/test/java/org/keycloak/tests/account/ResourcesRestServiceTest.java:894 - This uncached helper is invoked inside the 30-iteration setup loop, replacing one cached
TokenUtillogin with at least 30 password-grant requests and user sessions per test. Cache the access token for the test instance to avoid thousands of unnecessary logins across this class.
tests/base/src/test/java/org/keycloak/tests/account/ResourcesRestServiceTest.java:748 - The setup already creates granted tickets on every resource, and the migrated test removed the code that deleted tickets for this selected resource. If its existing requester is
alice, this lookup succeeds because of the granted permission rather than the new pending request, so restore ticket cleanup or select a resource/requester pair without an existing grant.
tests/base/src/test/java/org/keycloak/tests/account/LinkedAccountsRestServiceTest.java:363 - This legacy configuration starts with an empty realm, so
test-user@localhostanddirect-grantare absent andaddFederatedIdentitiessilently finds no user. Build onAccountRestRealmConfigbefore adding the identity providers.
Run Spotless on tests/base and commit the resulting import and formatting normalization for the migrated account REST tests. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (7)
tests/base/src/test/java/org/keycloak/tests/account/AccountRestServiceCorsTest.java:188
LegacyRealmConfigstarts from an emptyRealmBuilder, so this realm contains neithertest-user@localhostnor thedirect-grantclient used bygetToken(). All tests will fail to obtain a token; build this configuration onAccountRestRealmConfig(or explicitly add the required user/client) before disabling username edits.
tests/base/src/test/java/org/keycloak/tests/account/AccountRestServiceCorsTest.java:66- The migrated framework exposes Keycloak on port 8080 and its test HTTP server on port 8500; nothing in this test starts a server on the legacy port 8180. Navigating there will fail before the XHR runs, so use a reachable test origin and whitelist that same origin on the token client.
tests/base/src/test/java/org/keycloak/tests/account/LinkedAccountsRestServiceTest.java:363 - This configuration starts from an empty realm, but the tests authenticate
test-user@localhostthroughdirect-grant; it also meansaddFederatedIdentitiesfinds no test user and adds no links. Base this realm onAccountRestRealmConfig(without re-adding its existing users), then add the identity providers and federated identities.
tests/base/src/test/java/org/keycloak/tests/account/LinkedAccountsRestServiceTest.java:150 - The test framework serves realms directly under
/realms, without the legacy/authcontext path, so the generated broker URI cannot satisfy this assertion. Match the path produced frommanagedRealm.getBaseUrl()instead.
tests/base/src/test/java/org/keycloak/tests/account/ResourcesRestServiceTest.java:923 - The protection ticket endpoint returns
200 OK(PermissionTicketService#createusesResponse.ok(...)), not 201 or 204. Consequently the first ticket created inbefore()fails this assertion; assertResponse.Status.OKhere.
tests/base/src/test/java/org/keycloak/tests/account/ResourcesRestServiceTest.java:747 before()already creates a granted ticket for the first resource, requesteralice, andScope A; the protection endpoint rejects this duplicate with400 Permission already exists. The legacy test deleted that resource's existing tickets first, so equivalent cleanup must be restored before creating this ungranted request.
tests/base/src/test/java/org/keycloak/tests/account/SessionRestServiceTest.java:304- Each
getToken()performs a new password grant and creates anothertest-usersession. Calling it once for DELETE and again for GET leaves two sessions, so the finalassertEquals(1, ...)fails; obtain one token and reuse it for both requests.
michalvavrik
left a comment
There was a problem hiding this comment.
Please remove account,4 from testsuite/integration-arquillian/tests/base/testsuites/base-suite. Also remove testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AbstractRestServiceTest.java.
Also there are tests in org.keycloak.testsuite.account.custom which is not in this PR at all. However I did not find any dedicated issue mentioning custom in #44450. I think you need to migrate it, unless I missed something?
Summary
AccountRestServiceCorsTest,AccountRestServiceReadOnlyAttributesTest,LinkedAccountsRestServiceTest,ResourcesRestServiceTest, andSessionRestServiceTestintotests/baseManagedRealm,OAuthClient, andSimpleHttppatternstests/migration-util/commit-migration.shfixes: #50752