Keycloak 26.0.0 - Githubissues

ma1uta commented 1 month ago

What this PR does / why we need it:

Add support for Keycloak 26.0.0

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #1160

Special notes for your reviewer:

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

[x] the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

thomasdarimont commented 1 month ago

I just tested this branch and it worked for me.

However, I didn't test any KC26 specific configuration.

thomasdarimont commented 1 month ago

I also think we can drop the support for KC 18.x with the upgrade to KC26.

SmithJosh commented 1 month ago

Tested and this works for us as well. There was one bug we noticed though caused by this change in Keycloak 26 https://www.keycloak.org/docs/latest/release_notes/index.html#identity-providers-no-longer-available-from-the-realm-representation. The realm representation no longer contains identity provider mappers and so keycloak-config-cli tries to recreate them even if they already exist

thomasdarimont commented 1 month ago

Tested and this works for us as well. There was one bug we noticed though caused by this change in Keycloak 26 keycloak.org/docs/latest/release_notes/index.html#identity-providers-no-longer-available-from-the-realm-representation. The realm representation no longer contains identity provider mappers and so keycloak-config-cli tries to recreate them even if they already exist

I also stumbled upon this, according to https://www.keycloak.org/docs/26.0.2/upgrading/#identity-providers-no-longer-available-from-the-realm-representation we now need to query the endpoint /realms/{realm}/identity-provider/instances to handle this.

@SmithJosh I just gave this another try and it worked for me.

I did the following:

Created a fresh KC 26.0.2 env
I applied a realm config without IdPs
I added two IdPs to the realm config and applied the config -> IdPs were created correctly in the realm
Changed the display name of the IdP in realm config to trigger an update -> IdPs were updated correctly in the realm
I commented out one IdP in the realm config, applied the config -> IdP was removed
Manually added the IdP in the UI again
I removed the comment from the IdP in realm config, applied the config -> IdP was updated again

Which IdP update scenario did not work for you? I found a few places where org.keycloak.representations.idm.RealmRepresentation#getIdentityProviders() is used, e.g. de.adorsys.keycloak.config.factory.UsedAuthenticationFlowWorkaroundFactory.UsedAuthenticationFlowWorkaround#disableFirstBrokerLoginFlowsIfNeeded(..). I think those usages (except RealmImport) have to be adapted to the new API, e.g. change:

List<IdentityProviderRepresentation> identityProviders = existingRealm.getIdentityProviders();
...

To:

List<IdentityProviderRepresentation> identityProviders = identityProviderRepository.getAll(existingRealm.getRealm());

thomasdarimont commented 1 month ago

I updated my branch https://github.com/thomasdarimont/keycloak-config-cli/tree/update/keycloak-26.0.x with the fixed IdentityProviders lookup.

thomasdarimont commented 1 month ago

I spent some time on this today but need to do other stuff now. I needed to adjust several other things as well (IdentityProviderMappers handling), see the latest commits in my branch.

Almost there:

daviddavidgit commented 3 weeks ago

I had a look at the branch of @thomasdarimont. Several tests are failing because Keycloak added constraints in Keycloak 26 when deleting an authentication flow. One of the constraints is that when a client overrides the default browser flow with a "custom flow", one can not just delete the "custom flow" later. If we import a config file which defines the "custom flow", the strategy so far is to first delete the "custom flow" and readd it. But this leads to error in Keycloak. I think we have to find a solution for that.

ma1uta commented 3 weeks ago

Thanks @thomasdarimont for help. Also it seems after https://github.com/keycloak/keycloak/commit/c4005d29f07d10eeda580d804a48fdfc9441f074 client-common-sync from https://github.com/keycloak/keycloak-client/ need to be update. They added the new field in RealmRepresentation but not synced the keycloak-client repo.

ma1uta commented 3 weeks ago

Also keycloak 26.0.4 released.

thomasdarimont commented 3 weeks ago

@ma1uta keycloak-client 26.0.2 has been released that matches keycloak 26.0.4.

ma1uta commented 3 weeks ago

All pre-25 versions failed only with:

[ERROR] Failures: 
[ERROR]   ImportIdentityProvidersIT.shouldCreateOidcIdentityProvider:274 
Expected: is "example-client-secret"
     but: was "**********"
[ERROR]   ImportIdentityProvidersIT.shouldUpdateOidcIdentityProvider:323 
Expected: is "example-client-secret"
     but: was "**********"
[ERROR]   ImportIdentityProvidersIT.shouldUpdateOidcIdentityProviderWithDeleteAllMappers:628 
Expected: is "example-client-secret"
     but: was "**********"
[ERROR]   ImportIdentityProvidersIT.shouldUpdateOidcIdentityProviderWithMapper:372 
Expected: is "example-client-secret"
     but: was "**********"
[ERROR]   ImportIdentityProvidersIT.shouldUpdateOidcIdentityProviderWithReplacedMapper:564 
Expected: is "example-client-secret"
     but: was "**********"
[ERROR]   ImportIdentityProvidersIT.shouldUpdateOidcIdentityProviderWithUpdatedMapper:436 
Expected: is "example-client-secret"
     but: was "**********"
[ERROR]   ImportIdentityProvidersIT.shouldUpdateOidcIdentityProviderWithUpdatedMapperWithPseudoId:500 
Expected: is "example-client-secret"
     but: was "**********"

jwklijnsma commented 3 weeks ago

What is the eta for this do you need helping with testing / fix code ?

leoandrea7 commented 3 weeks ago

Keycloak 26.0.5 was released

jwklijnsma commented 3 weeks ago

issue is fix

ma1uta commented 3 weeks ago

Failed tests with HTTP 500 Internal Server Error because keycloak 26 throw exception Cannot remove authentication flow, it is currently in use and linked with https://github.com/keycloak/keycloak/pull/31875. Still investigating.

And failed test with NotFound HTTP 404 Not Found because in keycloak 26 existing clients.authenticationSettings is null.

bohmber commented 3 weeks ago

Why keycloak should allow to delete and recreate a flow that is currently in use in a client?

ma1uta commented 3 weeks ago

There are two ways:

don't allow delete flow which is used
allow delete flow and replace with a standard one

But in previous versions of keycloak these tests succeed, need to find out what was changed in keycloak.

bohmber commented 3 weeks ago

[org.keycloak.services.error.KeycloakErrorHandler] (executor-thread-2) Uncaught server error: org.keycloak.models.ModelException: Cannot remove authentication flow, it is currently in use at org.keycloak.models.jpa.RealmAdapter.removeAuthenticationFlow(RealmAdapter.java:1460) at org.keycloak.models.cache.infinispan.RealmAdapter.removeAuthenticationFlow(RealmAdapter.java:1349) at org.keycloak.models.utils.KeycloakModelUtils.deepDeleteAuthenticationFlow(KeycloakModelUtils.java:967) at org.keycloak.services.resources.admin.AuthenticationManagementResource.deleteFlow(AuthenticationManagementResource.java:370) at org.keycloak.services.resources.admin.AuthenticationManagementResource$quarkusrestinvoker$deleteFlow_c7a3b2245d7e636a2fcab9cafd6ed82fb5d1f875.invoke(Unknown Source) at org.jboss.resteasy.reactive.server.handlers.InvocationHandler.handle(InvocationHandler.java:29) at io.quarkus.resteasy.reactive.server.runtime.QuarkusResteasyReactiveRequestContext.invokeHandler(QuarkusResteasyReactiveRequestContext.java:141) at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:147) at io.quarkus.vertx.core.runtime.VertxCoreRecorder$14.runWith(VertxCoreRecorder.java:635) at org.jboss.threads.EnhancedQueueExecutor$Task.doRunWith(EnhancedQueueExecutor.java:2516) at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2495) at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1521) at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:11) at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:11) at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) at java.base/java.lang.Thread.run(Thread.java:1583)

bohmber commented 2 weeks ago

The specific code is here in AuthenticationFlowsImportService#recreateTopLevelFlow To update a flow the flow will be deleted and recreated. Before the deletion all references to this flow will be disabled/unasigned and afterwards enabled/asign again. This workaround is not done on clients. I think the check on client flows was not working < Keycloak 26. And I don't know why there is a difference in updating build-in flows and other flows. Build-in flows will be updated without this workaround.

   /**
     * Deletes the top-level flow and all its executions and recreates them.
     */
    private void recreateTopLevelFlow(
            RealmImport realmImport,
            AuthenticationFlowRepresentation topLevelFlowToImport,
            AuthenticationFlowRepresentation existingAuthenticationFlow
    ) {
        AuthenticationFlowRepresentation patchedAuthenticationFlow = CloneUtil.patch(
                existingAuthenticationFlow, topLevelFlowToImport, "id"
        );

        if (existingAuthenticationFlow.isBuiltIn()) {
            throw new InvalidImportException(String.format(
                    "Unable to recreate flow '%s' in realm '%s': Deletion or creation of built-in flows is not possible",
                    patchedAuthenticationFlow.getAlias(), realmImport.getRealm()
            ));
        }

        UsedAuthenticationFlowWorkaroundFactory.UsedAuthenticationFlowWorkaround workaround = workaroundFactory.buildFor(realmImport);
        workaround.disableTopLevelFlowIfNeeded(topLevelFlowToImport.getAlias());

        authenticatorConfigImportService.deleteAuthenticationConfigs(realmImport, patchedAuthenticationFlow);
        authenticationFlowRepository.delete(realmImport.getRealm(), patchedAuthenticationFlow.getId());
        authenticationFlowRepository.createTopLevel(realmImport.getRealm(), patchedAuthenticationFlow);

        AuthenticationFlowRepresentation createdTopLevelFlow = authenticationFlowRepository.getByAlias(
                realmImport.getRealm(), topLevelFlowToImport.getAlias()
        );
        executionFlowsImportService.createExecutionsAndExecutionFlows(realmImport, topLevelFlowToImport, createdTopLevelFlow);

        workaround.resetFlowIfNeeded();
    }

ma1uta commented 1 week ago

@bohmber I found the original issue: https://github.com/keycloak/keycloak/issues/30707. So, I replace all overrides with temporary flow before deletion and restore after creation.

bohmber commented 1 week ago

@ma1uta The last failing test is something similar https://github.com/adorsys/keycloak-config-cli/pull/1162/commits/8f2f5dc5d3b65c06c660a3fb7b18265df605e8a2 in the file 44_update_realm_remove_authz_policy_realm-management.json the realm-management client has authorizationServicesEnabled with an empty resources section in authorizationSettings. The authorizationSettings will be deleted during update of the client. Later the code tries to update the deleted authorizationSettings. That is the jakarta.ws.rs.NotFoundException: HTTP 404 Not Found.

ma1uta commented 1 week ago

Keycloak create AuthorizationSettings in two cases: