Skip to content

Migrate account REST service tests to tests/base - #51678

Open
edewit wants to merge 3 commits into
keycloak:mainfrom
edewit:migrate-account-rest-service-cors-main
Open

Migrate account REST service tests to tests/base#51678
edewit wants to merge 3 commits into
keycloak:mainfrom
edewit:migrate-account-rest-service-cors-main

Conversation

@edewit

@edewit edewit commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • migrate AccountRestServiceCorsTest, AccountRestServiceReadOnlyAttributesTest, LinkedAccountsRestServiceTest, ResourcesRestServiceTest, and SessionRestServiceTest into tests/base
  • update migrated classes to the JUnit 5 test framework and replace legacy test utilities with current ManagedRealm, OAuthClient, and SimpleHttp patterns
  • commit migration using tests/migration-util/commit-migration.sh

fixes: #50752

Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
@edewit
edewit requested a review from a team as a code owner August 12, 2026 12:30
Copilot AI balanced review requested due to automatic review settings August 12, 2026 12:30

Copilot AI 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.

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, and SimpleHttp.
  • 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 testUserLookupReturnsMinimalData will receive alice@test.com instead of its expected alice@localhost. Register both changes through ManagedRealm cleanup 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.

Comment thread tests/base/src/test/java/org/keycloak/tests/account/ResourcesRestServiceTest.java Outdated
Comment on lines +917 to +920
int status = simpleHttp.doPost(getProtectionPermissionUrl())
.auth(getToken())
.json(ticket)
.asStatus();
Comment thread tests/base/src/test/java/org/keycloak/tests/account/ResourcesRestServiceTest.java Outdated
Comment on lines +64 to +65
@InjectOAuthClient
OAuthClient oauth;
Comment on lines +41 to +42
@KeycloakIntegrationTest
public class AccountRestServiceCorsTest {
Comment on lines +58 to +59
@KeycloakIntegrationTest
public class AccountRestServiceReadOnlyAttributesTest extends AbstractRestServiceTest {
Comment on lines +67 to +68
@KeycloakIntegrationTest
public class LinkedAccountsRestServiceTest {
Comment on lines +86 to +87
@KeycloakIntegrationTest
public class ResourcesRestServiceTest extends AbstractRestServiceTest {
Comment on lines +55 to +56
@KeycloakIntegrationTest
public class SessionRestServiceTest extends AbstractRestServiceTest {
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Copilot AI review requested due to automatic review settings August 12, 2026 12:51
@edewit
edewit requested review from a team as code owners August 12, 2026 12:51

Copilot AI 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.

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

  • LegacyRealmConfig starts from an empty realm, so this configuration creates neither test-user@localhost nor the direct-grant client used by getToken(); 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/base listens 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 on direct-grant.
    tests/base/src/test/java/org/keycloak/tests/account/LinkedAccountsRestServiceTest.java:104
  • After switching to managedRealm.getBaseUrl(), generated broker links use /realms/test/..., but testBuildLinkedAccountUri still 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. In testLogout, the calls at lines 301 and 304 create two test-user sessions, so the final assertEquals(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 TokenUtil login 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@localhost and direct-grant are absent and addFederatedIdentities silently finds no user. Build on AccountRestRealmConfig before 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>
Copilot AI review requested due to automatic review settings August 12, 2026 13:22

Copilot AI 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.

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

  • LegacyRealmConfig starts from an empty RealmBuilder, so this realm contains neither test-user@localhost nor the direct-grant client used by getToken(). All tests will fail to obtain a token; build this configuration on AccountRestRealmConfig (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@localhost through direct-grant; it also means addFederatedIdentities finds no test user and adds no links. Base this realm on AccountRestRealmConfig (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 /auth context path, so the generated broker URI cannot satisfy this assertion. Match the path produced from managedRealm.getBaseUrl() instead.
    tests/base/src/test/java/org/keycloak/tests/account/ResourcesRestServiceTest.java:923
  • The protection ticket endpoint returns 200 OK (PermissionTicketService#create uses Response.ok(...)), not 201 or 204. Consequently the first ticket created in before() fails this assertion; assert Response.Status.OK here.
    tests/base/src/test/java/org/keycloak/tests/account/ResourcesRestServiceTest.java:747
  • before() already creates a granted ticket for the first resource, requester alice, and Scope A; the protection endpoint rejects this duplicate with 400 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 another test-user session. Calling it once for DELETE and again for GET leaves two sessions, so the final assertEquals(1, ...) fails; obtain one token and reuse it for both requests.

@michalvavrik michalvavrik 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.

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?

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.

Migrate account package to the new test framework

4 participants