Skip to content

Commit

Permalink
Moving client->authenticated to a flag instead of an int
Browse files Browse the repository at this point in the history
Signed-off-by: artikell <[email protected]>
  • Loading branch information
artikell committed Jun 9, 2024
1 parent d28ae52 commit dd270df
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 13 deletions.
4 changes: 2 additions & 2 deletions src/acl.c
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ void ACLFreeUserAndKillClients(user *u) {
* more defensive to set the default user and put
* it in non authenticated mode. */
c->user = DefaultUser;
c->authenticated = 0;
c->flags &= ~CLIENT_AUTHENTICATED;
/* We will write replies to this client later, so we can't
* close it directly even if async. */
if (c == server.current_client) {
Expand Down Expand Up @@ -1494,7 +1494,7 @@ void addAuthErrReply(client *c, robj *err) {
* The return value is AUTH_OK on success (valid username / password pair) & AUTH_ERR otherwise. */
int checkPasswordBasedAuth(client *c, robj *username, robj *password) {
if (ACLCheckUserCredentials(username, password) == C_OK) {
c->authenticated = 1;
c->flags |= CLIENT_AUTHENTICATED;
c->user = ACLGetUserByName(username->ptr, sdslen(username->ptr));
moduleNotifyUserChanged(c);
return AUTH_OK;
Expand Down
6 changes: 3 additions & 3 deletions src/module.c
Original file line number Diff line number Diff line change
Expand Up @@ -7917,7 +7917,7 @@ int checkModuleAuthentication(client *c, robj *username, robj *password, robj **
}
if (c->flags & CLIENT_MODULE_AUTH_HAS_RESULT) {
c->flags &= ~CLIENT_MODULE_AUTH_HAS_RESULT;
if (c->authenticated) return AUTH_OK;
if (c->flags & CLIENT_AUTHENTICATED) return AUTH_OK;
}
return AUTH_ERR;
}
Expand Down Expand Up @@ -9465,7 +9465,7 @@ void revokeClientAuthentication(client *c) {
moduleNotifyUserChanged(c);

c->user = DefaultUser;
c->authenticated = 0;
c->flags &= ~CLIENT_AUTHENTICATED;
/* We will write replies to this client later, so we can't close it
* directly even if async. */
if (c == server.current_client) {
Expand Down Expand Up @@ -9787,7 +9787,7 @@ static int authenticateClientWithUser(ValkeyModuleCtx *ctx,
moduleNotifyUserChanged(ctx->client);

ctx->client->user = user;
ctx->client->authenticated = 1;
ctx->client->flags |= CLIENT_AUTHENTICATED;

if (clientHasModuleAuthInProgress(ctx->client)) {
ctx->client->flags |= CLIENT_MODULE_AUTH_HAS_RESULT;
Expand Down
12 changes: 8 additions & 4 deletions src/networking.c
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,18 @@ static void clientSetDefaultAuth(client *c) {
/* If the default user does not require authentication, the user is
* directly authenticated. */
c->user = DefaultUser;
c->authenticated = (c->user->flags & USER_FLAG_NOPASS) && !(c->user->flags & USER_FLAG_DISABLED);
if ((c->user->flags & USER_FLAG_NOPASS) && !(c->user->flags & USER_FLAG_DISABLED)) {
c->flags |= CLIENT_AUTHENTICATED;
} else {
c->flags &= ~CLIENT_AUTHENTICATED;
}
}

int authRequired(client *c) {
/* Check if the user is authenticated. This check is skipped in case
* the default user is flagged as "nopass" and is active. */
int auth_required =
(!(DefaultUser->flags & USER_FLAG_NOPASS) || (DefaultUser->flags & USER_FLAG_DISABLED)) && !c->authenticated;
int auth_required = (!(DefaultUser->flags & USER_FLAG_NOPASS) || (DefaultUser->flags & USER_FLAG_DISABLED)) &&
!(c->flags & CLIENT_AUTHENTICATED);
return auth_required;
}

Expand Down Expand Up @@ -3642,7 +3646,7 @@ void helloCommand(client *c) {
}

/* At this point we need to be authenticated to continue. */
if (!c->authenticated) {
if (!(c->flags & CLIENT_AUTHENTICATED)) {
addReplyError(c, "-NOAUTH HELLO must be called with the client already "
"authenticated, otherwise the HELLO <proto> AUTH <user> <pass> "
"option can be used to authenticate the client and "
Expand Down
5 changes: 2 additions & 3 deletions src/replication.c
Original file line number Diff line number Diff line change
Expand Up @@ -1721,12 +1721,11 @@ void replicationCreateMasterClient(connection *conn, int dbid) {
* to pass the execution to a background thread and unblock after the
* execution is done. This is the reason why we allow blocking the replication
* connection. */
server.primary->flags |= CLIENT_PRIMARY;
server.primary->flags |= (CLIENT_PRIMARY | CLIENT_AUTHENTICATED);

/* Allocate a private query buffer for the primary client instead of using the shared query buffer.
* This is done because the primary's query buffer data needs to be preserved for my sub-replicas to use. */
server.primary->querybuf = sdsempty();
server.primary->authenticated = 1;
server.primary->reploff = server.primary_initial_offset;
server.primary->read_reploff = server.primary->reploff;
server.primary->user = NULL; /* This client can do everything. */
Expand Down Expand Up @@ -3306,7 +3305,7 @@ void replicationResurrectCachedMaster(connection *conn) {
server.primary->conn = conn;
connSetPrivateData(server.primary->conn, server.primary);
server.primary->flags &= ~(CLIENT_CLOSE_AFTER_REPLY | CLIENT_CLOSE_ASAP);
server.primary->authenticated = 1;
server.primary->flags |= CLIENT_AUTHENTICATED;
server.primary->last_interaction = server.unixtime;
server.repl_state = REPL_STATE_CONNECTED;
server.repl_down_since = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,7 @@ extern int configOOMScoreAdjValuesDefaults[CONFIG_OOM_COUNT];
#define CLIENT_MODULE_PREVENT_REPL_PROP (1ULL << 49) /* Module client do not want to propagate to replica */
#define CLIENT_REPROCESSING_COMMAND (1ULL << 50) /* The client is re-processing the command. */
#define CLIENT_REPLICATION_DONE (1ULL << 51) /* Indicate that replication has been done on the client */
#define CLIENT_AUTHENTICATED (1ULL << 52) /* Indicate a client has successfully authenticated */

/* Client block type (btype field in client structure)
* if CLIENT_BLOCKED flag is set. */
Expand Down Expand Up @@ -1238,7 +1239,6 @@ typedef struct client {
dictEntry *cur_script; /* Cached pointer to the dictEntry of the script being executed. */
time_t last_interaction; /* Time of the last interaction, used for timeout */
time_t obuf_soft_limit_reached_time;
int authenticated; /* Needed when the default user requires auth. */
int repl_state; /* Replication state if this is a replica. */
int repl_start_cmd_stream_on_ack; /* Install replica write handler on first ACK. */
int repldbfd; /* Replication DB file descriptor. */
Expand Down

0 comments on commit dd270df

Please sign in to comment.