authorize: enforce https for all redirect endpoints except localhost

CONTRIBUTING.md

@@ -95,7 +95,7 @@ For legal reasons, no anonymous or pseudonymous contributions are accepted ([con
 To make a pull request, you will need a GitHub account; if you are unclear on this process, see GitHub's documentation on [forking](https://help.github.com/articles/fork-a-repo) and [pull requests](https://help.github.com/articles/using-pull-requests). Pull requests should be targeted at the `master` branch. Before creating a pull request, go through this checklist:
 
 1. Create a feature branch off of `master` so that changes do not get mixed up.
-1. [Rebase](http://git-scm.com/book/en/Git-Branching-Rebasing) your local changes against the `master` branch.
+1. [Rebase](https://git-scm.com/book/en/Git-Branching-Rebasing) your local changes against the `master` branch.
 1. Run the full project test suite with the `go test ./...` (or equivalent) command and confirm that it passes.
 1. Run `gofmt -s` (if the project is written in Go).
 1. Accept the Developer's Certificate of Origin on all commits (see above).

LICENSE

@@ -192,7 +192,7 @@ Apache License
    you may not use this file except in compliance with the License.
    You may obtain a copy of the License at
 
-       http://www.apache.org/licenses/LICENSE-2.0
+       https://www.apache.org/licenses/LICENSE-2.0
 
    Unless required by applicable law or agreed to in writing, software
    distributed under the License is distributed on an "AS IS" BASIS,

README.md

@@ -15,8 +15,8 @@ Be aware that "go get github.com/ory-am/fosite" will give you the master branch,
 Once releases roll out, you will be able to fetch a specific fosite API version through gopkg.in.
 
 These Standards have been reviewed during the development of Fosite:
-* [OAuth 2.0 Multiple Response Type Encoding Practices](http://openid.net/specs/oauth-v2-multiple-response-types-1_0.html)
-* [OpenID Connect Core 1.0](http://openid.net/specs/openid-connect-core-1_0.html)
+* [OAuth 2.0 Multiple Response Type Encoding Practices](https://openid.net/specs/oauth-v2-multiple-response-types-1_0.html)
+* [OpenID Connect Core 1.0](https://openid.net/specs/openid-connect-core-1_0.html)
 * [The OAuth 2.0 Authorization Framework](https://tools.ietf.org/html/rfc6749)
 * [OAuth 2.0 Threat Model and Security Considerations](https://tools.ietf.org/html/rfc6819)
 
@@ -86,6 +86,9 @@ of things we implemented in Fosite:
 * [Binding of Authorization "code" to "redirect_uri"](https://tools.ietf.org/html/rfc6819#section-5.2.4.6)
 * [Opaque access tokens](https://tools.ietf.org/html/rfc6749#section-1.4)
 * [Opaque refresh tokens](https://tools.ietf.org/html/rfc6749#section-1.5)
+* [Ensure Confidentiality of Requests](https://tools.ietf.org/html/rfc6819#section-5.1.1)
+  Fosite ensures that redirect URIs use https *(except localhost)* but you need to implement
+  TLS for the token and auth endpoints yourself.
 
 Not implemented yet:
 * [Use of Asymmetric Cryptography](https://tools.ietf.org/html/rfc6819#section-5.1.4.1.5) - enigma should use asymmetric
@@ -142,7 +145,7 @@ go install github.com/ory-am/fosite/fosite-example
 fosite-example
 ```
 
-There should be a server listening on [localhost:3846](http://localhost:3846/). You can check out the example's
+There should be a server listening on [localhost:3846](https://localhost:3846/). You can check out the example's
 source code [here](/fosite-example/main.go).
 
 ### Installation

authorize_error_test.go

@@ -31,8 +31,8 @@ func TestWriteAuthorizeError(t *testing.T) {
 	defer ctrl.Finish()
 
 	var urls = []string{
-		"http://foobar.com/",
-		"http://foobar.com/?foo=bar",
+		"https://foobar.com/",
+		"https://foobar.com/?foo=bar",
 	}
 	var purls = []*url.URL{}
 	for _, u := range urls {
@@ -68,7 +68,7 @@ func TestWriteAuthorizeError(t *testing.T) {
 				rw.EXPECT().WriteHeader(http.StatusFound)
 			},
 			checkHeader: func(k int) {
-				a, _ := url.Parse("http://foobar.com/?error=invalid_request&error_description=The+request+is+missing+a+required+parameter%2C+includes+an+invalid+parameter+value%2C+includes+a+parameter+more+than+once%2C+or+is+otherwise+malformed")
+				a, _ := url.Parse("https://foobar.com/?error=invalid_request&error_description=The+request+is+missing+a+required+parameter%2C+includes+an+invalid+parameter+value%2C+includes+a+parameter+more+than+once%2C+or+is+otherwise+malformed")
 				b, _ := url.Parse(header.Get("Location"))
 				assert.Equal(t, a, b, "%d", k)
 			},
@@ -82,7 +82,7 @@ func TestWriteAuthorizeError(t *testing.T) {
 				rw.EXPECT().WriteHeader(http.StatusFound)
 			},
 			checkHeader: func(k int) {
-				a, _ := url.Parse("http://foobar.com/?error=invalid_request&error_description=The+request+is+missing+a+required+parameter%2C+includes+an+invalid+parameter+value%2C+includes+a+parameter+more+than+once%2C+or+is+otherwise+malformed&foo=bar")
+				a, _ := url.Parse("https://foobar.com/?error=invalid_request&error_description=The+request+is+missing+a+required+parameter%2C+includes+an+invalid+parameter+value%2C+includes+a+parameter+more+than+once%2C+or+is+otherwise+malformed&foo=bar")
 				b, _ := url.Parse(header.Get("Location"))
 				assert.Equal(t, a, b, "%d", k)
 			},

authorize_helper.go

@@ -5,6 +5,7 @@ import (
 	"github.com/go-errors/errors"
 	. "github.com/ory-am/fosite/client"
 	"net/url"
+	"strings"
 )
 
 // GetRedirectURIFromRequestValues extracts the redirect_uri from values but does not do any sort of validation.
@@ -29,7 +30,7 @@ func GetRedirectURIFromRequestValues(values url.Values) (string, error) {
 // uri validation.
 //
 // Considered specifications
-// * http://tools.ietf.org/html/rfc6749#section-3.1.2.3
+// * https://tools.ietf.org/html/rfc6749#section-3.1.2.3
 //   If multiple redirection URIs have been registered, if only part of
 //   the redirection URI has been registered, or if no redirection URI has
 //   been registered, the client MUST include a redirection URI with the
@@ -80,6 +81,7 @@ func MatchRedirectURIWithClientRedirectURIs(rawurl string, client Client) (*url.
 //   * The endpoint URI MUST NOT include a fragment component.
 // * https://tools.ietf.org/html/rfc3986#section-4.3
 //   absolute-URI  = scheme ":" hier-part [ "?" query ]
+// * https://tools.ietf.org/html/rfc6819#section-5.1.1
 func IsValidRedirectURI(redirectURI *url.URL) bool {
 	// We need to explicitly check for a scheme
 	if !govalidator.IsRequestURL(redirectURI.String()) {
@@ -91,5 +93,14 @@ func IsValidRedirectURI(redirectURI *url.URL) bool {
 		return false
 	}
 
+	if redirectURI.Scheme != "https" && !isLocalhost(redirectURI) {
+		return false
+	}
+
 	return true
 }
+
+func isLocalhost(redirectURI *url.URL) bool {
+	host := strings.Split(redirectURI.Host, ":")[0]
+	return host == "localhost" || host == "127.0.0.1"
+}

authorize_helper_test.go

@@ -8,6 +8,22 @@ import (
 	"testing"
 )
 
+func TestIsLocalhost(t *testing.T) {
+	for k, c := range []struct {
+		expect bool
+		rawurl string
+	}{
+		{expect: false, rawurl: "https://foo.bar"},
+		{expect: true, rawurl: "https://localhost"},
+		{expect: true, rawurl: "https://localhost:1234"},
+		{expect: true, rawurl: "https://127.0.0.1:1234"},
+		{expect: true, rawurl: "https://127.0.0.1"},
+	} {
+		u, _ := url.Parse(c.rawurl)
+		assert.Equal(t, c.expect, isLocalhost(u), "case %d", k)
+	}
+}
+
 // Test for
 // * https://tools.ietf.org/html/rfc6749#section-3.1.2
 //   The endpoint URI MAY include an
@@ -21,8 +37,8 @@ func TestGetRedirectURI(t *testing.T) {
 		expected string
 	}{
 		{in: "", isError: false, expected: ""},
-		{in: "http://google.com/", isError: false, expected: "http://google.com/"},
-		{in: "http://google.com/?foo=bar%20foo+baz", isError: false, expected: "http://google.com/?foo=bar foo baz"},
+		{in: "https://google.com/", isError: false, expected: "https://google.com/"},
+		{in: "https://google.com/?foo=bar%20foo+baz", isError: false, expected: "https://google.com/?foo=bar foo baz"},
 	} {
 		values := url.Values{}
 		values.Set("redirect_uri", c.in)
@@ -55,34 +71,34 @@ func TestDoesClientWhiteListRedirect(t *testing.T) {
 	}{
 		{
 			client:  &SecureClient{RedirectURIs: []string{""}},
-			url:     "http://foo.com/cb",
+			url:     "https://foo.com/cb",
 			isError: true,
 		},
 		{
-			client:  &SecureClient{RedirectURIs: []string{"http://bar.com/cb"}},
-			url:     "http://foo.com/cb",
+			client:  &SecureClient{RedirectURIs: []string{"https://bar.com/cb"}},
+			url:     "https://foo.com/cb",
 			isError: true,
 		},
 		{
-			client:   &SecureClient{RedirectURIs: []string{"http://bar.com/cb"}},
+			client:   &SecureClient{RedirectURIs: []string{"https://bar.com/cb"}},
 			url:      "",
 			isError:  false,
-			expected: "http://bar.com/cb",
+			expected: "https://bar.com/cb",
 		},
 		{
 			client:  &SecureClient{RedirectURIs: []string{""}},
 			url:     "",
 			isError: true,
 		},
 		{
-			client:   &SecureClient{RedirectURIs: []string{"http://bar.com/cb"}},
-			url:      "http://bar.com/cb",
+			client:   &SecureClient{RedirectURIs: []string{"https://bar.com/cb"}},
+			url:      "https://bar.com/cb",
 			isError:  false,
-			expected: "http://bar.com/cb",
+			expected: "https://bar.com/cb",
 		},
 		{
-			client:  &SecureClient{RedirectURIs: []string{"http://bar.com/cb"}},
-			url:     "http://bar.com/cb123",
+			client:  &SecureClient{RedirectURIs: []string{"https://bar.com/cb"}},
+			url:     "https://bar.com/cb123",
 			isError: true,
 		},
 	} {

authorize_request_handler_test.go

@@ -25,7 +25,7 @@ func TestNewAuthorizeRequest(t *testing.T) {
 	store := NewMockStorage(ctrl)
 	defer ctrl.Finish()
 
-	redir, _ := url.Parse("http://foo.bar/cb")
+	redir, _ := url.Parse("https://foo.bar/cb")
 	for k, c := range []struct {
 		desc          string
 		conf          *Fosite
@@ -59,7 +59,7 @@ func TestNewAuthorizeRequest(t *testing.T) {
 		{
 			desc:          "invalid client fails",
 			conf:          &Fosite{Store: store},
-			query:         url.Values{"redirect_uri": []string{"http://foo.bar/cb"}},
+			query:         url.Values{"redirect_uri": []string{"https://foo.bar/cb"}},
 			expectedError: ErrInvalidClient,
 			mock: func() {
 				store.EXPECT().GetClient(gomock.Any()).Return(nil, errors.New("foo"))
@@ -95,7 +95,7 @@ func TestNewAuthorizeRequest(t *testing.T) {
 			desc: "client and request redirects mismatch",
 			conf: &Fosite{Store: store},
 			query: url.Values{
-				"redirect_uri": []string{"http://foo.bar/cb"},
+				"redirect_uri": []string{"https://foo.bar/cb"},
 				"client_id":    []string{"1234"},
 			},
 			expectedError: ErrInvalidRequest,
@@ -108,43 +108,43 @@ func TestNewAuthorizeRequest(t *testing.T) {
 			desc: "no state",
 			conf: &Fosite{Store: store},
 			query: url.Values{
-				"redirect_uri":  []string{"http://foo.bar/cb"},
+				"redirect_uri":  []string{"https://foo.bar/cb"},
 				"client_id":     []string{"1234"},
 				"response_type": []string{"code"},
 			},
 			expectedError: ErrInvalidState,
 			mock: func() {
-				store.EXPECT().GetClient("1234").Return(&SecureClient{RedirectURIs: []string{"http://foo.bar/cb"}}, nil)
+				store.EXPECT().GetClient("1234").Return(&SecureClient{RedirectURIs: []string{"https://foo.bar/cb"}}, nil)
 			},
 		},
 		/* short state */
 		{
 			desc: "short state",
 			conf: &Fosite{Store: store},
 			query: url.Values{
-				"redirect_uri":  {"http://foo.bar/cb"},
+				"redirect_uri":  {"https://foo.bar/cb"},
 				"client_id":     {"1234"},
 				"response_type": {"code"},
 				"state":         {"short"},
 			},
 			expectedError: ErrInvalidState,
 			mock: func() {
-				store.EXPECT().GetClient("1234").Return(&SecureClient{RedirectURIs: []string{"http://foo.bar/cb"}}, nil)
+				store.EXPECT().GetClient("1234").Return(&SecureClient{RedirectURIs: []string{"https://foo.bar/cb"}}, nil)
 			},
 		},
 		/* success case */
 		{
 			desc: "should pass",
 			conf: &Fosite{Store: store},
 			query: url.Values{
-				"redirect_uri":  {"http://foo.bar/cb"},
+				"redirect_uri":  {"https://foo.bar/cb"},
 				"client_id":     {"1234"},
 				"response_type": {"code token"},
 				"state":         {"strong-state"},
 				"scope":         {"foo bar"},
 			},
 			mock: func() {
-				store.EXPECT().GetClient("1234").Return(&SecureClient{RedirectURIs: []string{"http://foo.bar/cb"}}, nil)
+				store.EXPECT().GetClient("1234").Return(&SecureClient{RedirectURIs: []string{"https://foo.bar/cb"}}, nil)
 			},
 			expectedError: ErrInvalidScope,
 		},
@@ -153,18 +153,18 @@ func TestNewAuthorizeRequest(t *testing.T) {
 			desc: "should pass",
 			conf: &Fosite{Store: store},
 			query: url.Values{
-				"redirect_uri":  {"http://foo.bar/cb"},
+				"redirect_uri":  {"https://foo.bar/cb"},
 				"client_id":     {"1234"},
 				"response_type": {"code token"},
 				"state":         {"strong-state"},
 				"scope":         {DefaultRequiredScopeName + " foo bar"},
 			},
 			mock: func() {
-				store.EXPECT().GetClient("1234").Return(&SecureClient{RedirectURIs: []string{"http://foo.bar/cb"}}, nil)
+				store.EXPECT().GetClient("1234").Return(&SecureClient{RedirectURIs: []string{"https://foo.bar/cb"}}, nil)
 			},
 			expect: &AuthorizeRequest{
 				RedirectURI:   redir,
-				Client:        &SecureClient{RedirectURIs: []string{"http://foo.bar/cb"}},
+				Client:        &SecureClient{RedirectURIs: []string{"https://foo.bar/cb"}},
 				ResponseTypes: []string{"code", "token"},
 				State:         "strong-state",
 				Scopes:        []string{DefaultRequiredScopeName, "foo", "bar"},

authorize_request_test.go

@@ -28,14 +28,14 @@ func TestAuthorizeRequest(t *testing.T) {
 		},
 		{
 			ar: &AuthorizeRequest{
-				RedirectURI: urlparse("http://foobar"),
+				RedirectURI: urlparse("https://foobar"),
 			},
 			isRedirValid: false,
 		},
 		{
 			ar: &AuthorizeRequest{
 				Client:      &client.SecureClient{RedirectURIs: []string{""}},
-				RedirectURI: urlparse("http://foobar"),
+				RedirectURI: urlparse("https://foobar"),
 			},
 			isRedirValid: false,
 		},
@@ -55,22 +55,22 @@ func TestAuthorizeRequest(t *testing.T) {
 		},
 		{
 			ar: &AuthorizeRequest{
-				Client:      &client.SecureClient{RedirectURIs: []string{"http://foobar.com#123"}},
-				RedirectURI: urlparse("http://foobar.com#123"),
+				Client:      &client.SecureClient{RedirectURIs: []string{"https://foobar.com#123"}},
+				RedirectURI: urlparse("https://foobar.com#123"),
 			},
 			isRedirValid: false,
 		},
 		{
 			ar: &AuthorizeRequest{
-				Client:      &client.SecureClient{RedirectURIs: []string{"http://foobar.com"}},
-				RedirectURI: urlparse("http://foobar.com#123"),
+				Client:      &client.SecureClient{RedirectURIs: []string{"https://foobar.com"}},
+				RedirectURI: urlparse("https://foobar.com#123"),
 			},
 			isRedirValid: false,
 		},
 		{
 			ar: &AuthorizeRequest{
-				Client:        &client.SecureClient{RedirectURIs: []string{"http://foobar.com/cb"}},
-				RedirectURI:   urlparse("http://foobar.com/cb"),
+				Client:        &client.SecureClient{RedirectURIs: []string{"https://foobar.com/cb"}},
+				RedirectURI:   urlparse("https://foobar.com/cb"),
 				RequestedAt:   time.Now(),
 				ResponseTypes: []string{"foo", "bar"},
 				Scopes:        []string{"foo", "bar"},