Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions chart/templates/portal.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ spec:
value: {{ .Values.ui.vcs.baseUrl | quote }}
- name: VCS_PUBLIC_BASE_URL
value: {{ .Values.ui.vcs.publicBaseUrl | quote }}
{{- 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 }}
Comment on lines +49 to +56

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C3 'API_TOKEN|ui\.token|secretKeyRef' chart

Repository: 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: token

Remove 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.

Suggested change
{{- 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.

resources: {}
lifecycle:
postStart:
Expand Down
6 changes: 6 additions & 0 deletions chart/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -230,3 +230,9 @@ ui:
enabled: false
host: localhost
issuer: letsencrypt
# 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
Comment on lines +233 to +238

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

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.

Suggested change
# 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.

8 changes: 8 additions & 0 deletions portal/internal/auth/adaptors/selectors.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ func convertDBCredentialToWebauthn(c Credential) webauthn.Credential {
}
}

func (db *WebauthnStore) GetUserByName(userName string) (string, []byte, error) {
u, err := db.GetUser(userName)
if err != nil {
return "", nil, err
}
return u.Name, u.ID, nil
}

func (db *WebauthnStore) GetUser(userName string) (*auth.User, error) {
u, err := db.queries.GetUserByName(context.Background(), userName)
if err != nil {
Expand Down
3 changes: 3 additions & 0 deletions portal/internal/auth/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ type PasskeyStore interface {

// User authentication session methods
SessionStore

// GetUserByName returns the canonical name and WebAuthn user ID for API token auth.
GetUserByName(userName string) (name string, id []byte, err error)
}

func NewWebauthn() (*webauthn.WebAuthn, error) {
Expand Down
6 changes: 6 additions & 0 deletions portal/pkg/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ type Settings struct {
VcsPublicBaseUrl string `env:"VCS_PUBLIC_BASE_URL"`
OperatorUrl string `env:"OPERATOR_URL" default:"litefunctions-operator:50051"`
IngestorUrl string `env:"INGESTOR_URL" default:"http://litefunctions-ingestor:3000"`

// API_TOKEN_ENABLED: when true, Authorization: Bearer <API_TOKEN> authenticates
// as user API_TOKEN_USER (must exist in DB), bypassing session cookies.
APITokenEnabled bool `env:"API_TOKEN_ENABLED" default:"false"`
APIToken string `env:"API_TOKEN"`
APITokenUser string `env:"API_TOKEN_USER" default:"system"`
}

var (
Expand Down
105 changes: 92 additions & 13 deletions portal/pkg/server/middleware/auth.go
Original file line number Diff line number Diff line change
@@ -1,39 +1,102 @@
package middleware

import (
"crypto/subtle"
"log"
"net/http"
"strings"

"github.com/ashupednekar/litefunctions/portal/internal/auth"
"github.com/ashupednekar/litefunctions/portal/pkg"
"github.com/gin-gonic/gin"
)

func AuthMiddleware(sessionStore auth.SessionStore) gin.HandlerFunc {
func constantTimeEqual(a, b string) bool {
if len(a) != len(b) {
return false
}
return subtle.ConstantTimeCompare([]byte(a), []byte(b)) == 1
}

func parseBearerToken(h string) (string, bool) {
h = strings.TrimSpace(h)
const prefix = "Bearer "
if len(h) < len(prefix) || !strings.EqualFold(h[:len(prefix)], prefix) {
return "", false
}
tok := strings.TrimSpace(h[len(prefix):])
if tok == "" {
return "", false
}
return tok, true
}

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)
}
Comment on lines +34 to +39

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C2 'redirect=\+|Redirect\(http\.StatusFound' portal

Repository: ashupednekar/litefunctions

Length of output: 915


🏁 Script executed:

head -20 portal/pkg/server/middleware/auth.go

Repository: ashupednekar/litefunctions

Length of output: 440


🏁 Script executed:

sed -n '30,45n' portal/pkg/server/middleware/auth.go

Repository: ashupednekar/litefunctions

Length of output: 52


🏁 Script executed:

sed -n '30,45p' portal/pkg/server/middleware/auth.go

Repository: ashupednekar/litefunctions

Length of output: 564


🏁 Script executed:

rg -n "Redirect.*\+.*Request" portal/pkg/server/middleware/auth.go

Repository: ashupednekar/litefunctions

Length of output: 137


🏁 Script executed:

cat -n portal/pkg/server/middleware/auth.go | head -50

Repository: 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.

Suggested change
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).

c.Abort()
}

// tryAPIBearerAuth handles Authorization: Bearer when API_TOKEN_ENABLED is set.
// Returns true if the request was fully handled (Next or Abort).
func tryAPIBearerAuth(c *gin.Context, store auth.PasskeyStore) bool {
if !pkg.Cfg.APITokenEnabled || pkg.Cfg.APIToken == "" {
return false
}
tok, hasBearer := parseBearerToken(c.GetHeader("Authorization"))
if !hasBearer {
return false
}
if !constantTimeEqual(tok, pkg.Cfg.APIToken) {
log.Printf("[WARN] invalid API token attempt for %s", c.Request.URL.Path)
c.JSON(http.StatusUnauthorized, gin.H{"error": "unauthorized"})
c.Abort()
return true
}
userName := pkg.Cfg.APITokenUser
if userName == "" {
userName = "system"
}
name, userID, err := store.GetUserByName(userName)
if err != nil {
log.Printf("[ERROR] API token user %q not found: %v", userName, err)
c.JSON(http.StatusUnauthorized, gin.H{"error": "api token user not found"})
c.Abort()
return true
}
c.Set("userID", userID)
c.Set("userName", name)
c.Next()
return true
}

func AuthMiddleware(store auth.PasskeyStore) gin.HandlerFunc {
return func(c *gin.Context) {
if tryAPIBearerAuth(c, store) {
return
}

sessionID, err := c.Cookie(auth.SessionCookieName)
if err != nil {
log.Printf("[DEBUG] No session cookie found: %v", err)

c.Redirect(http.StatusFound, "/?redirect="+c.Request.URL.Path)
c.Abort()
authRequiredFailure(c)
return
}

userName, userID, found, err := sessionStore.GetUserSession(sessionID)
userName, userID, found, err := store.GetUserSession(sessionID)
if err != nil {
log.Printf("[ERROR] Error retrieving session: %v", err)
c.Redirect(http.StatusFound, "/?redirect="+c.Request.URL.Path)
c.Abort()
authRequiredFailure(c)
return
}

if !found {
log.Printf("[DEBUG] Session not found or expired")

c.SetCookie(auth.SessionCookieName, "", -1, "/", "", false, true)
c.Redirect(http.StatusFound, "/?redirect="+c.Request.URL.Path)
c.Abort()
authRequiredFailure(c)
return
}

Expand All @@ -43,14 +106,30 @@ func AuthMiddleware(sessionStore auth.SessionStore) gin.HandlerFunc {
}
}

func OptionalAuthMiddleware(sessionStore auth.SessionStore) gin.HandlerFunc {
func OptionalAuthMiddleware(store auth.PasskeyStore) gin.HandlerFunc {
return func(c *gin.Context) {
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
}
Comment on lines +111 to +125

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
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.

}
sessionID, err := c.Cookie(auth.SessionCookieName)
if err == nil {
userName, userID, found, err := sessionStore.GetUserSession(sessionID)
userName, userID, found, err := store.GetUserSession(sessionID)
if err == nil && found {
c.Set("userID", userID)
c.Set("userName", userName)
c.Set("userName", userName)
c.Set("authenticated", true)
}
}
Expand Down