Uploaded image for project: 'Keycloak'
  1. Keycloak
  2. KEYCLOAK-12319

[REL] Default first broker login flow is broken after migration

    XMLWordPrintable

    Details

    • Sprint:
      Keycloak Sprint 34
    • Workaround:
      Workaround Exists
    • Workaround Description:
      Hide

      Manually re-configure the "First Broker Login" flow. Basically the REQUIRED and ALTERNATIVE items can't be at same level now, so there is a need to create the additional REQUIRED subflow and move the ALTERNATIVE items into it.

      So for example, instead of the configuration like:

      - Review Profile - REQUIRED
      - Create Userif Unique - ALTERNATIVE
      - Handle existing account - ALTERNATIVE
         ...
      

      There is a need to have this:

      - Review Profile REQUIRED
      - Create user or autolink subflow - REQUIRED
            - Create User if Unique ALTERNATIVE
            - Handle existing account ALTERNATIVE
            ...
      

      For the inspiration, you can create new Realm in Keycloak 8.0 and take a look how the default FirstBrokerLogin flow looks in that new realm. So you need to change it to the same/similar configuration in existing realms.

      Show
      Manually re-configure the "First Broker Login" flow. Basically the REQUIRED and ALTERNATIVE items can't be at same level now, so there is a need to create the additional REQUIRED subflow and move the ALTERNATIVE items into it. So for example, instead of the configuration like: - Review Profile - REQUIRED - Create Userif Unique - ALTERNATIVE - Handle existing account - ALTERNATIVE ... There is a need to have this: - Review Profile REQUIRED - Create user or autolink subflow - REQUIRED - Create User if Unique ALTERNATIVE - Handle existing account ALTERNATIVE ... For the inspiration, you can create new Realm in Keycloak 8.0 and take a look how the default FirstBrokerLogin flow looks in that new realm. So you need to change it to the same/similar configuration in existing realms.
    • Git Pull Request:
    • Docs QE Status:
      NEW
    • QE Status:
      NEW

      Description

      What

      Multifactor authentication prototype has broken the behavior of the default first broke login flow. see: https://issues.jboss.org/browse/KEYCLOAK-11745

      The first broke login flow looked as followed in the Keycloak version older than 8.0 :

      • Review Profile - REQUIRED
      • Create User if Unique - ALTERNATIVE
      • Handle Existing Account - ALTERNATIVE
        • sub things ....

      The DefaultAuthenticationFlow.java has now following code snippet (line 280):

      @Override
      public Response processFlow() {
              //separate flow elements into required and alternative elements
              List<AuthenticationExecutionModel> requiredList = new ArrayList<>();
              List<AuthenticationExecutionModel> alternativeList = new ArrayList<>();
      
              for (AuthenticationExecutionModel execution : executions) {
                  ...    //  fill required and alternative executions in separate list
              }
      
              //handle required elements : all required elements need to be executed
              boolean requiredElementsSuccessful = true;
              Iterator<AuthenticationExecutionModel> requiredIListIterator = requiredList.listIterator();
              while (requiredIListIterator.hasNext()) {
                  ...   // do required executions 
              }
      
              //Evaluate alternative elements only if there are no required elements. This may also occur if there was only condition elements
              if (requiredList.isEmpty()) {
                   // The above line is the problem --> ONLY do alternative executions when there are no required actions on the same level
                    ....
               }
      }
      

      This implies that a user is never created (if unique) and thus not added to the authSession and therefore this results in an error page with invalid_user_credentials
      in log:

      14:50:29,880 WARN  [org.keycloak.events] (default task-15) type=UPDATE_PROFILE_ERROR, realmId=demo, clientId=account, userId=null, ipAddress=127.0.0.1, error=invalid_user_credentials, identity_provider=itsme-oidc, auth_method=openid-connect, updated_email=t@t, redirect_uri=http://localhost:8080/auth/realms/demo/account/login-redirect, identity_provider_identity=9rlg9oixbdlu3uomid9wwiffxj8ro6n0rbff, code_id=zDSmnzjFiIGK9JiAEeHL4_jIP1Iy7SJlOVlFkOc8RTI, authSessionParentId=2f5958b6-50f4-483e-b5ff-d69689a4db83, authSessionTabId=4NZZn8i14Yg
      14:55:44,176 WARN  [org.keycloak.services] (default task-17) KC-SERVICES0013: Failed authentication: org.keycloak.authentication.AuthenticationFlowException
      	at org.keycloak.authentication.AuthenticationProcessor.authenticationAction(AuthenticationProcessor.java:961)
      

      How

      Possible fix of this JIRA:

      • Create new class like MigrateTo9_0_0 . Register that class in MigrationModelManager.
      • In that class, we should have migration of the flows done in a way that if there is any authenticationFlow with the ALTERNATIVE elements configured after REQUIRED elements like this:
      - Auth1 - REQUIRED
      - Auth2 - ALTERNATIVE
      - Auth3 - ALTERNATIVE
      

      we will re-configure it by creating separate subflow and move ALTERNATIVE elements to that subflow. So after migration, it will look like this:

      - Auth1 - REQUIRED
      - New subflow - Auth2 or Auth3 - REQUIRED
          - Auth2 - ALTERNATIVE
          - Auth3 - ALTERNATIVE
      
      • In case that flow is mixing REQUIRED and ALTERNATIVE at same level, but there are some more REQUIRED after alternatives, we won't do anything. For example like this:
        - Auth1 REQUIRED
        - Auth2 ALTERNATIVE
        - Auth3 REQUIRED
        

        In that case, we may just need to log WARNING that administrator should migrate the flows manually.

      • Will be good to add new test to MigrationTest and ensure that "First Broker Login" flow looks as expected after the migration (There are not REQUIRED and ALTERNATIVE at the same level anymore). Bonus will be to trigger broker login during MigrationTest, but that could be lots of the work (as brokering will need to be setup etc in the MigrationTest). So hopefully manually doublecheck the flow content will be fine.
      • Some new section to the migration documentation should be added. Especially the documentation should mention that REQUIRED and ALTERNATIVE can't be at same level anymore and administrator should change his authentication flow if he uses the setup like this. It is recommended to doublecheck that configuration of your authentication flows looks as expected and it is recommended to do some testing if behaviour is as expected.
      • Hint: for the inspiration, take a look at class DefaultAuthenticationFlows and check the history of that class and how the "First Broker Login" flow was re-configured in the commit for adding authentication prototype - commit for KEYCLOAK-11745

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              mposolda Marek Posolda
              Reporter:
              tom.rutsaert Tom Rutsaert
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: