The problem/use-case that the feature addresses

It is very difficult to implement a custom AUTH command, since we also need to support the HELLO use cases. Specifically, HELLO variations such as: HELLO 3 setname clientname <name> where we need to make changes on the actual client. Currently, there is no way for a Redis Module to run commands on the actual Redis client that called into the Module (example: through module commands). Using the RM_Call API to call the command: HELLO 3 setname clientname <name> - to set the clientname and resp protocol will not work since it uses a fake client in its implementation.

Description of the feature

Extend RM_Call with a new flag “REDISMODULE_ARGV_MYSELF”.

When this flag is provided to the RM_Call API, it will indicate that we want to call the command on the same client (in RedisModuleCtx). With this, instead of using a fake client (as RM_Call currently does), it will run call() using ctx->client and then return the response through RedisModuleCallReply.

RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const char *fmt, ...) {

Alternatives you've considered

  1. The alternative is to create a Module API specifically for the purpose of setting the clientname and resp fields of the actual client from the RedisModuleContext.
int RM_set_client_fields(RedisModuleCtx *ctx, const char* client_name, int protocol) {
  1. Update the client setname command to optionally accept client ID. With this, we can use the RM_Call API to run this command along with the ID of the client that we want to rename:
client setname <name> clientid <id>

Comment From: madolson

This implementation is based off of the ideas documented here, https://github.com/redis/redis/issues/8329, of using command filtering in order to rename the authentication commands in order to provide custom auth. This addresses the problem brought up there as well that we can't really intercept HELLO, because we can't emulate the response easily.

My intuition is that the recommended option, being able to execute RM_Call() with the real client arguments, is likely going to have edge cases I'm not aware of. So would like to ping @oranagra to evaluate the feasibility about that.

The backup option of providing and API to emulate HELLO seems reasonable enough.

Comment From: oranagra

I think that letting the module use the real client for RM_Call is the wrong approach. Modules should generally have an API to let them manipulate redis, rather than use RM_Call to execute (limited) user commands. I think the right solution is certainly to add APIs to manipulate client fields and other internals directly. e.g. RM_SetClientName, RM_SetClientProtocol, etc. (to go alongside with the existing RM_AuthenticateClientWithUser)

Comment From: zuiderkwast

I can implement these two API functions.

Actually, I'll go for RM_SetClientNameById and RM_SetClientProtocolById to match e.g. RM_GetClientUserNameById. You can use RM_GetClientId to get the current client id.

Comment From: yossigo

I want to propose reconsidering the idea of extending AUTH and HELLO by intercepting and re-implementing them.

Plugging an authentication module is a viable use case. Because of client compatibility, it's reasonable to assume the command syntax won't change, only the implementation of turning user/password into a user structure. So, wouldn't it make more sense for a module to register an authentication function that returns a user?

BTW I don't have anything against RM_SetClientNameById. I'm not in favor of RM_SetClientProtocolById as I believe it promotes modules that make Redis incompatible with clients, that shouldn't expect the protocol version to suddenly change.

Comment From: zuiderkwast

Perhaps we should provide a way to register an auth csllback instead? RM_RegisterAuthCallback, which callback is called by the hello and auth commands.

Comment From: madolson

@zuiderkwast @yossigo You two said the same thing right?

The pushback was w.r.t. to blocking commands, since we have no way to block in the middle of an AUTH or HELLO command. I don't think it would be all that difficult to extend the blocking framework so we can block the authentication commands either.

The more details that get fleshed out here, it seems a lot more straight forward to just implement a custom authentication hook directly.

Comment From: madolson

Related discussion: https://github.com/redis/redis/issues/8329

Comment From: zuiderkwast

Yes I said the same thing as Yossi. It was all his idea. :-)

Comment From: madolson

Summary from core meeting was that adding custom authentication is probably the right approach, so next step is to spend some time investigating what is the best API for this.

Comment From: zuiderkwast

Roughly, I imagine an API function for registering an auth callback and hook in ACLCheckUserCredentials that calls the auth callback. Only one module can do this so there should probably be some check that the auth callback isn't already set and a way to unregister it, which should also happen when the module is unloaded.

First I thought about a callback that simply gets username and password and returns a boolean, but it should also play well with the existing ACL APIs such as RM_AuthenticateClientWithUser. I'm not enough familiar with these, so I unassigned myself.

@KarthikSubbarao do you want to continue this?

Comment From: madolson

@KarthikSubbarao Said he wanted to pick it up, he works at AWS so assigning it to myself. He'll follow up and post a design.

Comment From: zuiderkwast

Replaced by #11256