Repository: valkey-io/valkey
File: src/acl.c (C)

Code before:
 * are correct, the connection is put in authenticated state and the
 * connection user reference is populated.
 *
 * The return value is AUTH_OK on success (valid username / password pair) & AUTH_ERR otherwise. */
static int checkPasswordBasedAuth(client *c, robj *username, robj *password) {
    int result;

    if (ACLCheckUserCredentials(username, password) == C_OK) {
        user *user = ACLGetUserByName(username->ptr, sdslen(username->ptr));
        clientSetUser(c, user, 1);
        moduleNotifyUserChanged(c);
        result = AUTH_OK;
    } else {
        addACLLogEntry(c, ACL_DENIED_AUTH, (c->flag.multi) ? ACL_LOG_CTX_MULTI : ACL_LOG_CTX_TOPLEVEL, 0, username->ptr,
                       NULL);
        result = AUTH_ERR;
    }

    {
        ValkeyModuleAuthenticationInfo info = VALKEYMODULE_AUTHENTICATIONINFO_INITIALIZER_V1;
        info.client_id = c->id;
        info.username = username->ptr;
        info.module_name = NULL;
        info.result = result == AUTH_OK ? VALKEYMODULE_AUTH_RESULT_GRANTED
                                        : VALKEYMODULE_AUTH_RESULT_DENIED;
        moduleFireServerEvent(VALKEYMODULE_EVENT_AUTHENTICATION_ATTEMPT, 0, &info);
    }

    return result;
}

/* Attempt authenticating the user - first through module based authentication,
 * and then, if needed, with normal password based authentication.
 * Returns one of the following codes:
 * AUTH_OK - Indicates that authentication succeeded.
 * AUTH_ERR - Indicates that authentication failed.
 * AUTH_BLOCKED - Indicates module authentication is in progress through a blocking implementation.
 */
int ACLAuthenticateUser(client *c, robj *username, robj *password, robj **err) {
    int result = checkModuleAuthentication(c, username, password, err);
    /* If authentication was not handled by any Module, attempt normal password based auth. */
    if (result == AUTH_NOT_HANDLED) {
        result = checkPasswordBasedAuth(c, username, password);
    }
    return result;
}

/* For ACL purposes, every user has a bitmap with the commands that such
 * user is allowed to execute. In order to populate the bitmap, every command
 * should have an assigned ID (that is used to index the bitmap). This function
 * creates such an ID: it uses sequential IDs, reusing the same ID for the same

Review comment (security):
This code is not large, but it's duplicated. If we change V1 to V2 in the future, we'll need to remember to change in both places.

Let's move this and the duplicated code in `checkModuleAuthentication` to a new function in `module.{c,h}`? e.g. `moduleFireAuthenticationEvent(client_id, username, module_name, is_granted)`.

Then, acl.c doesn't need to include valkeymodule.h. It's enough to include module.h.

Code after:
 * connection user reference is populated.
 *
 * The return value is AUTH_OK on success (valid username / password pair) & AUTH_ERR otherwise. */
static int checkPasswordBasedAuth(client *c, robj *username, robj *password) {
    int result;

    if (ACLCheckUserCredentials(username, password) == C_OK) {
        user *user = ACLGetUserByName(username->ptr, sdslen(username->ptr));
        clientSetUser(c, user, 1);
        moduleNotifyUserChanged(c);
        result = AUTH_OK;
    } else {
        addACLLogEntry(c, ACL_DENIED_AUTH, (c->flag.multi) ? ACL_LOG_CTX_MULTI : ACL_LOG_CTX_TOPLEVEL, 0, username->ptr,
                       NULL);
        result = AUTH_ERR;
    }

    moduleFireAuthenticationEvent(c->id,
                                  username->ptr,
                                  NULL,
                                  result == AUTH_OK);

    return result;
}

/* Attempt authenticating the user - first through module based authentication,
 * and then, if needed, with normal password based authentication.
 * Returns one of the following codes:
 * AUTH_OK - Indicates that authentication succeeded.
 * AUTH_ERR - Indicates that authentication failed.
 * AUTH_BLOCKED - Indicates module authentication is in progress through a blocking implementation.
 */
int ACLAuthenticateUser(client *c, robj *username, robj *password, robj **err) {
    int result = checkModuleAuthentication(c, username, password, err);
    /* If authentication was not handled by any Module, attempt normal password based auth. */
    if (result == AUTH_NOT_HANDLED) {
        result = checkPasswordBasedAuth(c, username, password);
    }
    return result;
}

/* For ACL purposes, every user has a bitmap with the commands that such
 * user is allowed to execute. In order to populate the bitmap, every command
 * should have an assigned ID (that is used to index the bitmap). This function
 * creates such an ID: it uses sequential IDs, reusing the same ID for the same
 * command name, so that a command retains the same ID in case of modules that
 * are unloaded and later reloaded.
 *
 * The function does not take ownership of the 'cmdname' SDS string.
 * */
unsigned long ACLGetCommandID(sds cmdname) {