Skip to content

Commit 3e99b8d

Browse files
torcolvinbbrks
andauthored
[4.0.0.1 backport] CBG-4943 obey db scope for CORS.LoginOrigin (#7834)
Co-authored-by: Ben Brooks <[email protected]>
1 parent 4924ade commit 3e99b8d

File tree

8 files changed

+145
-56
lines changed

8 files changed

+145
-56
lines changed

auth/cors.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ func (cors *CORSConfig) AddResponseHeaders(request *http.Request, response http.
3232
}
3333
}
3434

35+
// IsEmpty returns true if the CORS configuration is empty - used instead of a nil check since we always initialize the CORS config struct.
36+
func (cors *CORSConfig) IsEmpty() bool {
37+
return cors == nil ||
38+
(len(cors.Origin) == 0 && len(cors.LoginOrigin) == 0 && len(cors.Headers) == 0 && cors.MaxAge == 0)
39+
}
40+
3541
func MatchedOrigin(allowOrigins []string, rqOrigins []string) string {
3642
for _, rv := range rqOrigins {
3743
for _, av := range allowOrigins {

rest/cors_test.go

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
package rest
1010

1111
import (
12+
"fmt"
1213
"net/http"
1314
"strconv"
1415
"strings"
@@ -286,6 +287,24 @@ func TestCORSUserNoAccess(t *testing.T) {
286287
}
287288
}
288289

290+
// TestCORSResponseHeadersEmptyConfig ensures that an empty CORS config results in no CORS headers being set on the response.
291+
func TestCORSResponseHeadersEmptyConfig(t *testing.T) {
292+
rt := NewRestTester(t, nil)
293+
// RestTester initializes using defaultTestingCORSOrigin - override to empty for this test
294+
rt.ServerContext().Config.API.CORS = &auth.CORSConfig{}
295+
defer rt.Close()
296+
297+
reqHeaders := map[string]string{
298+
"Origin": "http://example.com",
299+
}
300+
response := rt.SendRequestWithHeaders(http.MethodGet, "/{{.db}}/", "", reqHeaders)
301+
RequireStatus(t, response, http.StatusUnauthorized)
302+
require.Contains(t, response.Body.String(), ErrLoginRequired.Message)
303+
assert.NotContains(t, response.Header(), "Access-Control-Allow-Origin")
304+
assert.NotContains(t, response.Header(), "Access-Control-Allow-Credentials")
305+
assert.NotContains(t, response.Header(), "Access-Control-Allow-Headers")
306+
}
307+
289308
func TestCORSOriginPerDatabase(t *testing.T) {
290309
// Override the default (example.com) CORS configuration in the DbConfig for /db:
291310
const perDBMaxAge = 1234
@@ -384,6 +403,74 @@ func TestCORSOriginPerDatabase(t *testing.T) {
384403
}
385404
}
386405

406+
func TestCORSLoginOriginPerDatabase(t *testing.T) {
407+
// Override the default (example.com) CORS configuration in the DbConfig for /db:
408+
rt := NewRestTesterPersistentConfigNoDB(t)
409+
defer rt.Close()
410+
dbConfig := rt.NewDbConfig()
411+
dbConfig.CORS = &auth.CORSConfig{
412+
Origin: []string{"http://couchbase.com", "http://staging.couchbase.com"},
413+
LoginOrigin: []string{"http://couchbase.com"},
414+
Headers: []string{},
415+
}
416+
RequireStatus(t, rt.CreateDatabase("dbloginorigin", dbConfig), http.StatusCreated)
417+
418+
const username = "alice"
419+
rt.CreateUser(username, nil)
420+
421+
testCases := []struct {
422+
name string
423+
origin string
424+
responseCode int
425+
responseErrorBody string
426+
}{
427+
{
428+
name: "CORS login origin allowed couchbase",
429+
origin: "http://couchbase.com",
430+
responseCode: http.StatusOK,
431+
},
432+
{
433+
name: "CORS login origin not allowed staging",
434+
origin: "http://staging.couchbase.com",
435+
responseCode: http.StatusBadRequest,
436+
responseErrorBody: "No CORS",
437+
},
438+
}
439+
for _, test := range testCases {
440+
rt.Run(test.name, func(t *testing.T) {
441+
reqHeaders := map[string]string{
442+
"Origin": test.origin,
443+
"Authorization": GetBasicAuthHeader(t, username, RestTesterDefaultUserPassword),
444+
}
445+
resp := rt.SendRequestWithHeaders(http.MethodPost, "/{{.db}}/_session", "", reqHeaders)
446+
RequireStatus(t, resp, test.responseCode)
447+
if test.responseErrorBody != "" {
448+
require.Contains(t, resp.Body.String(), test.responseErrorBody)
449+
// the access control headers are returned based on Origin and not LoginOrigin which could be considered a bug
450+
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
451+
} else {
452+
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
453+
}
454+
if test.responseCode == http.StatusOK {
455+
cookie, err := http.ParseSetCookie(resp.Header().Get("Set-Cookie"))
456+
require.NoError(t, err)
457+
require.NotEmpty(t, cookie.Path)
458+
reqHeaders["Cookie"] = fmt.Sprintf("%s=%s", cookie.Name, cookie.Value)
459+
}
460+
resp = rt.SendRequestWithHeaders(http.MethodDelete, "/{{.db}}/_session", "", reqHeaders)
461+
RequireStatus(t, resp, test.responseCode)
462+
if test.responseErrorBody != "" {
463+
require.Contains(t, resp.Body.String(), test.responseErrorBody)
464+
// the access control headers are returned based on Origin and not LoginOrigin which could be considered a bug
465+
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
466+
} else {
467+
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
468+
}
469+
470+
})
471+
}
472+
}
473+
387474
func TestCORSValidation(t *testing.T) {
388475
rt := NewRestTester(t, &RestTesterConfig{
389476
PersistentConfig: true,

rest/facebook.go

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"net/http"
1313
"net/url"
1414

15-
"github.com/couchbase/sync_gateway/auth"
1615
"github.com/couchbase/sync_gateway/base"
1716
)
1817

@@ -26,18 +25,14 @@ type FacebookResponse struct {
2625

2726
// POST /_facebook creates a facebook-based login session and sets its cookie.
2827
func (h *handler) handleFacebookPOST() error {
29-
// CORS not allowed for login #115 #762
30-
originHeader := h.rq.Header["Origin"]
31-
if len(originHeader) > 0 {
32-
matched := auth.MatchedOrigin(h.server.Config.API.CORS.LoginOrigin, originHeader)
33-
if matched == "" {
34-
return base.HTTPErrorf(http.StatusBadRequest, "No CORS")
35-
}
28+
err := h.checkLoginCORS()
29+
if err != nil {
30+
return err
3631
}
3732
var params struct {
3833
AccessToken string `json:"access_token"`
3934
}
40-
err := h.readJSONInto(&params)
35+
err = h.readJSONInto(&params)
4136
if err != nil {
4237
return err
4338
}

rest/google.go

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ package rest
1313
import (
1414
"net/http"
1515

16-
"github.com/couchbase/sync_gateway/auth"
1716
"github.com/couchbase/sync_gateway/base"
1817
)
1918

@@ -28,20 +27,16 @@ type GoogleResponse struct {
2827

2928
// POST /_google creates a google-based login session and sets its cookie.
3029
func (h *handler) handleGooglePOST() error {
31-
// CORS not allowed for login #115 #762
32-
originHeader := h.rq.Header["Origin"]
33-
if len(originHeader) > 0 {
34-
matched := auth.MatchedOrigin(h.server.Config.API.CORS.LoginOrigin, originHeader)
35-
if matched == "" {
36-
return base.HTTPErrorf(http.StatusBadRequest, "No CORS")
37-
}
30+
err := h.checkLoginCORS()
31+
if err != nil {
32+
return err
3833
}
3934

4035
var params struct {
4136
IDToken string `json:"id_token"`
4237
}
4338

44-
err := h.readJSONInto(&params)
39+
err = h.readJSONInto(&params)
4540
if err != nil {
4641
return err
4742
}

rest/handler.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -335,11 +335,8 @@ func (h *handler) validateAndWriteHeaders(method handlerMethod, accessPermission
335335
defer func() {
336336
// Now that we know the DB, add CORS headers to the response:
337337
if h.privs != adminPrivs && h.privs != metricsPrivs {
338-
cors := h.server.Config.API.CORS
339-
if h.db != nil {
340-
cors = h.db.CORS
341-
}
342-
if cors != nil {
338+
cors := h.getCORSConfig()
339+
if !cors.IsEmpty() {
343340
cors.AddResponseHeaders(h.rq, h.response)
344341
}
345342
}
@@ -1788,3 +1785,11 @@ func (h *handler) pathTemplateContains(pattern string) bool {
17881785
}
17891786
return strings.Contains(pathTemplate, pattern)
17901787
}
1788+
1789+
// getCORSConfig will return the CORS config for the handler's database if set, otherwise it will return the server CORS config
1790+
func (h *handler) getCORSConfig() *auth.CORSConfig {
1791+
if h.db != nil {
1792+
return h.db.CORS
1793+
}
1794+
return h.server.Config.API.CORS
1795+
}

rest/routing.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ func wrapRouter(sc *ServerContext, privs handlerPrivs, serverType serverType, ro
456456
cors = db.CORS
457457
}
458458
}
459-
if cors != nil && privs != adminPrivs && privs != metricsPrivs {
459+
if !cors.IsEmpty() && privs != adminPrivs && privs != metricsPrivs {
460460
cors.AddResponseHeaders(rq, response)
461461
}
462462
if len(options) == 0 {

rest/session_api.go

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,9 @@ func (h *handler) handleSessionGET() error {
3636

3737
// POST /_session creates a login session and sets its cookie
3838
func (h *handler) handleSessionPOST() error {
39-
// CORS not allowed for login #115 #762
40-
originHeader := h.rq.Header["Origin"]
41-
if len(originHeader) > 0 {
42-
matched := ""
43-
if h.server.Config.API.CORS != nil {
44-
matched = auth.MatchedOrigin(h.server.Config.API.CORS.LoginOrigin, originHeader)
45-
}
46-
if matched == "" {
47-
return base.HTTPErrorf(http.StatusBadRequest, "No CORS")
48-
}
39+
err := h.checkLoginCORS()
40+
if err != nil {
41+
return err
4942
}
5043

5144
// NOTE: handleSessionPOST doesn't handle creating users from OIDC - checkPublicAuth calls out into AuthenticateUntrustedJWT.
@@ -100,18 +93,10 @@ func (h *handler) getUserFromSessionRequestBody() (auth.User, error) {
10093

10194
// DELETE /_session logs out the current session
10295
func (h *handler) handleSessionDELETE() error {
103-
// CORS not allowed for login #115 #762
104-
originHeader := h.rq.Header["Origin"]
105-
if len(originHeader) > 0 {
106-
matched := ""
107-
if h.server.Config.API.CORS != nil {
108-
matched = auth.MatchedOrigin(h.server.Config.API.CORS.LoginOrigin, originHeader)
109-
}
110-
if matched == "" {
111-
return base.HTTPErrorf(http.StatusBadRequest, "No CORS")
112-
}
96+
err := h.checkLoginCORS()
97+
if err != nil {
98+
return err
11399
}
114-
115100
cookie := h.db.Authenticator(h.ctx()).DeleteSessionForCookie(h.ctx(), h.rq)
116101
if cookie == nil {
117102
return base.HTTPErrorf(http.StatusNotFound, "no session")
@@ -340,3 +325,16 @@ func (h *handler) formatSessionResponse(user auth.User) db.Body {
340325
return response
341326

342327
}
328+
329+
// checkLoginCORS validates the auth.CORSConfig.LoginOrigin section of CORS for requests.
330+
// Note: Validation of the general Origin header against auth.CORSConfig.Origin happens separately in validateAndWriteHeaders.
331+
func (h *handler) checkLoginCORS() error {
332+
originHeader := h.rq.Header["Origin"]
333+
if len(originHeader) > 0 {
334+
cors := h.getCORSConfig()
335+
if cors.IsEmpty() || auth.MatchedOrigin(cors.LoginOrigin, originHeader) == "" {
336+
return base.HTTPErrorf(http.StatusBadRequest, "No CORS")
337+
}
338+
}
339+
return nil
340+
}

rest/session_test.go

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,16 @@ func TestCORSLoginOriginOnSessionPost(t *testing.T) {
4141

4242
// #issue 991
4343
func TestCORSLoginOriginOnSessionPostNoCORSConfig(t *testing.T) {
44-
rt := NewRestTester(t, nil)
44+
rt := NewRestTesterPersistentConfigNoDB(t)
4545
defer rt.Close()
46+
// Set CORS to nil, RestTester initializes CORS by default and it is inherited when creating a DatabaseContext
47+
rt.ServerContext().Config.API.CORS = nil
4648

49+
RequireStatus(t, rt.CreateDatabase("db", rt.NewDbConfig()), http.StatusCreated)
4750
reqHeaders := map[string]string{
4851
"Origin": "http://example.com",
4952
}
5053

51-
// Set CORS to nil
52-
sc := rt.ServerContext()
53-
sc.Config.API.CORS = nil
54-
5554
response := rt.SendRequestWithHeaders("POST", "/db/_session", `{"name":"jchris","password":"secret"}`, reqHeaders)
5655
RequireStatus(t, response, 400)
5756
}
@@ -88,17 +87,21 @@ func TestCORSLogoutOriginOnSessionDelete(t *testing.T) {
8887
}
8988

9089
func TestCORSLogoutOriginOnSessionDeleteNoCORSConfig(t *testing.T) {
91-
rt := NewRestTester(t, &RestTesterConfig{GuestEnabled: true})
90+
rt := NewRestTesterPersistentConfigNoDB(t)
9291
defer rt.Close()
9392

93+
// Set CORS to nil, RestTester initializes CORS by default and it is inherited when creating a DatabaseContext
94+
rt.ServerContext().Config.API.CORS = nil
95+
96+
const username = "alice"
97+
RequireStatus(t, rt.CreateDatabase("db", rt.NewDbConfig()), http.StatusCreated)
98+
rt.CreateUser(username, nil)
99+
94100
reqHeaders := map[string]string{
95-
"Origin": "http://example.com",
101+
"Origin": "http://example.com",
102+
"Authorization": GetBasicAuthHeader(t, username, RestTesterDefaultUserPassword),
96103
}
97104

98-
// Set CORS to nil
99-
sc := rt.ServerContext()
100-
sc.Config.API.CORS = nil
101-
102105
response := rt.SendRequestWithHeaders("DELETE", "/db/_session", "", reqHeaders)
103106
RequireStatus(t, response, 400)
104107

0 commit comments

Comments
 (0)