feat(portal): API token (Bearer) auth + Helm ui.token - #38
Conversation
- Bearer token bypasses session; maps to API_TOKEN_USER in DB - Helm: ui.token.value hardcoded in values with TODO for Secret Made-with: Cursor
📝 WalkthroughWalkthroughThe changes add API bearer-token authentication support to the portal application. This includes new configuration fields for token settings, Helm template modifications to inject environment variables conditionally, a new method to retrieve users by name, and middleware refactoring to parse and validate bearer tokens alongside existing cookie-based session authentication. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Middleware as Auth Middleware
participant Store as PasskeyStore
participant Config as Config
Client->>Middleware: HTTP Request + Authorization: Bearer {token}
Middleware->>Config: Check if APITokenEnabled
alt Token Auth Enabled
Middleware->>Middleware: Parse Bearer Token
Middleware->>Config: Get Configured APIToken & APITokenUser
Middleware->>Middleware: Constant-Time Compare Token
alt Token Valid
Middleware->>Store: GetUserByName(APITokenUser)
Store-->>Middleware: User Name & ID
Middleware->>Middleware: Set userID & userName
Middleware->>Client: Call Next()
else Token Invalid
Middleware->>Client: 401 Unauthorized
end
else Token Auth Disabled
Middleware->>Store: GetUserSession(sessionID)
Store-->>Middleware: Session Data
alt Session Valid
Middleware->>Client: Call Next()
else Session Invalid
Middleware->>Client: 401 / Redirect to Login
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
portal/internal/auth/adaptors/selectors.go (1)
35-40: Avoid loading WebAuthn credentials for token auth.
GetUserByNameonly needs name/ID, butdb.GetUseralso fetches and converts credentials. Query the user directly so bearer auth is not slowed or broken by unrelated credential reads.♻️ Proposed refactor
func (db *WebauthnStore) GetUserByName(userName string) (string, []byte, error) { - u, err := db.GetUser(userName) + u, err := db.queries.GetUserByName(context.Background(), userName) if err != nil { return "", nil, err } return u.Name, u.ID, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@portal/internal/auth/adaptors/selectors.go` around lines 35 - 40, GetUserByName on WebauthnStore currently calls db.GetUser which loads and converts full WebAuthn credentials; change it to fetch only the user's Name and ID (avoid calling GetUser) so token/bearer auth doesn't load credentials. Implement a direct lightweight query or new helper (e.g., GetUserBasic or a SQL select) inside WebauthnStore.GetUserByName that returns only u.Name and u.ID (or returns an error), and remove any credential-related conversions; keep the method signature the same so callers are unaffected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@chart/templates/portal.yaml`:
- Around line 49-56: The template currently renders API_TOKEN as a literal value
from .Values.ui.token.value which embeds secrets into the chart; change the
Deployment env var API_TOKEN in chart/templates/portal.yaml to use
valueFrom.secretKeyRef (the same pattern used for VCS_TOKEN, DATABASE_URL,
NGROK_AUTHTOKEN) and pull the Secret name/key from chart values (e.g.
.Values.ui.token.secret and .Values.ui.token.key) instead of
.Values.ui.token.value, and remove the literal value from values.yaml so the
Secret is provided externally.
In `@chart/values.yaml`:
- Around line 233-238: The chart currently enables API bearer auth by default
with a public token (token.enabled: true, token.user: system, token.value:
litefunctionsxxxtoken); change the defaults to safe values: set token.enabled to
false, clear token.value (empty string) and do not set a default token.user, and
add a comment instructing operators to provide a secret-backed token via a
Kubernetes Secret before enabling; update any docs referencing token.value to
require explicit secret configuration rather than shipping a usable default.
In `@portal/pkg/server/middleware/auth.go`:
- Around line 111-125: The current logic prematurely returns after a matching
bearer token even when store.GetUserByName fails, which prevents session
fallback; change the flow inside the APIToken branch (parseBearerToken,
constantTimeEqual) so that you only call c.Next() and return when
store.GetUserByName succeeds (err == nil) and you set
userID/userName/authenticated; if GetUserByName returns an error, do not
return—allow the middleware to continue so session-based auth can run as a
fallback.
- Around line 34-39: The redirect value is concatenated raw in
authRequiredFailure causing malformed URLs for paths with special characters;
change the redirect construction to use url.QueryEscape(c.Request.URL.Path) so
the path is properly URL-encoded and update imports to include net/url if
missing (ensure the redirect becomes
"/?redirect="+url.QueryEscape(c.Request.URL.Path) inside authRequiredFailure).
---
Nitpick comments:
In `@portal/internal/auth/adaptors/selectors.go`:
- Around line 35-40: GetUserByName on WebauthnStore currently calls db.GetUser
which loads and converts full WebAuthn credentials; change it to fetch only the
user's Name and ID (avoid calling GetUser) so token/bearer auth doesn't load
credentials. Implement a direct lightweight query or new helper (e.g.,
GetUserBasic or a SQL select) inside WebauthnStore.GetUserByName that returns
only u.Name and u.ID (or returns an error), and remove any credential-related
conversions; keep the method signature the same so callers are unaffected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 17173f54-504d-4fbe-bb03-338d2c50c21b
📒 Files selected for processing (6)
chart/templates/portal.yamlchart/values.yamlportal/internal/auth/adaptors/selectors.goportal/internal/auth/spec.goportal/pkg/config.goportal/pkg/server/middleware/auth.go
| {{- if .Values.ui.token.enabled }} | ||
| - name: API_TOKEN_ENABLED | ||
| value: "true" | ||
| - name: API_TOKEN_USER | ||
| value: {{ .Values.ui.token.user | quote }} | ||
| - name: API_TOKEN | ||
| value: {{ .Values.ui.token.value | quote }} | ||
| {{- end }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 'API_TOKEN|ui\.token|secretKeyRef' chartRepository: ashupednekar/litefunctions
Length of output: 6790
🏁 Script executed:
cat chart/values.yaml | sed -n '234,250p'Repository: ashupednekar/litefunctions
Length of output: 221
Read API_TOKEN from a Secret, not a literal value.
Line 55 renders the bearer token into the Deployment manifest and Helm release metadata. The value is currently stored as plain text in chart/values.yaml (litefunctionsxxxtoken), which should never be committed to version control. Use valueFrom.secretKeyRef with chart values for the Secret name/key, following the pattern already established for VCS_TOKEN, DATABASE_URL, and NGROK_AUTHTOKEN elsewhere in the chart.
🔒 Proposed template direction
- name: API_TOKEN
- value: {{ .Values.ui.token.value | quote }}
+ valueFrom:
+ secretKeyRef:
+ name: {{ required "ui.token.secret is required when ui.token.enabled=true" .Values.ui.token.secret | quote }}
+ key: {{ required "ui.token.key is required when ui.token.enabled=true" .Values.ui.token.key | quote }}Update chart/values.yaml:
ui:
token:
enabled: true
user: system
secret: litefunctions-api-token
key: tokenRemove the value field from chart/values.yaml and ensure the Secret is created separately (outside the chart or via external secret management).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{- if .Values.ui.token.enabled }} | |
| - name: API_TOKEN_ENABLED | |
| value: "true" | |
| - name: API_TOKEN_USER | |
| value: {{ .Values.ui.token.user | quote }} | |
| - name: API_TOKEN | |
| value: {{ .Values.ui.token.value | quote }} | |
| {{- end }} | |
| {{- if .Values.ui.token.enabled }} | |
| - name: API_TOKEN_ENABLED | |
| value: "true" | |
| - name: API_TOKEN_USER | |
| value: {{ .Values.ui.token.user | quote }} | |
| - name: API_TOKEN | |
| valueFrom: | |
| secretKeyRef: | |
| name: {{ required "ui.token.secret is required when ui.token.enabled=true" .Values.ui.token.secret | quote }} | |
| key: {{ required "ui.token.key is required when ui.token.enabled=true" .Values.ui.token.key | quote }} | |
| {{- end }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@chart/templates/portal.yaml` around lines 49 - 56, The template currently
renders API_TOKEN as a literal value from .Values.ui.token.value which embeds
secrets into the chart; change the Deployment env var API_TOKEN in
chart/templates/portal.yaml to use valueFrom.secretKeyRef (the same pattern used
for VCS_TOKEN, DATABASE_URL, NGROK_AUTHTOKEN) and pull the Secret name/key from
chart values (e.g. .Values.ui.token.secret and .Values.ui.token.key) instead of
.Values.ui.token.value, and remove the literal value from values.yaml so the
Secret is provided externally.
| # When enabled, portal accepts Authorization: Bearer <token> as the given DB user (same as UI login). | ||
| # TODO: load API_TOKEN from a Kubernetes Secret instead of plain values. | ||
| token: | ||
| enabled: true | ||
| user: system | ||
| value: litefunctionsxxxtoken |
There was a problem hiding this comment.
Do not ship API auth enabled with a known token.
These defaults enable bearer auth as system with a public token value. Any reachable default install would accept Authorization: Bearer litefunctionsxxxtoken.
🔒 Proposed safer defaults
# When enabled, portal accepts Authorization: Bearer <token> as the given DB user (same as UI login).
# TODO: load API_TOKEN from a Kubernetes Secret instead of plain values.
token:
- enabled: true
+ enabled: false
user: system
- value: litefunctionsxxxtoken
+ value: ""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # When enabled, portal accepts Authorization: Bearer <token> as the given DB user (same as UI login). | |
| # TODO: load API_TOKEN from a Kubernetes Secret instead of plain values. | |
| token: | |
| enabled: true | |
| user: system | |
| value: litefunctionsxxxtoken | |
| # When enabled, portal accepts Authorization: Bearer <token> as the given DB user (same as UI login). | |
| # TODO: load API_TOKEN from a Kubernetes Secret instead of plain values. | |
| token: | |
| enabled: false | |
| user: system | |
| value: "" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@chart/values.yaml` around lines 233 - 238, The chart currently enables API
bearer auth by default with a public token (token.enabled: true, token.user:
system, token.value: litefunctionsxxxtoken); change the defaults to safe values:
set token.enabled to false, clear token.value (empty string) and do not set a
default token.user, and add a comment instructing operators to provide a
secret-backed token via a Kubernetes Secret before enabling; update any docs
referencing token.value to require explicit secret configuration rather than
shipping a usable default.
| func authRequiredFailure(c *gin.Context) { | ||
| if strings.HasPrefix(c.Request.URL.Path, "/api/") { | ||
| c.JSON(http.StatusUnauthorized, gin.H{"error": "unauthorized"}) | ||
| } else { | ||
| c.Redirect(http.StatusFound, "/?redirect="+c.Request.URL.Path) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C2 'redirect=\+|Redirect\(http\.StatusFound' portalRepository: ashupednekar/litefunctions
Length of output: 915
🏁 Script executed:
head -20 portal/pkg/server/middleware/auth.goRepository: ashupednekar/litefunctions
Length of output: 440
🏁 Script executed:
sed -n '30,45n' portal/pkg/server/middleware/auth.goRepository: ashupednekar/litefunctions
Length of output: 52
🏁 Script executed:
sed -n '30,45p' portal/pkg/server/middleware/auth.goRepository: ashupednekar/litefunctions
Length of output: 564
🏁 Script executed:
rg -n "Redirect.*\+.*Request" portal/pkg/server/middleware/auth.goRepository: ashupednekar/litefunctions
Length of output: 137
🏁 Script executed:
cat -n portal/pkg/server/middleware/auth.go | head -50Repository: ashupednekar/litefunctions
Length of output: 1718
Escape the redirect query value.
Line 38 concatenates c.Request.URL.Path directly into the redirect query parameter. Paths containing special characters (e.g., ?, &, =) will produce malformed redirect URLs. Use url.QueryEscape() to properly encode the value.
Proposed fix
import (
"crypto/subtle"
"log"
"net/http"
+ "net/url"
"strings"
"github.com/ashupednekar/litefunctions/portal/internal/auth"
"github.com/ashupednekar/litefunctions/portal/pkg"
"github.com/gin-gonic/gin"
) func authRequiredFailure(c *gin.Context) {
if strings.HasPrefix(c.Request.URL.Path, "/api/") {
c.JSON(http.StatusUnauthorized, gin.H{"error": "unauthorized"})
} else {
- c.Redirect(http.StatusFound, "/?redirect="+c.Request.URL.Path)
+ c.Redirect(http.StatusFound, "/?redirect="+url.QueryEscape(c.Request.URL.Path))
}
c.Abort()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func authRequiredFailure(c *gin.Context) { | |
| if strings.HasPrefix(c.Request.URL.Path, "/api/") { | |
| c.JSON(http.StatusUnauthorized, gin.H{"error": "unauthorized"}) | |
| } else { | |
| c.Redirect(http.StatusFound, "/?redirect="+c.Request.URL.Path) | |
| } | |
| func authRequiredFailure(c *gin.Context) { | |
| if strings.HasPrefix(c.Request.URL.Path, "/api/") { | |
| c.JSON(http.StatusUnauthorized, gin.H{"error": "unauthorized"}) | |
| } else { | |
| c.Redirect(http.StatusFound, "/?redirect="+url.QueryEscape(c.Request.URL.Path)) | |
| } | |
| c.Abort() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@portal/pkg/server/middleware/auth.go` around lines 34 - 39, The redirect
value is concatenated raw in authRequiredFailure causing malformed URLs for
paths with special characters; change the redirect construction to use
url.QueryEscape(c.Request.URL.Path) so the path is properly URL-encoded and
update imports to include net/url if missing (ensure the redirect becomes
"/?redirect="+url.QueryEscape(c.Request.URL.Path) inside authRequiredFailure).
| if pkg.Cfg.APITokenEnabled && pkg.Cfg.APIToken != "" { | ||
| tok, ok := parseBearerToken(c.GetHeader("Authorization")) | ||
| if ok && constantTimeEqual(tok, pkg.Cfg.APIToken) { | ||
| userName := pkg.Cfg.APITokenUser | ||
| if userName == "" { | ||
| userName = "system" | ||
| } | ||
| if name, userID, err := store.GetUserByName(userName); err == nil { | ||
| c.Set("userID", userID) | ||
| c.Set("userName", name) | ||
| c.Set("authenticated", true) | ||
| } | ||
| c.Next() | ||
| return | ||
| } |
There was a problem hiding this comment.
Do not skip session fallback when token user lookup fails.
With a matching bearer token, OptionalAuthMiddleware calls Next() even if GetUserByName fails. That makes a valid session on the same request get ignored and silently treats the request as anonymous.
🐛 Proposed fix
if ok && constantTimeEqual(tok, pkg.Cfg.APIToken) {
userName := pkg.Cfg.APITokenUser
if userName == "" {
userName = "system"
}
if name, userID, err := store.GetUserByName(userName); err == nil {
c.Set("userID", userID)
c.Set("userName", name)
c.Set("authenticated", true)
+ c.Next()
+ return
+ } else {
+ log.Printf("[ERROR] API token user %q not found: %v", userName, err)
}
- c.Next()
- return
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if pkg.Cfg.APITokenEnabled && pkg.Cfg.APIToken != "" { | |
| tok, ok := parseBearerToken(c.GetHeader("Authorization")) | |
| if ok && constantTimeEqual(tok, pkg.Cfg.APIToken) { | |
| userName := pkg.Cfg.APITokenUser | |
| if userName == "" { | |
| userName = "system" | |
| } | |
| if name, userID, err := store.GetUserByName(userName); err == nil { | |
| c.Set("userID", userID) | |
| c.Set("userName", name) | |
| c.Set("authenticated", true) | |
| } | |
| c.Next() | |
| return | |
| } | |
| if pkg.Cfg.APITokenEnabled && pkg.Cfg.APIToken != "" { | |
| tok, ok := parseBearerToken(c.GetHeader("Authorization")) | |
| if ok && constantTimeEqual(tok, pkg.Cfg.APIToken) { | |
| userName := pkg.Cfg.APITokenUser | |
| if userName == "" { | |
| userName = "system" | |
| } | |
| if name, userID, err := store.GetUserByName(userName); err == nil { | |
| c.Set("userID", userID) | |
| c.Set("userName", name) | |
| c.Set("authenticated", true) | |
| c.Next() | |
| return | |
| } else { | |
| log.Printf("[ERROR] API token user %q not found: %v", userName, err) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@portal/pkg/server/middleware/auth.go` around lines 111 - 125, The current
logic prematurely returns after a matching bearer token even when
store.GetUserByName fails, which prevents session fallback; change the flow
inside the APIToken branch (parseBearerToken, constantTimeEqual) so that you
only call c.Next() and return when store.GetUserByName succeeds (err == nil) and
you set userID/userName/authenticated; if GetUserByName returns an error, do not
return—allow the middleware to continue so session-based auth can run as a
fallback.
Adds
Authorization: Bearersupport for programmatic access (same DB user asAPI_TOKEN_USER). Helm wiresAPI_TOKENfromui.token.value(TODO: use Kubernetes Secret).Made with Cursor
Summary by CodeRabbit
New Features
Configuration