Skip to content

Add HTTP Basic Auth support to deploy#216

Open
r4mbo7 wants to merge 2 commits into
basecamp:mainfrom
r4mbo7:basic-auth
Open

Add HTTP Basic Auth support to deploy#216
r4mbo7 wants to merge 2 commits into
basecamp:mainfrom
r4mbo7:basic-auth

Conversation

@r4mbo7

@r4mbo7 r4mbo7 commented Jun 4, 2026

Copy link
Copy Markdown

Add a --basic-auth flag to kamal-proxy deploy that protects a service behind HTTP Basic Authentication. Requests without valid credentials receive a 401 with a WWW-Authenticate challenge.

The password is hashed (SHA-256) before it is stored in the service options, so the proxy never persists it in plaintext, and credentials are compared in constant time to avoid timing attacks. The middleware runs inside the error-page middleware so a custom 401 page can render.

Fixes basecamp/kamal#1604
companion PR: basecamp/kamal#1865

Add a --basic-auth flag to `kamal-proxy deploy` that protects a service
behind HTTP Basic Authentication. Requests without valid credentials
receive a 401 with a WWW-Authenticate challenge.

The password is hashed (SHA-256) before it is stored in the service
options, so the proxy never persists it in plaintext, and credentials
are compared in constant time to avoid timing attacks. The middleware
runs inside the error-page middleware so a custom 401 page can render.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 4, 2026 22:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds optional HTTP Basic Authentication to proxied services, configurable via the deploy CLI, implemented as middleware, and documented with basic usage guidance.

Changes:

  • Extend ServiceOptions with basic auth username + password hash and wire middleware into the service handler chain.
  • Add a Basic Auth middleware implementation plus unit tests.
  • Add --basic-auth username:password deploy flag and document usage in README.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
internal/server/service.go Adds basic auth fields to service options and conditionally wraps the service handler with auth middleware
internal/server/basic_auth_middleware.go Introduces Basic Auth middleware, hashing helper, and constant-time comparisons
internal/server/basic_auth_middleware_test.go Adds unit tests covering allowed/denied basic-auth request paths
internal/cmd/deploy.go Adds --basic-auth parsing and stores hashed password into service options
README.md Documents how to enable basic auth during deploy and describes behavior

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/server/service.go Outdated
Comment on lines +12 to +18
// HashBasicAuthCredential returns the hex-encoded SHA-256 digest of a Basic Auth
// credential. The password is hashed before it is stored in the service options,
// so the proxy never persists it in plaintext.
func HashBasicAuthCredential(value string) string {
sum := sha256.Sum256([]byte(value))
return hex.EncodeToString(sum[:])
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deliberately keeping SHA-256 here. This isn't a password database — it's a single, operator-chosen shared credential whose plaintext already lives in the deploy config, so the hash is just defense-in-depth for the persisted state file rather than protection against mass offline cracking. The bigger issue with an adaptive hash is cost: bcrypt/Argon2 would run on every proxied request on the hot path (tens of ms each), which is a self-inflicted DoS vector for a reverse proxy. Comparisons are already constant-time, so the timing-safety goal is met. Happy to revisit if you'd prefer an adaptive hash with a verification cache, but that seemed like more machinery than this feature warrants — open to your call.

Comment thread internal/server/basic_auth_middleware.go
Comment thread internal/server/basic_auth_middleware.go
Comment thread internal/cmd/deploy.go
deployCommand.cmd.Flags().BoolVar(&deployCommand.args.TargetOptions.ForwardHeaders, "forward-headers", false, "Forward X-Forwarded headers to target (default false if TLS enabled; otherwise true)")
deployCommand.cmd.Flags().BoolVar(&deployCommand.args.TargetOptions.ScopeCookiePaths, "scope-cookie-paths", false, "Scope cookie paths to match path prefix")

deployCommand.cmd.Flags().StringVar(&deployCommand.basicAuth, "basic-auth", "", "Require HTTP Basic Auth, in the form username:password")

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point on the leak surface, and I've added a README warning. I kept it as a single --basic-auth flag for now because (a) this command is invoked by Kamal over SSH via docker exec, not typed interactively, so shell history doesn't really apply, and (b) every other kamal-proxy deploy option is a plain flag — a --basic-auth-password-stdin/env/file variant would be an inconsistent interface here and wouldn't fully close the gap unless Kamal adopted stdin on its side too. I'm happy to add a stdin/env/file input as a follow-up if you'd like it; just wanted to keep this PR's surface consistent with the existing CLI.

Comment thread README.md
Comment thread internal/server/basic_auth_middleware_test.go
- Only enable basic auth when both username and password hash are set, so a
  partially-configured service isn't silently locked out (BasicAuthEnabled).
- Fail closed on a malformed/wrong-length stored hash by falling back to a
  fixed-length zero hash, keeping comparisons constant-time over equal lengths.
- Precompute the username hash once at construction instead of per request.
- Document that command-line credentials can leak via shell history, process
  listings, and CI logs.
- Cover the invalid-hash cases with tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Comment on lines +12 to +18
// HashBasicAuthCredential returns the hex-encoded SHA-256 digest of a Basic Auth
// credential. The password is hashed before it is stored in the service options,
// so the proxy never persists it in plaintext.
func HashBasicAuthCredential(value string) string {
sum := sha256.Sum256([]byte(value))
return hex.EncodeToString(sum[:])
}
Comment on lines +26 to +33
func WithBasicAuthMiddleware(username, passwordHash string, next http.Handler) http.Handler {
// If the stored hash is malformed (bad hex or not a SHA-256 digest), fall back
// to a fixed-length zero hash. No real password hashes to all zeroes, so every
// request fails closed while comparisons still run over equal-length inputs.
decoded, err := hex.DecodeString(passwordHash)
if err != nil || len(decoded) != sha256.Size {
decoded = make([]byte, sha256.Size)
}
Comment on lines +30 to +33
decoded, err := hex.DecodeString(passwordHash)
if err != nil || len(decoded) != sha256.Size {
decoded = make([]byte, sha256.Size)
}
Comment thread internal/cmd/deploy.go
Comment on lines +95 to +102
if c.basicAuth != "" {
username, password, found := strings.Cut(c.basicAuth, ":")
if !found || username == "" || password == "" {
return fmt.Errorf("basic-auth must be in the form username:password")
}
c.args.ServiceOptions.BasicAuthUsername = username
c.args.ServiceOptions.BasicAuthPasswordHash = server.HashBasicAuthCredential(password)
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Suggestion: Basic auth

2 participants