dennis/homeassistant-stelloauth-addon
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
docs/english-ui-and-user-docs
homeassistant-stelloauth-addon/stelloauth/patches/stelloauth-security.patch
T
Dennis Juhler Aagaard f3c1abf2cf
CI / validate (pull_request) Failing after 1m33s
Details
fix: resolve final addon review
2026-09-24 19:02:05 +02:00

553 lines
19 KiB
Diff
Raw Permalink Blame History

diff --git a/internal/app/oauth.go b/internal/app/oauth.go
index f0b9548..0c59a11 100644
--- a/internal/app/oauth.go
+++ b/internal/app/oauth.go
@@ -162,12 +162,11 @@ func performChromedpOAuth(
reqURL := e.Request.URL
// Capture OAuth redirect
if strings.HasPrefix(reqURL, redirectPrefix) {
- log.Printf("[%s] Redirect URL: %s", requestID, reqURL)
+ logCapturedOAuthRedirect(requestID, reqURL)
parsed, err := url.Parse(reqURL)
if err == nil {
if code := parsed.Query().Get("code"); code != "" {
oauthCode = code
- log.Printf("[%s] Captured OAuth code from redirect request", requestID)
}
}
} else if strings.Contains(reqURL, "OPErrorPage.php") {
@@ -175,7 +174,7 @@ func performChromedpOAuth(
// contextId when the login took too long).
if parsed, perr := url.Parse(reqURL); perr == nil {
flowError = friendlyOPError(parsed.Query().Get("code"), parsed.Query().Get("message"))
- log.Printf("[%s] Stellantis error page: %s", requestID, reqURL)
+ logStellantisErrorPage(requestID, reqURL)
}
} else if debug != nil && isRelevantURL(reqURL) {
// Only show relevant OAuth flow URLs in debug output
@@ -452,7 +451,7 @@ func friendlyOPError(code, message string) string {
func codeFromLocation(browserCtx context.Context, redirectPrefix string) string {
var currentURL string
_ = chromedp.Run(browserCtx, chromedp.Location(&currentURL))
- log.Printf("Current URL: %s", currentURL)
+ logCurrentBrowserLocation(currentURL)
if !strings.HasPrefix(currentURL, redirectPrefix) {
return ""
}
@@ -462,3 +461,15 @@ func codeFromLocation(browserCtx context.Context, redirectPrefix string) string
}
return parsed.Query().Get("code")
}
+
+func logCapturedOAuthRedirect(requestID, _ string) {
+ log.Printf("[%s] Captured OAuth redirect", requestID)
+}
+
+func logStellantisErrorPage(requestID, _ string) {
+ log.Printf("[%s] Stellantis error page received", requestID)
+}
+
+func logCurrentBrowserLocation(_ string) {
+ log.Printf("Current browser location checked")
+}
diff --git a/internal/app/oauth_test.go b/internal/app/oauth_test.go
index 3589572..b9ad90d 100644
--- a/internal/app/oauth_test.go
+++ b/internal/app/oauth_test.go
@@ -6,6 +6,32 @@ import (
"testing"
)
+func TestOAuthEventLogsDoNotContainURLsCodesOrTokens(t *testing.T) {
+ cookie := "event-cookie"
+ accessToken := "event-access-token"
+ refreshToken := "event-refresh-token"
+ redirectURL := "mypeugeot://oauth2redirect/dk?code=event-oauth-code&access_token=" + accessToken
+ errorURL := "https://idpcvs.peugeot.com/am/oauth2/OPErrorPage.php?cookie=" + cookie
+ currentURL := redirectURL + "&refresh_token=" + refreshToken
+ oauthCode := "event-oauth-code"
+
+ logs := captureLogs(t, func() {
+ logCapturedOAuthRedirect("redirect-request-id", redirectURL)
+ logStellantisErrorPage("error-request-id", errorURL)
+ logCurrentBrowserLocation(currentURL)
+ })
+ want := "[redirect-request-id] Captured OAuth redirect\n" +
+ "[error-request-id] Stellantis error page received\n" +
+ "Current browser location checked\n"
+ if logs != want {
+ t.Fatalf("logs = %q, want %q", logs, want)
+ }
+ assertLogRedacted(t, logs,
+ cookie, accessToken, refreshToken,
+ redirectURL, errorURL, currentURL, oauthCode,
+ )
+}
+
func TestFriendlyOPError(t *testing.T) {
cases := []struct {
name string
diff --git a/internal/app/server.go b/internal/app/server.go
index 48a487e..946d8cf 100644
--- a/internal/app/server.go
+++ b/internal/app/server.go
@@ -121,6 +121,13 @@ func getClientIP(r *http.Request) string {
return r.RemoteAddr
}
+func remoteClientIP(r *http.Request) string {
+ if host, _, err := net.SplitHostPort(r.RemoteAddr); err == nil {
+ return host
+ }
+ return strings.TrimSpace(r.RemoteAddr)
+}
+
// refundIfExpired gives back the rate-limit charge when the OAuth attempt
// failed with a transient "session expired" error (bounded by the limiter).
func refundIfExpired(clientIP, requestID string, err error) {
@@ -129,6 +136,14 @@ func refundIfExpired(clientIP, requestID string, err error) {
}
}
+func logOAuthRequest(requestID, clientIP string, req OAuthRequest) {
+ log.Printf("[%s] OAuth request from %s (%s/%s)", requestID, clientIP, req.Brand, req.Country)
+}
+
+func logOAuthFailure(requestID string, _ error) {
+ log.Printf("[%s] OAuth failed", requestID)
+}
+
func handleOAuth(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
sendError(w, "Method not allowed", http.StatusMethodNotAllowed)
@@ -136,7 +151,7 @@ func handleOAuth(w http.ResponseWriter, r *http.Request) {
}
// Get client IP early for rate limiting
- clientIP := getClientIP(r)
+ clientIP := remoteClientIP(r)
// Check rate limit
if !rateLimiter.isAllowed(clientIP) {
@@ -160,7 +175,7 @@ func handleOAuth(w http.ResponseWriter, r *http.Request) {
// Generate request ID
requestID := uuid.New().String()
- log.Printf("[%s] OAuth request from %s for user %s (%s/%s)", requestID, clientIP, req.Email, req.Brand, req.Country)
+ logOAuthRequest(requestID, clientIP, req)
// Check if client accepts SSE
if r.Header.Get("Accept") == "text/event-stream" {
@@ -171,7 +186,7 @@ func handleOAuth(w http.ResponseWriter, r *http.Request) {
code, err := performOAuth(req, requestID, nil, nil)
if err != nil {
refundIfExpired(clientIP, requestID, err)
- log.Printf("[%s] OAuth failed: %s", requestID, err.Error())
+ logOAuthFailure(requestID, err)
sendError(w, err.Error(), http.StatusBadRequest)
return
}
@@ -206,7 +221,7 @@ func handleOAuthSSE(w http.ResponseWriter, req OAuthRequest, requestID, clientIP
code, err := performOAuth(req, requestID, progress, debug)
if err != nil {
refundIfExpired(clientIP, requestID, err)
- log.Printf("[%s] OAuth failed: %s", requestID, err.Error())
+ logOAuthFailure(requestID, err)
_, _ = fmt.Fprintf(w, "data: {\"type\":\"error\",\"message\":\"%s\"}\n\n", err.Error())
flusher.Flush()
return
diff --git a/internal/app/server_test.go b/internal/app/server_test.go
index b56bb27..730e658 100644
--- a/internal/app/server_test.go
+++ b/internal/app/server_test.go
@@ -1,12 +1,16 @@
package app
import (
+ "bytes"
"encoding/json"
+ "errors"
+ "log"
"net/http"
"net/http/httptest"
"os"
"strings"
"testing"
+ "time"
)
func TestMain(m *testing.M) {
@@ -101,6 +105,47 @@ func TestHandleOAuth_InvalidBody(t *testing.T) {
}
}
+func useSingleRequestRateLimiter(t *testing.T) {
+ t.Helper()
+ previous := rateLimiter
+ rateLimiter = &RateLimiter{
+ requests: make(map[string][]time.Time),
+ refunds: make(map[string][]time.Time),
+ limit: 1,
+ window: time.Hour,
+ enabled: true,
+ }
+ t.Cleanup(func() { rateLimiter = previous })
+}
+
+func setSpoofedClientHeaders(req *http.Request, suffix string) {
+ req.Header.Set("Forwarded", "for=203.0.113."+suffix)
+ req.Header.Set("X-Forwarded-For", "198.51.100."+suffix)
+ req.Header.Set("X-Real-IP", "192.0.2."+suffix)
+}
+
+func TestHandleOAuthRateLimitUsesDirectPeer(t *testing.T) {
+ useSingleRequestRateLimiter(t)
+
+ first := httptest.NewRequest(http.MethodPost, "/oauth", strings.NewReader("invalid json"))
+ first.RemoteAddr = "10.0.0.8:41001"
+ setSpoofedClientHeaders(first, "11")
+ firstResponse := httptest.NewRecorder()
+ handleOAuth(firstResponse, first)
+ if firstResponse.Code != http.StatusBadRequest {
+ t.Fatalf("first status = %d, want %d", firstResponse.Code, http.StatusBadRequest)
+ }
+
+ second := httptest.NewRequest(http.MethodPost, "/oauth", strings.NewReader("invalid json"))
+ second.RemoteAddr = "10.0.0.8:41001"
+ setSpoofedClientHeaders(second, "22")
+ secondResponse := httptest.NewRecorder()
+ handleOAuth(secondResponse, second)
+ if secondResponse.Code != http.StatusTooManyRequests {
+ t.Fatalf("second status = %d, want %d", secondResponse.Code, http.StatusTooManyRequests)
+ }
+}
+
func TestHandleOAuth_MissingFields(t *testing.T) {
body := `{"brand":"MyPeugeot","country":"","email":"","password":""}`
req := httptest.NewRequest(http.MethodPost, "/oauth", strings.NewReader(body))
@@ -188,3 +233,74 @@ func TestParseClientIP(t *testing.T) {
}
}
}
+
+func TestRemoteClientIPIgnoresForwardedHeaders(t *testing.T) {
+ req := httptest.NewRequest(http.MethodPost, "/oauth", nil)
+ req.RemoteAddr = "192.0.2.10:1234"
+ req.Header.Set("Forwarded", "for=203.0.113.1")
+ req.Header.Set("X-Forwarded-For", "203.0.113.2")
+ req.Header.Set("X-Real-IP", "203.0.113.3")
+
+ if got := remoteClientIP(req); got != "192.0.2.10" {
+ t.Errorf("remoteClientIP() = %q, want %q", got, "192.0.2.10")
+ }
+}
+
+func captureLogs(t *testing.T, fn func()) string {
+ t.Helper()
+ var logs bytes.Buffer
+ originalWriter := log.Writer()
+ originalFlags := log.Flags()
+ log.SetOutput(&logs)
+ log.SetFlags(0)
+ defer func() {
+ log.SetOutput(originalWriter)
+ log.SetFlags(originalFlags)
+ }()
+
+ fn()
+ return logs.String()
+}
+
+func assertLogRedacted(t *testing.T, logs string, sentinels ...string) {
+ t.Helper()
+ for _, sentinel := range sentinels {
+ if strings.Contains(logs, sentinel) {
+ t.Errorf("logs contain sensitive value %q: %s", sentinel, logs)
+ }
+ }
+}
+
+func TestOAuthRequestAndFailureLogsDoNotContainSensitiveValues(t *testing.T) {
+ email := "sentinel-email@example.invalid"
+ password := "sentinel-password"
+ cookie := "sentinel-cookie"
+ accessToken := "sentinel-access-token"
+ refreshToken := "sentinel-refresh-token"
+ authorizeURL := "https://idpcvs.peugeot.com/am/oauth2/authorize?sentinel=authorize-url"
+ redirectURL := "mypeugeot://oauth2redirect/dk?sentinel=redirect-url"
+ oauthCode := "sentinel-oauth-code"
+ req := OAuthRequest{
+ Brand: "MyPeugeot",
+ Country: "DK",
+ Email: email,
+ Password: password,
+ }
+ failure := errors.New(strings.Join([]string{
+ cookie, accessToken, refreshToken, authorizeURL, redirectURL, oauthCode,
+ }, "|"))
+
+ logs := captureLogs(t, func() {
+ logOAuthRequest("request-id", "192.0.2.10", req)
+ logOAuthFailure("request-id", failure)
+ })
+ want := "[request-id] OAuth request from 192.0.2.10 (MyPeugeot/DK)\n" +
+ "[request-id] OAuth failed\n"
+ if logs != want {
+ t.Fatalf("logs = %q, want %q", logs, want)
+ }
+ assertLogRedacted(t, logs,
+ email, password, cookie, accessToken, refreshToken,
+ authorizeURL, redirectURL, oauthCode,
+ )
+}
diff --git a/internal/app/worker.go b/internal/app/worker.go
index 00b9cf8..3dbe54a 100644
--- a/internal/app/worker.go
+++ b/internal/app/worker.go
@@ -7,9 +7,20 @@ import (
"log"
"net/http"
"net/url"
+ "strings"
"uuid"
)
+const maxWorkerRequestBytes = 64 << 10
+
+var approvedWorkerHosts = map[string]struct{}{
+ "idpcvs.citroen.com": {},
+ "idpcvs.driveds.com": {},
+ "idpcvs.opel.com": {},
+ "idpcvs.peugeot.com": {},
+ "idpcvs.vauxhall.co.uk": {},
+}
+
// workerRequest is the worker-v2-compatible OAuth request: the caller supplies a
// fully-built authorize URL instead of brand/country. This mirrors the API of
// github.com/andreadegiovine/homeassistant-stellantis-vehicles-worker-v2 so the
@@ -40,15 +51,21 @@ func handleWorker(w http.ResponseWriter, r *http.Request) {
return
}
- clientIP := getClientIP(r)
+ clientIP := remoteClientIP(r)
if !rateLimiter.isAllowed(clientIP) {
log.Printf("Rate limit exceeded for %s (worker)", clientIP)
sendWorkerError(w, "Rate limit exceeded. Try again later.", http.StatusTooManyRequests)
return
}
+ r.Body = http.MaxBytesReader(w, r.Body, maxWorkerRequestBytes)
var req workerRequest
if err := json.UnmarshalRead(r.Body, &req); err != nil {
+ var maxBytesErr *http.MaxBytesError
+ if errors.As(err, &maxBytesErr) {
+ sendWorkerError(w, "Request body too large", http.StatusRequestEntityTooLarge)
+ return
+ }
sendWorkerError(w, "Invalid request body", http.StatusBadRequest)
return
}
@@ -58,6 +75,11 @@ func handleWorker(w http.ResponseWriter, r *http.Request) {
return
}
+ if err := validateWorkerURL(req.URL); err != nil {
+ sendWorkerError(w, err.Error(), http.StatusBadRequest)
+ return
+ }
+
scheme, err := redirectScheme(req.URL)
if err != nil {
sendWorkerError(w, err.Error(), http.StatusBadRequest)
@@ -65,12 +87,12 @@ func handleWorker(w http.ResponseWriter, r *http.Request) {
}
requestID := uuid.New().String()
- log.Printf("[%s] worker OAuth request from %s for user %s", requestID, clientIP, req.Email)
+ logWorkerOAuthRequest(requestID, clientIP, req)
code, err := performChromedpOAuth(req.URL, req.Email, req.Password, scheme, requestID, nil, nil)
if err != nil {
refundIfExpired(clientIP, requestID, err)
- log.Printf("[%s] worker OAuth failed: %s", requestID, err.Error())
+ logWorkerOAuthFailure(requestID, err)
sendWorkerError(w, err.Error(), http.StatusBadRequest)
return
}
@@ -80,6 +102,31 @@ func handleWorker(w http.ResponseWriter, r *http.Request) {
_ = json.MarshalWrite(w, workerCode{Code: code})
}
+func validateWorkerURL(rawURL string) error {
+ parsed, err := url.Parse(rawURL)
+ if err != nil {
+ return errors.New("invalid url")
+ }
+ if parsed.Scheme != "https" || parsed.User != nil || parsed.Port() != "" || parsed.Fragment != "" {
+ return errors.New("invalid url")
+ }
+ if _, ok := approvedWorkerHosts[strings.ToLower(parsed.Hostname())]; !ok {
+ return errors.New("invalid url")
+ }
+ if parsed.EscapedPath() != "/am/oauth2/authorize" {
+ return errors.New("invalid url")
+ }
+ return nil
+}
+
+func logWorkerOAuthRequest(requestID, clientIP string, _ workerRequest) {
+ log.Printf("[%s] worker OAuth request from %s", requestID, clientIP)
+}
+
+func logWorkerOAuthFailure(requestID string, _ error) {
+ log.Printf("[%s] worker OAuth failed", requestID)
+}
+
// redirectScheme extracts the custom redirect scheme (e.g. "mymap") from the
// authorize URL's redirect_uri query parameter; the code-capture listener keys on
// "<scheme>://".
diff --git a/internal/app/worker_test.go b/internal/app/worker_test.go
index e64964d..7141640 100644
--- a/internal/app/worker_test.go
+++ b/internal/app/worker_test.go
@@ -2,12 +2,81 @@ package app
import (
"encoding/json"
+ "errors"
"net/http"
"net/http/httptest"
"strings"
"testing"
)
+func TestWorkerRequestAndFailureLogsDoNotContainSensitiveValues(t *testing.T) {
+ email := "worker-email@example.invalid"
+ password := "worker-password"
+ cookie := "worker-cookie"
+ accessToken := "worker-access-token"
+ refreshToken := "worker-refresh-token"
+ authorizeURL := "https://idpcvs.peugeot.com/am/oauth2/authorize?sentinel=worker-authorize-url"
+ redirectURL := "mypeugeot://oauth2redirect/dk?sentinel=worker-redirect-url"
+ oauthCode := "worker-oauth-code"
+ req := workerRequest{
+ URL: authorizeURL,
+ Email: email,
+ Password: password,
+ }
+ failure := errors.New(strings.Join([]string{
+ cookie, accessToken, refreshToken, redirectURL, oauthCode,
+ }, "|"))
+
+ logs := captureLogs(t, func() {
+ logWorkerOAuthRequest("worker-request-id", "192.0.2.20", req)
+ logWorkerOAuthFailure("worker-request-id", failure)
+ })
+ want := "[worker-request-id] worker OAuth request from 192.0.2.20\n" +
+ "[worker-request-id] worker OAuth failed\n"
+ if logs != want {
+ t.Fatalf("logs = %q, want %q", logs, want)
+ }
+ assertLogRedacted(t, logs,
+ email, password, cookie, accessToken, refreshToken,
+ authorizeURL, redirectURL, oauthCode,
+ )
+}
+
+func TestValidateWorkerURL(t *testing.T) {
+ allowed := []string{
+ "https://idpcvs.citroen.com/am/oauth2/authorize?redirect_uri=mycitroen%3A%2F%2Foauth2redirect%2Fdk",
+ "https://idpcvs.driveds.com/am/oauth2/authorize?redirect_uri=mymap%3A%2F%2Foauth2redirect%2Fdk",
+ "https://idpcvs.opel.com/am/oauth2/authorize?redirect_uri=myopel%3A%2F%2Foauth2redirect%2Fdk",
+ "https://idpcvs.peugeot.com/am/oauth2/authorize?redirect_uri=mypeugeot%3A%2F%2Foauth2redirect%2Fdk",
+ "https://idpcvs.vauxhall.co.uk/am/oauth2/authorize?redirect_uri=myvauxhall%3A%2F%2Foauth2redirect%2Fgb",
+ "https://IDPCVS.OPEL.COM/am/oauth2/authorize?redirect_uri=myopel%3A%2F%2Foauth2redirect%2Fdk",
+ }
+ for _, rawURL := range allowed {
+ t.Run(rawURL, func(t *testing.T) {
+ if err := validateWorkerURL(rawURL); err != nil {
+ t.Fatalf("validateWorkerURL() error = %v", err)
+ }
+ })
+ }
+
+ rejected := map[string]string{
+ "http": "http://idpcvs.peugeot.com/am/oauth2/authorize?redirect_uri=mypeugeot%3A%2F%2Foauth2redirect%2Fdk",
+ "userinfo": "https://user@idpcvs.peugeot.com/am/oauth2/authorize?redirect_uri=mypeugeot%3A%2F%2Foauth2redirect%2Fdk",
+ "explicit port": "https://idpcvs.peugeot.com:443/am/oauth2/authorize?redirect_uri=mypeugeot%3A%2F%2Foauth2redirect%2Fdk",
+ "evil suffix": "https://idpcvs.peugeot.com.evil.example/am/oauth2/authorize?redirect_uri=mypeugeot%3A%2F%2Foauth2redirect%2Fdk",
+ "extra path component": "https://idpcvs.peugeot.com/am/oauth2/authorize/extra?redirect_uri=mypeugeot%3A%2F%2Foauth2redirect%2Fdk",
+ "escaped path": "https://idpcvs.peugeot.com/am/oauth2%2Fauthorize?redirect_uri=mypeugeot%3A%2F%2Foauth2redirect%2Fdk",
+ "fragment": "https://idpcvs.peugeot.com/am/oauth2/authorize?redirect_uri=mypeugeot%3A%2F%2Foauth2redirect%2Fdk#fragment",
+ }
+ for name, rawURL := range rejected {
+ t.Run(name, func(t *testing.T) {
+ if err := validateWorkerURL(rawURL); err == nil {
+ t.Fatal("validateWorkerURL() error = nil, want rejection")
+ }
+ })
+ }
+}
+
func TestHandleWorker_MethodNotAllowed(t *testing.T) {
req := httptest.NewRequest(http.MethodGet, "/worker", nil)
w := httptest.NewRecorder()
@@ -30,6 +99,47 @@ func TestHandleWorker_InvalidBody(t *testing.T) {
}
}
+func TestHandleWorkerRateLimitUsesDirectPeer(t *testing.T) {
+ useSingleRequestRateLimiter(t)
+
+ first := httptest.NewRequest(http.MethodPost, "/worker", strings.NewReader("invalid json"))
+ first.RemoteAddr = "10.0.0.9:41002"
+ setSpoofedClientHeaders(first, "33")
+ firstResponse := httptest.NewRecorder()
+ handleWorker(firstResponse, first)
+ if firstResponse.Code != http.StatusBadRequest {
+ t.Fatalf("first status = %d, want %d", firstResponse.Code, http.StatusBadRequest)
+ }
+
+ second := httptest.NewRequest(http.MethodPost, "/worker", strings.NewReader("invalid json"))
+ second.RemoteAddr = "10.0.0.9:41002"
+ setSpoofedClientHeaders(second, "44")
+ secondResponse := httptest.NewRecorder()
+ handleWorker(secondResponse, second)
+ if secondResponse.Code != http.StatusTooManyRequests {
+ t.Fatalf("second status = %d, want %d", secondResponse.Code, http.StatusTooManyRequests)
+ }
+}
+
+func TestHandleWorker_RequestTooLarge(t *testing.T) {
+ body := `{"url":"` + strings.Repeat("x", 65<<10) + `"}`
+ req := httptest.NewRequest(http.MethodPost, "/worker", strings.NewReader(body))
+ w := httptest.NewRecorder()
+
+ handleWorker(w, req)
+
+ if w.Code != http.StatusRequestEntityTooLarge {
+ t.Fatalf("status = %d, want %d", w.Code, http.StatusRequestEntityTooLarge)
+ }
+ var resp workerError
+ if err := json.NewDecoder(w.Body).Decode(&resp); err != nil {
+ t.Fatalf("decode response: %v", err)
+ }
+ if resp.Message != "Request body too large" {
+ t.Errorf("message = %q, want %q", resp.Message, "Request body too large")
+ }
+}
+
func TestHandleWorker_MissingParams(t *testing.T) {
body := `{"url":"","email":"","password":""}`
req := httptest.NewRequest(http.MethodPost, "/worker", strings.NewReader(body))
@@ -83,7 +193,7 @@ func TestRedirectScheme(t *testing.T) {
},
{
name: "opel custom scheme",
- authURL: "https://example.com/authorize?redirect_uri=myopel%3A%2F%2Foauth2redirect%2Fde",
+ authURL: "https://idpcvs.opel.com/am/oauth2/authorize?redirect_uri=myopel%3A%2F%2Foauth2redirect%2Fde",
want: "myopel",
},
{
Reference in New Issue View Git Blame Copy Permalink