Skip to content

Commit 6f6316b

Browse files
kevinmcconnellkevinmcconnell
authored
Merge pull request #178 from basecamp/cookie-path-prefix
Add `--scope-cookie-paths`
2 parents 34c5a80 + 7ef4b52 commit 6f6316b

File tree

5 files changed

+353
-4
lines changed

5 files changed

+353
-4
lines changed

‎internal/cmd/deploy.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ func newDeployCommand() *deployCommand {
6161

6262
deployCommand.cmd.Flags().StringSliceVar(&deployCommand.args.TargetOptions.LogRequestHeaders, "log-request-header", nil, "Additional request header to log (may be specified multiple times)")
6363
deployCommand.cmd.Flags().StringSliceVar(&deployCommand.args.TargetOptions.LogResponseHeaders, "log-response-header", nil, "Additional response header to log (may be specified multiple times)")
64-
6564
deployCommand.cmd.Flags().BoolVar(&deployCommand.args.TargetOptions.ForwardHeaders, "forward-headers", false, "Forward X-Forwarded headers to target (default false if TLS enabled; otherwise true)")
65+
deployCommand.cmd.Flags().BoolVar(&deployCommand.args.TargetOptions.ScopeCookiePaths, "scope-cookie-paths", false, "Scope cookie paths to match path prefix")
6666

6767
deployCommand.cmd.MarkFlagRequired("target")
6868
deployCommand.cmd.MarkFlagsRequiredTogether("tls-certificate-path", "tls-private-key-path")

‎internal/server/cookie_scope.go‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package server
2+
3+
import (
4+
"net"
5+
"net/http"
6+
"net/url"
7+
"strings"
8+
)
9+
10+
// CookieScope handles scoping Set-Cookie paths to a path prefix.
11+
type CookieScope struct {
12+
pathPrefix string
13+
host string
14+
}
15+
16+
func NewCookieScope(pathPrefix string, host string) *CookieScope {
17+
if h, _, err := net.SplitHostPort(host); err == nil {
18+
host = h
19+
}
20+
21+
return &CookieScope{
22+
pathPrefix: pathPrefix,
23+
host: host,
24+
}
25+
}
26+
27+
func (cs *CookieScope) ApplyToHeader(header http.Header) {
28+
cookies := header["Set-Cookie"]
29+
for i, v := range cookies {
30+
cookie, err := http.ParseSetCookie(v)
31+
if err != nil || !cs.domainMatches(cookie.Domain) {
32+
continue
33+
}
34+
35+
cookie.Path, err = url.JoinPath(cs.pathPrefix, strings.Trim(cookie.Path, "/"))
36+
if err == nil {
37+
cookies[i] = cookie.String()
38+
}
39+
}
40+
}
41+
42+
func (cs *CookieScope) domainMatches(cookieDomain string) bool {
43+
return cookieDomain == "" || cookieDomain == cs.host
44+
}

‎internal/server/cookie_scope_test.go‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
package server
2+
3+
import (
4+
"net/http"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
)
9+
10+
func TestCookieScope_ApplyToHeader(t *testing.T) {
11+
tests := []struct {
12+
name string
13+
pathPrefix string
14+
host string
15+
inputCookies []string
16+
expectedPaths []string
17+
}{
18+
{
19+
name: "scopes cookie with root path",
20+
pathPrefix: "/api",
21+
host: "example.com",
22+
inputCookies: []string{"session=abc; Path=/"},
23+
expectedPaths: []string{"/api"},
24+
},
25+
{
26+
name: "scopes cookie with subpath",
27+
pathPrefix: "/api",
28+
host: "example.com",
29+
inputCookies: []string{"session=abc; Path=/admin"},
30+
expectedPaths: []string{"/api/admin"},
31+
},
32+
{
33+
name: "scopes cookie without path",
34+
pathPrefix: "/api",
35+
host: "example.com",
36+
inputCookies: []string{"session=abc"},
37+
expectedPaths: []string{"/api"},
38+
},
39+
{
40+
name: "scopes first-party cookie with matching domain",
41+
pathPrefix: "/api",
42+
host: "example.com",
43+
inputCookies: []string{"session=abc; Path=/; Domain=example.com"},
44+
expectedPaths: []string{"/api"},
45+
},
46+
{
47+
name: "does not scope third-party cookie",
48+
pathPrefix: "/api",
49+
host: "example.com",
50+
inputCookies: []string{"tracking=xyz; Path=/; Domain=other.com"},
51+
expectedPaths: []string{"/"},
52+
},
53+
{
54+
name: "handles multiple cookies",
55+
pathPrefix: "/app",
56+
host: "example.com",
57+
inputCookies: []string{"a=1; Path=/", "b=2; Path=/foo", "c=3; Path=/; Domain=third.com"},
58+
expectedPaths: []string{"/app", "/app/foo", "/"},
59+
},
60+
{
61+
name: "handles host with port",
62+
pathPrefix: "/api",
63+
host: "example.com:8080",
64+
inputCookies: []string{"session=abc; Path=/; Domain=example.com"},
65+
expectedPaths: []string{"/api"},
66+
},
67+
}
68+
69+
for _, tt := range tests {
70+
t.Run(tt.name, func(t *testing.T) {
71+
cs := NewCookieScope(tt.pathPrefix, tt.host)
72+
header := http.Header{}
73+
header["Set-Cookie"] = tt.inputCookies
74+
75+
cs.ApplyToHeader(header)
76+
77+
cookies := header["Set-Cookie"]
78+
assert.Len(t, cookies, len(tt.expectedPaths))
79+
80+
for i, cookieStr := range cookies {
81+
cookie, err := http.ParseSetCookie(cookieStr)
82+
assert.NoError(t, err)
83+
assert.Equal(t, tt.expectedPaths[i], cookie.Path, "cookie %d path mismatch", i)
84+
}
85+
})
86+
}
87+
}
88+
89+
func TestCookieScope_DomainMatching(t *testing.T) {
90+
tests := []struct {
91+
name string
92+
host string
93+
cookieDomain string
94+
shouldScope bool
95+
}{
96+
{
97+
name: "empty domain matches",
98+
host: "example.com",
99+
cookieDomain: "",
100+
shouldScope: true,
101+
},
102+
{
103+
name: "exact domain match",
104+
host: "example.com",
105+
cookieDomain: "example.com",
106+
shouldScope: true,
107+
},
108+
{
109+
name: "different domain does not match",
110+
host: "example.com",
111+
cookieDomain: "other.com",
112+
shouldScope: false,
113+
},
114+
{
115+
name: "subdomain does not match parent",
116+
host: "example.com",
117+
cookieDomain: "sub.example.com",
118+
shouldScope: false,
119+
},
120+
{
121+
name: "parent does not match subdomain host",
122+
host: "sub.example.com",
123+
cookieDomain: "example.com",
124+
shouldScope: false,
125+
},
126+
{
127+
name: "host with port matches domain",
128+
host: "example.com:8080",
129+
cookieDomain: "example.com",
130+
shouldScope: true,
131+
},
132+
}
133+
134+
for _, tt := range tests {
135+
t.Run(tt.name, func(t *testing.T) {
136+
cs := NewCookieScope("/api", tt.host)
137+
138+
header := http.Header{}
139+
if tt.cookieDomain == "" {
140+
header["Set-Cookie"] = []string{"test=value; Path=/original"}
141+
} else {
142+
header["Set-Cookie"] = []string{"test=value; Path=/original; Domain=" + tt.cookieDomain}
143+
}
144+
145+
cs.ApplyToHeader(header)
146+
147+
cookie, err := http.ParseSetCookie(header["Set-Cookie"][0])
148+
assert.NoError(t, err)
149+
150+
if tt.shouldScope {
151+
assert.Equal(t, "/api/original", cookie.Path)
152+
} else {
153+
assert.Equal(t, "/original", cookie.Path)
154+
}
155+
})
156+
}
157+
}

‎internal/server/router_test.go‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,109 @@ func TestRouter_RoutingMultipleHosts(t *testing.T) {
415415
assert.Equal(t, "second", body)
416416
}
417417

418+
func TestRouter_PathBasedRoutingCookiePrefixPrefix(t *testing.T) {
419+
checkRequest := func(scopeCookiePaths bool, path string, expectedCookiePath string) {
420+
router := testRouter(t)
421+
_, backend1 := testBackendWithHandler(t, func(w http.ResponseWriter, r *http.Request) {
422+
http.SetCookie(w, &http.Cookie{
423+
Name: "session",
424+
Value: "secret",
425+
Path: "/something",
426+
HttpOnly: true,
427+
})
428+
w.WriteHeader(http.StatusOK)
429+
})
430+
_, backend2 := testBackendWithHandler(t, func(w http.ResponseWriter, r *http.Request) {
431+
http.SetCookie(w, &http.Cookie{
432+
Name: "session",
433+
Value: "secret",
434+
Path: "/",
435+
HttpOnly: true,
436+
})
437+
w.WriteHeader(http.StatusOK)
438+
})
439+
440+
serviceOptions := defaultServiceOptions
441+
serviceOptions.StripPrefix = true
442+
targetOptions := defaultTargetOptions
443+
targetOptions.ScopeCookiePaths = scopeCookiePaths
444+
445+
serviceOptions.PathPrefixes = []string{"/api", "/app"}
446+
require.NoError(t, router.DeployService("service1", []string{backend1}, defaultEmptyReaders, serviceOptions, targetOptions, DefaultDeployTimeout, DefaultDrainTimeout))
447+
serviceOptions.PathPrefixes = []string{"/chat"}
448+
require.NoError(t, router.DeployService("service2", []string{backend2}, defaultEmptyReaders, serviceOptions, targetOptions, DefaultDeployTimeout, DefaultDrainTimeout))
449+
450+
req := httptest.NewRequest(http.MethodGet, path, nil)
451+
w := httptest.NewRecorder()
452+
router.ServeHTTP(w, req)
453+
require.Equal(t, http.StatusOK, w.Result().StatusCode)
454+
require.Len(t, w.Result().Cookies(), 1)
455+
456+
cookie := w.Result().Cookies()[0]
457+
assert.Equal(t, expectedCookiePath, cookie.Path)
458+
}
459+
460+
checkRequest(true, "/app", "/app/something")
461+
checkRequest(true, "/api", "/api/something")
462+
checkRequest(false, "/app", "/something")
463+
checkRequest(false, "/api", "/something")
464+
465+
checkRequest(true, "/chat", "/chat")
466+
checkRequest(false, "/chat", "/")
467+
}
468+
469+
func TestRouter_PathBasedRoutingCookiePrefixThirdPartyDomain(t *testing.T) {
470+
router := testRouter(t)
471+
_, backend := testBackendWithHandler(t, func(w http.ResponseWriter, r *http.Request) {
472+
http.SetCookie(w, &http.Cookie{
473+
Name: "first_party",
474+
Value: "value1",
475+
Path: "/original",
476+
Domain: "example.com",
477+
})
478+
http.SetCookie(w, &http.Cookie{
479+
Name: "third_party",
480+
Value: "value2",
481+
Path: "/original",
482+
Domain: "other.com",
483+
})
484+
http.SetCookie(w, &http.Cookie{
485+
Name: "no_domain",
486+
Value: "value3",
487+
Path: "/original",
488+
})
489+
w.WriteHeader(http.StatusOK)
490+
})
491+
492+
serviceOptions := defaultServiceOptions
493+
serviceOptions.StripPrefix = true
494+
serviceOptions.PathPrefixes = []string{"/api"}
495+
targetOptions := defaultTargetOptions
496+
targetOptions.ScopeCookiePaths = true
497+
498+
require.NoError(t, router.DeployService("service", []string{backend}, defaultEmptyReaders, serviceOptions, targetOptions, DefaultDeployTimeout, DefaultDrainTimeout))
499+
500+
req := httptest.NewRequest(http.MethodGet, "http://example.com/api/test", nil)
501+
w := httptest.NewRecorder()
502+
router.ServeHTTP(w, req)
503+
require.Equal(t, http.StatusOK, w.Result().StatusCode)
504+
require.Len(t, w.Result().Cookies(), 3)
505+
506+
cookies := w.Result().Cookies()
507+
508+
// First-party cookie (domain matches request host) should be scoped
509+
assert.Equal(t, "first_party", cookies[0].Name)
510+
assert.Equal(t, "/api/original", cookies[0].Path)
511+
512+
// Third-party cookie (domain doesn't match) should NOT be scoped
513+
assert.Equal(t, "third_party", cookies[1].Name)
514+
assert.Equal(t, "/original", cookies[1].Path)
515+
516+
// Cookie without domain should be scoped
517+
assert.Equal(t, "no_domain", cookies[2].Name)
518+
assert.Equal(t, "/api/original", cookies[2].Path)
519+
}
520+
418521
func TestRouter_PathBasedRoutingStripPrefix(t *testing.T) {
419522
router := testRouter(t)
420523
_, backend := testBackendWithHandler(t, func(w http.ResponseWriter, r *http.Request) {

0 commit comments

Comments
 (0)