Skip to content

Commit

Permalink
authorize: enforce https for all redirect endpoints except localhost
Browse files Browse the repository at this point in the history
  • Loading branch information
Aeneas Rekkas committed Jan 12, 2016
1 parent 8c625c9 commit d65b45a
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 72 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
2 changes: 1 addition & 1 deletion LICENSE
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
35 changes: 17 additions & 18 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -43,16 +43,9 @@ These Standards have been reviewed during the development of Fosite:

## Motivation

Why write another OAuth2 server side library for Go Lang?

Other libraries are perfect for a non-critical set ups, but [fail](https://github.com/RangelReale/osin/issues/107)
to comply with advanced security requirements. Additionally, the frameworks we analyzed did not support extension
of the OAuth2 protocol easily. But OAuth2 is an extensible framework. Your OAuth2 should as well.
This is unfortunately not an issue exclusive to Go's eco system but to many others as well.

Fosite was written because [Hydra](https://github.com/ory-am/hydra) required a more secure and extensible OAuth2 library
then the one it was using. We quickly realized, that OAuth2 implementations out there are *not secure* nor *extensible,
so we decided to write one *that is*.
Fosite was written because our OAuth2 and OpenID Connect service [Hydra](https://github.com/ory-am/hydra)
required a secure and extensible OAuth2 library. We had to realize that nothing matching our requirements
was out there, so we decided to build it ourselves.

## A word on quality

Expand Down Expand Up @@ -86,6 +79,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
Expand Down Expand Up @@ -132,6 +128,9 @@ This section is WIP and we welcome discussions via PRs or in the issues.

### See it in action

The example does not have nice visuals but it should give you an idea of what you can do with Fosite and a few lines
of code.

![Authorize Code Grant](docs/authorize-code-grant.gif)

You can run this minimalistic example by doing
Expand All @@ -142,7 +141,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
Expand All @@ -156,7 +155,7 @@ Fosite is being shipped through gopkg.in so new updates don't break your code.
go get gopkg.in/ory-am/fosite.v{X}/...
```

To see a full list of available versions check the [tags](https://github.com/ory-am/fosite/tags) page. If you want
To see a full list of available versions check [gopkg.in/ory-am/fosite.v0](https://gopkg.in/ory-am/fosite.v0). If you want
to use api version 2 for example (version 2 does not exist yet), do:

```
Expand Down Expand Up @@ -357,13 +356,13 @@ the `AuthorizeEndpointHandler` API.

You can find a complete list of handlers inside the [handler directory](). A short list is documented here:

* *github.com/ory-am/fosite/handler/core/explicit.AuthorizeExplicitEndpointHandler:* implements the
* `github.com/ory-am/fosite/handler/core/explicit.AuthorizeExplicitEndpointHandler` implements the
[Authorization Code Grant](https://tools.ietf.org/html/rfc6749#section-4.1)
* *github.com/ory-am/fosite/handler/core/implicit.AuthorizeImplicitEndpointHandler:* implements the
* `github.com/ory-am/fosite/handler/core/implicit.AuthorizeImplicitEndpointHandler` implements the
[Implicit Grant](https://tools.ietf.org/html/rfc6749#section-4.2)
* *github.com/ory-am/fosite/handler/core/token/owner.TokenROPasswordCredentialsEndpointHandler:* implements the
* `github.com/ory-am/fosite/handler/core/token/owner.TokenROPasswordCredentialsEndpointHandler` implements the
[Resource Owner Password Credentials Grant](https://tools.ietf.org/html/rfc6749#section-4.3)
* *github.com/ory-am/fosite/handler/core/token/client.TokenClientCredentialsEndpointHandler:* implements the
* `github.com/ory-am/fosite/handler/core/token/client.TokenClientCredentialsEndpointHandler` implements the
[Client Credentials Grant](https://tools.ietf.org/html/rfc6749#section-4.4)

### Replaceable storage
Expand Down
8 changes: 4 additions & 4 deletions authorize_error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
},
Expand All @@ -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)
},
Expand Down
13 changes: 12 additions & 1 deletion authorize_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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()) {
Expand All @@ -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"
}
40 changes: 28 additions & 12 deletions authorize_helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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,
},
} {
Expand Down
24 changes: 12 additions & 12 deletions authorize_request_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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,
Expand All @@ -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,
},
Expand All @@ -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"},
Expand Down
16 changes: 8 additions & 8 deletions authorize_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand All @@ -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"},
Expand Down
Loading

0 comments on commit d65b45a

Please sign in to comment.