fix: jwt auth default without password should be rejected by kostasrim · Pull Request #8291 · dragonflydb/dragonfly

kostasrim · 2026-09-10T08:25:49Z

Reject a default user authentication without a jwt token when jwt authentication is on

Signed-off-by: Kostas Kyrimis <kostas@dragonflydb.io>

qodo-code-review · 2026-09-10T08:25:54Z

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

augmentcode · 2026-09-10T08:28:00Z

🤖 Augment PR Summary

Summary: Requires authentication on newly created non-privileged Redis connections whenever JWT validation is enabled, even if the default ACL user is nopass.
Why: Prevents implicit default-user access from bypassing JWT token validation.

_{🤖 Was this summary useful? React with 👍 or 👎}

augmentcode

Review completed. 2 suggestions posted.

Comment augment review to trigger a new review at any time.

augmentcode · 2026-09-10T08:28:01Z

    } else {
-      res->req_auth = !user_registry_.AuthUser("default", "");
+      // JWT validation must gate every connection even if the local "default" user is nopass.
+      res->req_auth = acl::JwtValidator::IsEnabled() || !user_registry_.AuthUser("default", "");


src/server/main_service.cc:1897 — Existing non-privileged Redis connections opened while the default user is nopass retain req_auth == false; because this check only runs in CreateContext, they continue executing commands without a JWT after CONFIG SET jwt_validate yes, despite this flag being runtime-mutable. This leaves the intended authentication boundary bypassable until those connections close.

Severity: high

_{🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.}

augmentcode · 2026-09-10T08:28:01Z

    } else {
-      res->req_auth = !user_registry_.AuthUser("default", "");
+      // JWT validation must gate every connection even if the local "default" user is nopass.
+      res->req_auth = acl::JwtValidator::IsEnabled() || !user_registry_.AuthUser("default", "");


src/server/main_service.cc:1897 — This JWT gate is never reached for MEMCACHE connections: the preceding branch explicitly sets authenticated true and req_auth false. Consequently, enabling --memcached_port alongside JWT validation still gives unauthenticated clients default-user access without a token.

Severity: high

_{🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.}

Copilot

🟡 Changes recommended

The new JWT gating behavior is security-sensitive and should be covered by an automated regression test to prevent future bypass regressions.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR tightens the authentication gate in main_service so that when JWT-mode auth is enabled, clients can’t bypass authentication via a nopass default user on the Redis protocol listener.

Changes:

Require authentication on non-privileged Redis connections whenever acl::JwtValidator::IsEnabled() is true, even if default would otherwise authenticate with an empty password.

File summaries

File	Description
src/server/main_service.cc	Forces `req_auth` when JWT auth is enabled to prevent unauthenticated access via `default`/`nopass`.

Review details

Files reviewed: 1/1 changed files
Comments generated: 2
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

    } else {
-      res->req_auth = !user_registry_.AuthUser("default", "");
+      // JWT validation must gate every connection even if the local "default" user is nopass.
+      res->req_auth = acl::JwtValidator::IsEnabled() || !user_registry_.AuthUser("default", "");


      res->authenticated = true;  // Automatically authenticated for Memcached protocol
    } else {
-      res->req_auth = !user_registry_.AuthUser("default", "");
+      // JWT validation must gate every connection even if the local "default" user is nopass.


qodo-free-for-open-source-projects · 2026-09-10T08:30:00Z

PR Summary by Qodo

Enforce JWT authentication for nopass default users

🐞 Bug fix 🕐 Less than 10 minutes

AI Description

• Requires authentication on new Redis connections whenever JWT validation is enabled.
• Prevents nopass default users from bypassing JWT token validation.

Diagram

sequenceDiagram
  actor Client
  participant Service as Main Service
  participant JWT as JWT Validator
  participant Registry as User Registry
  Client->>Service: Open connection
  Service->>JWT: Check enabled
  JWT-->>Service: Mode status
  alt JWT enabled
    Service-->>Client: Require AUTH token
  else JWT disabled
    Service->>Registry: Check default nopass
    Registry-->>Service: Local auth result
    Service-->>Client: Set auth requirement
  end

High-Level Assessment

The direct JWT-mode check during connection-context creation is the appropriate fix because this is where the initial authentication gate is established. Moving the behavior into the user registry would incorrectly couple external JWT policy to local password validation, while changing command dispatch would address the bypass later and less clearly.

Files changed (1) +3 / -1

Bug fix (1) +3 / -1

main_service.ccRequire authentication when JWT validation is enabled +3/-1

Require authentication when JWT validation is enabled

• Imports the JWT validator and checks its enabled state while creating non-privileged Redis connection contexts. JWT mode now requires authentication even when the local default user accepts an empty password, preventing unauthenticated command access.

src/server/main_service.cc

qodo-free-for-open-source-projects · 2026-09-10T08:31:29Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

🔴 High

1. Existing clients bypass enabled JWT 🐞 Bug ⛨ Security

Description

Service::CreateContext snapshots JwtValidator::IsEnabled() into req_auth only when a
connection is created, while jwt_validate can later be changed through CONFIG SET. When JWT is
enabled at runtime, existing default-user connections retain req_auth == false and `authenticated
== false`, so command dispatch continues accepting their commands without a token.

Code

src/server/main_service.cc[1897]

+      res->req_auth = acl::JwtValidator::IsEnabled() || !user_registry_.AuthUser("default", "");

Evidence
The JWT flag is explicitly registered as mutable and documented as toggleable without restart, but
req_auth is assigned only in CreateContext. ConnectionContext initializes both req_auth and
authenticated to false, and dispatch gates commands solely when both req_auth is true and
authenticated is false, proving that a pre-existing nopass connection remains ungated after the
flag changes.
src/server/main_service.cc[1067-1078]
src/server/acl/jwt_validator.cc[27-32]
src/server/main_service.cc[1387-1393]
src/server/main_service.cc[1880-1902]
src/facade/conn_context.h[15-28]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
JWT mode is runtime-mutable, but normal connections only capture its state when their context is created. Connections opened while JWT is disabled and the default user is `nopass` therefore remain unrestricted after JWT is enabled.

## Fix Focus Areas
- src/server/main_service.cc[1387-1403]
- src/server/main_service.cc[1880-1902]
- src/server/main_service.cc[1067-1078]

## Recommended Fix
Evaluate the current JWT mode during command dispatch for eligible non-privileged Redis connections, rather than relying only on the connection-time `req_auth` snapshot. Preserve the intentional UDS, privileged, and Memcached exemptions, centralize the eligibility condition so creation and dispatch cannot diverge, and add a test that opens a nopass connection before enabling JWT and verifies its subsequent non-authentication commands receive `NOAUTH`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context sources

✅ Cross-repo context — repo relationships

Review mode: ⚖️ Balanced: This is a security-sensitive authentication behavior change affecting connection authorization, so it warrants a complete careful review despite the small diff.

Tip of the day

💡 Did you know, you can turn these tips off under Display preferences

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Powered by Qodo

qodo-free-for-open-source-projects · 2026-09-10T08:31:30Z

    } else {
-      res->req_auth = !user_registry_.AuthUser("default", "");
+      // JWT validation must gate every connection even if the local "default" user is nopass.
+      res->req_auth = acl::JwtValidator::IsEnabled() || !user_registry_.AuthUser("default", "");


🔴 High

1. Existing clients bypass enabled jwt 🐞 Bug ⛨ Security

Service::CreateContext snapshots JwtValidator::IsEnabled() into req_auth only when a connection is created, while jwt_validate can later be changed through CONFIG SET. When JWT is enabled at runtime, existing default-user connections retain req_auth == false and `authenticated == false`, so command dispatch continues accepting their commands without a token.

Agent Prompt

## Issue description JWT mode is runtime-mutable, but normal connections only capture its state when their context is created. Connections opened while JWT is disabled and the default user is `nopass` therefore remain unrestricted after JWT is enabled. ## Fix Focus Areas - src/server/main_service.cc[1387-1403] - src/server/main_service.cc[1880-1902] - src/server/main_service.cc[1067-1078] ## Recommended Fix Evaluate the current JWT mode during command dispatch for eligible non-privileged Redis connections, rather than relying only on the connection-time `req_auth` snapshot. Preserve the intentional UDS, privileged, and Memcached exemptions, centralize the eligibility condition so creation and dispatch cannot diverge, and add a test that opens a nopass connection before enabling JWT and verifies its subsequent non-authentication commands receive `NOAUTH`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

fix: jwt auth default without password should be rejected

2506348

Signed-off-by: Kostas Kyrimis <kostas@dragonflydb.io>

Copilot AI lite review requested due to automatic review settings September 10, 2026 08:25

kostasrim self-assigned this Sep 10, 2026

Copilot started reviewing on behalf of kostasrim September 10, 2026 08:26 View session

augmentcode Bot reviewed Sep 10, 2026

View reviewed changes

Copilot AI reviewed Sep 10, 2026

View reviewed changes

qodo-free-for-open-source-projects Bot reviewed Sep 10, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix: jwt auth default without password should be rejected - #8291

fix: jwt auth default without password should be rejected#8291
kostasrim wants to merge 1 commit into
mainfrom
fix-jwt

kostasrim commented Sep 10, 2026

Uh oh!

qodo-code-review Bot commented Sep 10, 2026

Uh oh!

augmentcode Bot commented Sep 10, 2026

Uh oh!

augmentcode Bot left a comment

Uh oh!

augmentcode Bot Sep 10, 2026 •

edited

Loading

Uh oh!

augmentcode Bot Sep 10, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 10, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 10, 2026

Uh oh!

qodo-free-for-open-source-projects Bot Sep 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

kostasrim commented Sep 10, 2026

Uh oh!

qodo-code-review Bot commented Sep 10, 2026

Qodo reviews are paused for this user.

Uh oh!

augmentcode Bot commented Sep 10, 2026

Uh oh!

augmentcode Bot left a comment

Choose a reason for hiding this comment

Uh oh!

augmentcode Bot Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

augmentcode Bot Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 10, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 10, 2026

Code Review by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

augmentcode Bot Sep 10, 2026 •

edited

Loading

augmentcode Bot Sep 10, 2026 •

edited

Loading