Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit96f9e61

Browse files
authored
Strip CORS headers from applications (#8057)
The problem is that the headers get doubled up (not overwritten) andbrowsers do not like multiple values for the allowed origin even thoughit appears the spec allows for it.We could prefer the application's headers instead of ours but since wecontrol OPTIONS I think preferring ours will by the more consistentexperience and also aligns with the original RFC.
1 parent24b95e1 commit96f9e61

File tree

5 files changed

+107
-9
lines changed

5 files changed

+107
-9
lines changed

‎coderd/httpmw/cors.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,20 @@ import (
1010
"github.com/coder/coder/coderd/httpapi"
1111
)
1212

13+
const (
14+
// Server headers.
15+
AccessControlAllowOriginHeader="Access-Control-Allow-Origin"
16+
AccessControlAllowCredentialsHeader="Access-Control-Allow-Credentials"
17+
AccessControlAllowMethodsHeader="Access-Control-Allow-Methods"
18+
AccessControlAllowHeadersHeader="Access-Control-Allow-Headers"
19+
VaryHeader="Vary"
20+
21+
// Client headers.
22+
OriginHeader="Origin"
23+
AccessControlRequestMethodsHeader="Access-Control-Request-Methods"
24+
AccessControlRequestHeadersHeader="Access-Control-Request-Headers"
25+
)
26+
1327
//nolint:revive
1428
funcCors(allowAllbool,origins...string)func(next http.Handler) http.Handler {
1529
iflen(origins)==0 {

‎coderd/workspaceapps/apptest/apptest.go

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -928,7 +928,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
928928
forceURLTransport(t,client)
929929

930930
// Create workspace.
931-
port:=appServer(t)
931+
port:=appServer(t,nil)
932932
workspace,_=createWorkspaceWithApps(t,client,user.OrganizationIDs[0],user,port)
933933

934934
// Verify that the apps have the correct sharing levels set.
@@ -1260,4 +1260,61 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
12601260
})
12611261
}
12621262
})
1263+
1264+
t.Run("CORSHeadersStripped",func(t*testing.T) {
1265+
t.Parallel()
1266+
1267+
appDetails:=setupProxyTest(t,&DeploymentOptions{
1268+
headers: http.Header{
1269+
"X-Foobar": []string{"baz"},
1270+
"Access-Control-Allow-Origin": []string{"http://localhost"},
1271+
"access-control-allow-origin": []string{"http://localhost"},
1272+
"Access-Control-Allow-Credentials": []string{"true"},
1273+
"Access-Control-Allow-Methods": []string{"PUT"},
1274+
"Access-Control-Allow-Headers": []string{"X-Foobar"},
1275+
"Vary": []string{
1276+
"Origin",
1277+
"origin",
1278+
"Access-Control-Request-Headers",
1279+
"access-Control-request-Headers",
1280+
"Access-Control-Request-Methods",
1281+
"ACCESS-CONTROL-REQUEST-METHODS",
1282+
"X-Foobar",
1283+
},
1284+
},
1285+
})
1286+
1287+
appURL:=appDetails.SubdomainAppURL(appDetails.Apps.Owner)
1288+
1289+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
1290+
defercancel()
1291+
1292+
resp,err:=requestWithRetries(ctx,t,appDetails.AppClient(t),http.MethodGet,appURL.String(),nil)
1293+
require.NoError(t,err)
1294+
deferresp.Body.Close()
1295+
1296+
require.Equal(t,http.StatusOK,resp.StatusCode)
1297+
require.Equal(t, []string(nil),resp.Header.Values("Access-Control-Allow-Origin"))
1298+
require.Equal(t, []string(nil),resp.Header.Values("Access-Control-Allow-Credentials"))
1299+
require.Equal(t, []string(nil),resp.Header.Values("Access-Control-Allow-Methods"))
1300+
require.Equal(t, []string(nil),resp.Header.Values("Access-Control-Allow-Headers"))
1301+
// Somehow there are two "Origin"s in Vary even though there should only be
1302+
// one (from the CORS middleware), even if you remove the headers being sent
1303+
// above. When I do nothing else but change the expected value below to
1304+
// have two "Origin"s suddenly Vary only has one. It is somehow always the
1305+
// opposite of whatever I put for the expected. So, reluctantly, remove
1306+
// duplicate "Origin" values.
1307+
vardeduped []string
1308+
varaddedOriginbool
1309+
for_,value:=rangeresp.Header.Values("Vary") {
1310+
ifvalue!="Origin"||!addedOrigin {
1311+
ifvalue=="Origin" {
1312+
addedOrigin=true
1313+
}
1314+
deduped=append(deduped,value)
1315+
}
1316+
}
1317+
require.Equal(t, []string{"Origin","X-Foobar"},deduped)
1318+
require.Equal(t, []string{"baz"},resp.Header.Values("X-Foobar"))
1319+
})
12631320
}

‎coderd/workspaceapps/apptest/setup.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ type DeploymentOptions struct {
5252
// The following fields are only used by setupProxyTestWithFactory.
5353
noWorkspacebool
5454
portuint16
55+
headers http.Header
5556
}
5657

5758
// Deployment is a license-agnostic deployment with all the fields that apps
@@ -184,7 +185,7 @@ func setupProxyTestWithFactory(t *testing.T, factory DeploymentFactory, opts *De
184185
}
185186

186187
ifopts.port==0 {
187-
opts.port=appServer(t)
188+
opts.port=appServer(t,opts.headers)
188189
}
189190
workspace,agnt:=createWorkspaceWithApps(t,deployment.SDKClient,deployment.FirstUser.OrganizationID,me,opts.port)
190191

@@ -233,7 +234,7 @@ func setupProxyTestWithFactory(t *testing.T, factory DeploymentFactory, opts *De
233234
returndetails
234235
}
235236

236-
funcappServer(t*testing.T)uint16 {
237+
funcappServer(t*testing.T,headers http.Header)uint16 {
237238
// Start a listener on a random port greater than the minimum app port.
238239
var (
239240
ln net.Listener
@@ -261,6 +262,11 @@ func appServer(t *testing.T) uint16 {
261262
_,err:=r.Cookie(codersdk.SessionTokenCookie)
262263
assert.ErrorIs(t,err,http.ErrNoCookie)
263264
w.Header().Set("X-Forwarded-For",r.Header.Get("X-Forwarded-For"))
265+
forname,values:=rangeheaders {
266+
for_,value:=rangevalues {
267+
w.Header().Add(name,value)
268+
}
269+
}
264270
w.WriteHeader(http.StatusOK)
265271
_,_=w.Write([]byte(proxyTestAppBody))
266272
}),

‎coderd/workspaceapps/proxy.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/coder/coder/coderd/httpapi"
2323
"github.com/coder/coder/coderd/httpmw"
2424
"github.com/coder/coder/coderd/tracing"
25+
"github.com/coder/coder/coderd/util/slice"
2526
"github.com/coder/coder/coderd/wsconncache"
2627
"github.com/coder/coder/codersdk"
2728
"github.com/coder/coder/site"
@@ -541,6 +542,26 @@ func (s *Server) proxyWorkspaceApp(rw http.ResponseWriter, r *http.Request, appT
541542
deferrelease()
542543
proxy.Transport=conn.HTTPTransport()
543544

545+
proxy.ModifyResponse=func(r*http.Response)error {
546+
r.Header.Del(httpmw.AccessControlAllowOriginHeader)
547+
r.Header.Del(httpmw.AccessControlAllowCredentialsHeader)
548+
r.Header.Del(httpmw.AccessControlAllowMethodsHeader)
549+
r.Header.Del(httpmw.AccessControlAllowHeadersHeader)
550+
varies:=r.Header.Values(httpmw.VaryHeader)
551+
r.Header.Del(httpmw.VaryHeader)
552+
forbiddenVary:= []string{
553+
httpmw.OriginHeader,
554+
httpmw.AccessControlRequestMethodsHeader,
555+
httpmw.AccessControlRequestHeadersHeader,
556+
}
557+
for_,value:=rangevaries {
558+
if!slice.ContainsCompare(forbiddenVary,value,strings.EqualFold) {
559+
r.Header.Add(httpmw.VaryHeader,value)
560+
}
561+
}
562+
returnnil
563+
}
564+
544565
// This strips the session token from a workspace app request.
545566
cookieHeaders:=r.Header.Values("Cookie")[:]
546567
r.Header.Del("Cookie")

‎docs/networking/port-forwarding.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,12 @@ will echo whatever the request sends.
124124

125125
These cross-origin headers are not configurable by administrative settings.
126126

127-
Applications can settheir ownheaderswhich willoverride the defaults but this
128-
will only apply to non-preflight requests. Preflight requests throughthe
129-
dashboard are never sent to applications and thus cannot be modified by
130-
them. Read more about the difference between simple requests and requests that
131-
trigger preflights
132-
[here](https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#simple_requests).
127+
If applications setany of the aboveheadersthey willbe stripped from the
128+
response except for`Vary` headers that are set to a value other thanthe ones
129+
listed above.
130+
131+
In other words, CORS behavior through the dashboard is not currently
132+
configurable by either admins or users.
133133

134134
####Allowed by default
135135

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp