Skip to content

Commit

Permalink
GH-141: Removed flawed same-origin support in favor of CORS
Browse files Browse the repository at this point in the history
  • Loading branch information
jirenius committed Mar 13, 2020
1 parent 54e7731 commit f546373
Show file tree
Hide file tree
Showing 8 changed files with 11 additions and 58 deletions.
2 changes: 1 addition & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ Server Options:
--tlskey <file> Private key for HTTP server certificate
--apiencoding <type> Encoding for web resources: json, jsonflat (default: json)
--creds <file> NATS User Credentials file
--alloworigin <origin> Allowed origin(s) for CORS: *, same-origin, <origin> (default: *)
--alloworigin <origin> Allowed origin(s): *, or <scheme>://<hostname>[:<port>] (default: *)
-c, --config <file> Configuration file
Logging Options:
Expand Down
21 changes: 0 additions & 21 deletions server/apiHandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,24 +33,6 @@ func (s *Service) setCommonHeaders(w http.ResponseWriter, r *http.Request) error
switch s.cfg.allowOrigin[0] {
case "*":
w.Header().Set("Access-Control-Allow-Origin", "*")
case "same-origin":
// Same-Origin-Policy is the default for HTTP and requires no additional
// headers. But to force the browser to stick to the Same-Origin-Policy,
// we require the request to contain a Content-Type header matching that
// of the API Encoding.
typ := r.Header["Content-Type"]
if len(typ) > 0 && typ[0] != "" {
for _, v := range strings.Split(typ[0], ",") {
t, _, err := mime.ParseMediaType(v)
if err != nil {
break
}
if t == s.mimetype {
return nil
}
}
}
return reserr.ErrUnsupportedMediaType

default:
// CORS validation
Expand All @@ -74,7 +56,6 @@ func (s *Service) setCommonHeaders(w http.ResponseWriter, r *http.Request) error

func (s *Service) apiHandler(w http.ResponseWriter, r *http.Request) {
err := s.setCommonHeaders(w, r)

if r.Method == "OPTIONS" {
w.Header().Set("Access-Control-Allow-Methods", s.cfg.allowMethods)
return
Expand Down Expand Up @@ -228,8 +209,6 @@ func httpError(w http.ResponseWriter, err error, enc APIEncoder) {
code = http.StatusServiceUnavailable
case reserr.CodeForbidden:
code = http.StatusForbidden
case reserr.CodeUnsupportedMediaType:
code = http.StatusUnsupportedMediaType
default:
code = http.StatusBadRequest
}
Expand Down
4 changes: 2 additions & 2 deletions server/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (c *Config) prepare() error {
if c.AllowOrigin != nil {
c.allowOrigin = strings.Split(*c.AllowOrigin, ";")
if err := validateAllowOrigin(c.allowOrigin); err != nil {
return fmt.Errorf("invalid allowOrigin setting (%s)\n\t%s\n\tvalid options are *, same-origin, or a list of semi-colon separated origins", *c.AllowOrigin, err)
return fmt.Errorf("invalid allowOrigin setting (%s)\n\t%s\n\tvalid options are *, or a list of semi-colon separated origins", *c.AllowOrigin, err)
}
sort.Strings(c.allowOrigin)
} else {
Expand All @@ -135,7 +135,7 @@ func validateAllowOrigin(s []string) error {
for i, o := range s {
o = toLowerASCII(o)
s[i] = o
if o == "same-origin" || o == "*" {
if o == "*" {
if len(s) > 1 {
return fmt.Errorf("'%s' must not be used together with other origin settings", o)
}
Expand Down
2 changes: 0 additions & 2 deletions server/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ func TestConfigPrepare(t *testing.T) {
invalidAddr := "127.0.0"
invalidHeaderAuth := "test"
allowOriginAll := "*"
allowOriginSame := "same-origin"
allowOriginSingle := "http://resgate.io"
allowOriginMultiple := "http://localhost;http://resgate.io"
allowOriginInvalidEmpty := ""
Expand All @@ -50,7 +49,6 @@ func TestConfigPrepare(t *testing.T) {
{Config{Addr: &localAddr, WSPath: "/"}, Config{Addr: &localAddr, Port: 80, WSPath: "/", APIPath: "/", scheme: "http", netAddr: "127.0.0.1:80", allowOrigin: []string{"*"}, allowMethods: "GET, POST, OPTIONS"}, false},
{Config{Addr: &ipv6Addr, WSPath: "/"}, Config{Addr: &ipv6Addr, Port: 80, WSPath: "/", APIPath: "/", scheme: "http", netAddr: "[::1]:80", allowOrigin: []string{"*"}, allowMethods: "GET, POST, OPTIONS"}, false},
{Config{AllowOrigin: &allowOriginAll, WSPath: "/"}, Config{Addr: nil, Port: 80, WSPath: "/", APIPath: "/", scheme: "http", netAddr: "0.0.0.0:80", allowOrigin: []string{"*"}, allowMethods: "GET, POST, OPTIONS"}, false},
{Config{AllowOrigin: &allowOriginSame, WSPath: "/"}, Config{Addr: nil, Port: 80, WSPath: "/", APIPath: "/", scheme: "http", netAddr: "0.0.0.0:80", allowOrigin: []string{"same-origin"}, allowMethods: "GET, POST, OPTIONS"}, false},
{Config{AllowOrigin: &allowOriginSingle, WSPath: "/"}, Config{Addr: nil, Port: 80, WSPath: "/", APIPath: "/", scheme: "http", netAddr: "0.0.0.0:80", allowOrigin: []string{"http://resgate.io"}, allowMethods: "GET, POST, OPTIONS"}, false},
{Config{AllowOrigin: &allowOriginMultiple, WSPath: "/"}, Config{Addr: nil, Port: 80, WSPath: "/", APIPath: "/", scheme: "http", netAddr: "0.0.0.0:80", allowOrigin: []string{"http://localhost", "http://resgate.io"}, allowMethods: "GET, POST, OPTIONS"}, false},

Expand Down
18 changes: 8 additions & 10 deletions server/reserr/reserr.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,10 @@ const (
CodeInvalidRequest = "system.invalidRequest"
CodeUnsupportedProtocol = "system.unsupportedProtocol"
// HTTP only error codes
CodeBadRequest = "system.badRequest"
CodeMethodNotAllowed = "system.methodNotAllowed"
CodeServiceUnavailable = "system.serviceUnavailable"
CodeForbidden = "system.forbidden"
CodeUnsupportedMediaType = "system.unsupportedMediaType"
CodeBadRequest = "system.badRequest"
CodeMethodNotAllowed = "system.methodNotAllowed"
CodeServiceUnavailable = "system.serviceUnavailable"
CodeForbidden = "system.forbidden"
)

// Pre-defined RES errors
Expand All @@ -70,9 +69,8 @@ var (
ErrInvalidRequest = &Error{Code: CodeInvalidRequest, Message: "Invalid request"}
ErrUnsupportedProtocol = &Error{Code: CodeUnsupportedProtocol, Message: "Unsupported protocol"}
// HTTP only errors
ErrBadRequest = &Error{Code: CodeBadRequest, Message: "Bad request"}
ErrMethodNotAllowed = &Error{Code: CodeMethodNotAllowed, Message: "Method not allowed"}
ErrServiceUnavailable = &Error{Code: CodeServiceUnavailable, Message: "Service unavailable"}
ErrForbiddenOrigin = &Error{Code: CodeForbidden, Message: "Forbidden origin"}
ErrUnsupportedMediaType = &Error{Code: CodeUnsupportedMediaType, Message: "Unsupported media type"}
ErrBadRequest = &Error{Code: CodeBadRequest, Message: "Bad request"}
ErrMethodNotAllowed = &Error{Code: CodeMethodNotAllowed, Message: "Method not allowed"}
ErrServiceUnavailable = &Error{Code: CodeServiceUnavailable, Message: "Service unavailable"}
ErrForbiddenOrigin = &Error{Code: CodeForbidden, Message: "Forbidden origin"}
)
10 changes: 0 additions & 10 deletions test/14http_get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,20 +239,10 @@ func TestHTTPGet_AllowOrigin_ExpectedResponse(t *testing.T) {
{"http://localhost", "", "*", http.StatusOK, map[string]string{"Access-Control-Allow-Origin": "*"}, []string{"Vary"}, successResponse},
{"http://localhost", "", "http://localhost", http.StatusOK, map[string]string{"Access-Control-Allow-Origin": "http://localhost", "Vary": "Origin"}, nil, successResponse},
{"https://resgate.io", "", "http://localhost;https://resgate.io", http.StatusOK, map[string]string{"Access-Control-Allow-Origin": "https://resgate.io", "Vary": "Origin"}, nil, successResponse},
{"http://example.com", "application/json", "same-origin", http.StatusOK, nil, []string{"Access-Control-Allow-Origin", "Vary"}, successResponse},
{"http://example.com", "application/json; charset=utf-8; ", "same-origin", http.StatusOK, nil, []string{"Access-Control-Allow-Origin", "Vary"}, successResponse},
{"http://example.com", "text/plain,application/json; charset=utf-8; ", "same-origin", http.StatusOK, nil, []string{"Access-Control-Allow-Origin", "Vary"}, successResponse},
// Invalid requests
{"http://example.com", "", "http://localhost;https://resgate.io", http.StatusForbidden, map[string]string{"Access-Control-Allow-Origin": "http://localhost", "Vary": "Origin"}, nil, reserr.ErrForbiddenOrigin},
{"http://example.com", "", "same-origin", http.StatusUnsupportedMediaType, nil, []string{"Access-Control-Allow-Origin", "Vary"}, reserr.ErrUnsupportedMediaType},
{"", "", "same-origin", http.StatusUnsupportedMediaType, nil, []string{"Access-Control-Allow-Origin", "Vary"}, reserr.ErrUnsupportedMediaType},
{"http://example.com", "text/plain", "same-origin", http.StatusUnsupportedMediaType, nil, []string{"Access-Control-Allow-Origin", "Vary"}, reserr.ErrUnsupportedMediaType},
{"http://example.com", "text/plain,multipart/form-data", "same-origin", http.StatusUnsupportedMediaType, nil, []string{"Access-Control-Allow-Origin", "Vary"}, reserr.ErrUnsupportedMediaType},
// No Origin header in request
{"", "", "*", http.StatusOK, map[string]string{"Access-Control-Allow-Origin": "*"}, []string{"Vary"}, successResponse},
{"", "application/json", "same-origin", http.StatusOK, nil, []string{"Access-Control-Allow-Origin", "Vary"}, successResponse},
{"", "application/json; charset=utf-8; ", "same-origin", http.StatusOK, nil, []string{"Access-Control-Allow-Origin", "Vary"}, successResponse},
{"", "text/plain,application/json; charset=utf-8; ", "same-origin", http.StatusOK, nil, []string{"Access-Control-Allow-Origin", "Vary"}, successResponse},
{"", "", "http://localhost", http.StatusOK, nil, []string{"Access-Control-Allow-Origin", "Vary"}, successResponse},
}

Expand Down
10 changes: 0 additions & 10 deletions test/15http_post_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,20 +271,10 @@ func TestHTTPPost_AllowOrigin_ExpectedResponse(t *testing.T) {
{"http://localhost", "", "*", http.StatusOK, map[string]string{"Access-Control-Allow-Origin": "*"}, []string{"Vary"}, successResponse},
{"http://localhost", "", "http://localhost", http.StatusOK, map[string]string{"Access-Control-Allow-Origin": "http://localhost", "Vary": "Origin"}, nil, successResponse},
{"https://resgate.io", "", "http://localhost;https://resgate.io", http.StatusOK, map[string]string{"Access-Control-Allow-Origin": "https://resgate.io", "Vary": "Origin"}, nil, successResponse},
{"http://example.com", "application/json", "same-origin", http.StatusOK, nil, []string{"Access-Control-Allow-Origin", "Vary"}, successResponse},
{"http://example.com", "application/json; charset=utf-8; ", "same-origin", http.StatusOK, nil, []string{"Access-Control-Allow-Origin", "Vary"}, successResponse},
{"http://example.com", "text/plain,application/json; charset=utf-8; ", "same-origin", http.StatusOK, nil, []string{"Access-Control-Allow-Origin", "Vary"}, successResponse},
// Invalid requests
{"http://example.com", "", "http://localhost;https://resgate.io", http.StatusForbidden, map[string]string{"Access-Control-Allow-Origin": "http://localhost", "Vary": "Origin"}, nil, reserr.ErrForbiddenOrigin},
{"http://example.com", "", "same-origin", http.StatusUnsupportedMediaType, nil, []string{"Access-Control-Allow-Origin", "Vary"}, reserr.ErrUnsupportedMediaType},
{"", "", "same-origin", http.StatusUnsupportedMediaType, nil, []string{"Access-Control-Allow-Origin", "Vary"}, reserr.ErrUnsupportedMediaType},
{"http://example.com", "text/plain", "same-origin", http.StatusUnsupportedMediaType, nil, []string{"Access-Control-Allow-Origin", "Vary"}, reserr.ErrUnsupportedMediaType},
{"http://example.com", "text/plain,multipart/form-data", "same-origin", http.StatusUnsupportedMediaType, nil, []string{"Access-Control-Allow-Origin", "Vary"}, reserr.ErrUnsupportedMediaType},
// No Origin header in request
{"", "", "*", http.StatusOK, map[string]string{"Access-Control-Allow-Origin": "*"}, []string{"Vary"}, successResponse},
{"", "application/json", "same-origin", http.StatusOK, nil, []string{"Access-Control-Allow-Origin", "Vary"}, successResponse},
{"", "application/json; charset=utf-8; ", "same-origin", http.StatusOK, nil, []string{"Access-Control-Allow-Origin", "Vary"}, successResponse},
{"", "text/plain,application/json; charset=utf-8; ", "same-origin", http.StatusOK, nil, []string{"Access-Control-Allow-Origin", "Vary"}, successResponse},
{"", "", "http://localhost", http.StatusOK, nil, []string{"Access-Control-Allow-Origin", "Vary"}, successResponse},
}

Expand Down
2 changes: 0 additions & 2 deletions test/21http_options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ func TestHTTPOptions_AllowOrigin_ExpectedResponseHeaders(t *testing.T) {
{"http://localhost", "http://localhost", map[string]string{"Access-Control-Allow-Origin": "http://localhost", "Vary": "Origin"}, nil},
{"https://resgate.io", "http://localhost;https://resgate.io", map[string]string{"Access-Control-Allow-Origin": "https://resgate.io", "Vary": "Origin"}, nil},
{"http://example.com", "http://localhost;https://resgate.io", map[string]string{"Access-Control-Allow-Origin": "http://localhost", "Vary": "Origin"}, nil},
{"http://example.com", "same-origin", nil, []string{"Access-Control-Allow-Origin", "Vary"}},
// No Origin header in request
{"", "*", map[string]string{"Access-Control-Allow-Origin": "*"}, []string{"Vary"}},
{"", "same-origin", nil, []string{"Access-Control-Allow-Origin", "Vary"}},
{"", "http://localhost", nil, []string{"Access-Control-Allow-Origin", "Vary"}},
}

Expand Down

0 comments on commit f546373

Please sign in to comment.