Configure authenticationFlowBindingOverrides for a client

[ybonnefond]

Describe the bug
It seems not possible to configure authenticationFlowBindingOverrides for a client.
Keycloak uses authentication flow ID in their client configuration instead of alias.

To Reproduce
I tried to force the authentication flow id and use this id in the client authenticationFlowBindingOverrides configuration but it does not seems to work at least if the auth flow already exists.

{
  "enabled": true,
  "realm": "realmWithFlow",
  "authenticationFlows": [
    {
      "alias": "my auth flow",
      "id": "ad7d518c-4129-483a-8351-e1223cb8eead",
      "description": "My auth flow for testing",
      "providerId": "basic-flow",
      "topLevel": true,
      "builtIn": false,
      "authenticationExecutions": [
        {
          "authenticator": "docker-http-basic-authenticator",
          "requirement": "DISABLED",
          "priority": 0,
          "userSetupAllowed": true,
          "autheticatorFlow": false
        }
      ]
    }
  ],
  "clients": [
    {
      "clientId": "moped-client",
      "authenticationFlowBindingOverrides": {
        "browser": "ad7d518c-4129-483a-8351-e1223cb8eead"
      },
      "name": "moped-client",
      "description": "Moped-Client",
      "enabled": true,
      "clientAuthenticatorType": "client-secret",
      "secret": "changed-special-client-secret",
      "redirectUris": [
        "https://moped-client.org/redirect"
      ],
      "webOrigins": [
        "https://moped-client.org/webOrigin"
      ]
    }
  ]
}

Expected behavior
The configuration should probably use authentication flow aliases and resolve them to ids before making the calls to Keycloak.

{
  "enabled": true,
  "realm": "realmWithFlow",
  "authenticationFlows": [
    {
      "alias": "my auth flow",
      "description": "My auth flow for testing",
      "providerId": "basic-flow",
      "topLevel": true,
      "builtIn": false,
      "authenticationExecutions": [
        {
          "authenticator": "docker-http-basic-authenticator",
          "requirement": "DISABLED",
          "priority": 0,
          "userSetupAllowed": true,
          "autheticatorFlow": false
        }
      ]
    }
  ],
  "clients": [
    {
      "clientId": "moped-client",
      "authenticationFlowBindingOverrides": {
        "browser": "my auth flow"
      },
      "name": "moped-client",
      "description": "Moped-Client",
      "enabled": true,
      "clientAuthenticatorType": "client-secret",
      "secret": "changed-special-client-secret",
      "redirectUris": [
        "https://moped-client.org/redirect"
      ],
      "webOrigins": [
        "https://moped-client.org/webOrigin"
      ]
    }
  ]
}

Environment (please complete the following information)

• Keycloak Version: 11.0.2
• keycloak-config-cli Version: v2.1.0-11.0.0
• Java Version: 11

[ybonnefond]

I had a look into the code and maybe a similar process to the client's authorizationSettings would fix this.

Exclude authenticationFlowBindingOverrides from client initial creation/update and have a specific import for the auth flow binding overrides which will first resolve flow aliases to ids and then updated the clients.

What do you think?

I am happy to give this a shot and make a PR to fix this if that approach looks ok to you.

[jkroepke]

Hi! Thanks for the report. Funny that there will be always someone that using a keycloak feature that I didn't know before.

First, I check if there is a undocument way that Keycloak supports.

But looking here, only a flow id is supported. https://github.com/keycloak/keycloak/blob/f486e97c182425bf7a2178f2d93bc4b308ca3bd6/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java#L1357-L1373

Exclude authenticationFlowBindingOverrides from client initial creation/update and have a specific import for the auth flow binding overrides which will first resolve flow aliases to ids and then updated the clients.

That might be the way to go, I would do this, too. Including some tests and documentation somewhere here.

I am happy to give this a shot and make a PR to fix this if that approach looks ok to you.

Sure. Some tips from my side:

Currently clients are configured before flows, because some flows settings depends agist clients. As you mention authenticationFlowBindingOverrides needs to be excluded from client config and needs to be configure separate. You could find the same behaivor on authorizationSettings.

Some additional scenarios should cover by tests. Like what happens if the flow referenced inside client gets deleted?

Since keycloak-config-cli re-creates flow on changes (+ the flow gets a new id) it needs to be verify what happens, e.g will the new flow id updated inside the client? What happens if the alias or the flow id would not found?

[jkroepke]

Was the close a miss click or du you not interested in a PR anymore?

[ybonnefond]

ah sorry, a miss click probably :)

[ybonnefond]

Thanks for your helpful comment, we will look into it on our side. I will likely not be the one who do the PR though.
We've got better java coder than myself.

[jkroepke]

We've got better java coder than myself.

All right! Since i'm a system operator, java isn't on my skill list, too. ;-)

[jBouyoud]

Hi there,

I'll try to implement it ;-P.

I see following test cases :

Explanation :

• 13 : Add two new flows and set this one of these to an existing client
• 14 : Change an exiting authenticationFlowBindingOverrides (on client), by an already existing flow
• 15 : Same file (check if all thing are good, the imported Id must be updated)
• 16 : Import is rejected with an error when your config is bad (flow alias doesn't exist)
• 17 : Remove authenticationFlowBindingOverrides from client
• 18 : Create a new realm with an auth flow and a client with an authenticationFlowBindingOverrides on the created flow
• 19 : Add a new client with an authenticationFlowBindingOverrides set

Do you thing there is another thing to test ?

[jkroepke]

Looks great!

Just one more: While authenticationFlowBindingOverrides is set, change something on an auth flow that triggers a re-create. The id inside authenticationFlowBindingOverrides should be updated to the new flowId.

Additionally. the code coverage will show us some untested codepath. Within this metric we should able to see if we miss something.

[jkroepke]

If you need help somewhere, just drop a WIP PR here.