BFD and direct grant

[Nemanja Hiršl]

Hello Keycloak team,

It seems there's a bug regarding BFD and how it behaves through browser flow or direct grant. The issue is described here: https://keycloak.discourse.group/t/brute-force-detection-failure-count-increments-browser-vs-direct-grant-authentication-flow/11059

I've tested and verified this behavior with KC 15.0.2

Analysis of the issue follows:

1. Brute force detection is turned on and configured on the realm

2. Testing the BFD algorithm through browser gives expected results: The account is (temporary) locked after configured number of (re)tries and the flow correctly detects when the account is locked.

3. When bad password is detected in  AbstractUsernameFormAuthenticator - context.failureChallenge(AuthenticationFlowError.INVALID_CREDENTIALS, challengeResponse); is returned to the user and when the locked account is detected  context.forceChallenge(challengeResponse); is returned to the user.
The difference between these two in DefaultAuthenticationFlow is that "failure" triggers the processor.logFailure(); which increments the BFD counter, and "force" don't.

4. The behavior with direct grant is different as the number of failures increases when the account is temporary locked. IMHO this should not happen and the usage from browser and direct grant shall be consistent.

5. Looking at the ValidateUsername it invokes "failure" instead of "force" challenge in cases when the user is locked:
        String bruteForceError = getDisabledByBruteForceEventError(context.getProtector(), context.getSession(), context.getRealm(), user);
        if (bruteForceError != null) {
            context.getEvent().user(user);
            context.getEvent().error(bruteForceError);
            Response challengeResponse = errorResponse(Response.Status.UNAUTHORIZED.getStatusCode(), "invalid_grant", "Invalid user credentials");
            context.failure(AuthenticationFlowError.INVALID_USER, challengeResponse);
            return;
        }

        if (!user.isEnabled()) {
            context.getEvent().user(user);
            context.getEvent().error(Errors.USER_DISABLED);
            Response challengeResponse = errorResponse(Response.Status.BAD_REQUEST.getStatusCode(), "invalid_grant", "Account disabled");
            context.failure(AuthenticationFlowError.INVALID_USER, challengeResponse);
            return;
        }

This is the root cause why the number of failures increases.

6. The solution to this issue might be just a proper call to forceChallenge:
context.forceChallenge(challengeResponse); in these two cases.

What do you think?

If you agree that this is a proper solution, just let me know and I'll create new PR. Otherwise, please suggest what might be a better way of fixing this issue.

Thanks.

Best regards,

Nemanja

[Nemanja Hiršl]

Jira issue: https://issues.redhat.com/browse/KEYCLOAK-19773

PR sent through jira