Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC8252#section-7.3 Loopback Interface Redirection #400

Merged
merged 3 commits into from
Mar 4, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion authorize_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ package fosite

import (
"net/url"
"regexp"
"strings"

"github.com/asaskevich/govalidator"
Expand Down Expand Up @@ -83,7 +84,7 @@ func MatchRedirectURIWithClientRedirectURIs(rawurl string, client Client) (*url.
// If no redirect_uri was given and the client has exactly one valid redirect_uri registered, use that instead
return redirectURIFromClient, nil
}
} else if rawurl != "" && StringInSlice(rawurl, client.GetRedirectURIs()) {
} else if rawurl != "" && isMatchingRedirectURI(rawurl, client.GetRedirectURIs()) {
// If a redirect_uri was given and the clients knows it (simple string comparison!)
// return it.
if parsed, err := url.Parse(rawurl); err == nil && IsValidRedirectURI(parsed) {
Expand All @@ -95,6 +96,40 @@ func MatchRedirectURIWithClientRedirectURIs(rawurl string, client Client) (*url.
return nil, errors.WithStack(ErrInvalidRequest.WithHint(`The "redirect_uri" parameter does not match any of the OAuth 2.0 Client's pre-registered redirect urls.`))
}

// Match a requested redirect URI against a pool of registered client URIs
//
// Test a given redirect URI against a pool of URIs provided by a registered client.
// If the OAuth 2.0 Client has loopback URIs registered either an IPv4 URI http://127.0.0.1 or
// an IPv6 URI http://[::1] a client is allowed to request a dynamic port and the server MUST accept
// it as a valid redirection uri.
//
// https://tools.ietf.org/html/rfc8252#section-7.3
// Native apps that are able to open a port on the loopback network
// interface without needing special permissions (typically, those on
// desktop operating systems) can use the loopback interface to receive
// the OAuth redirect.
//
// Loopback redirect URIs use the "http" scheme and are constructed with
// the loopback IP literal and whatever port the client is listening on.
func isMatchingRedirectURI(uri string, haystack []string) bool {
for _, b := range haystack {
l := strings.ToLower(b)
if l == strings.ToLower(uri) || isLoopbackURI(uri, l) {
return true
}
}
return false
}

func isLoopbackURI(uri string, registeredURI string) bool {
if registeredURI != "http://127.0.0.1" && registeredURI != "http://[::1]" {
return false
}

match, _ := regexp.MatchString("http://(127.0.0.1|\\[::1\\]):?(\\d+)?", uri)
return match
}

// IsValidRedirectURI validates a redirect_uri as specified in:
//
// * https://tools.ietf.org/html/rfc6749#section-3.1.2
Expand Down
40 changes: 40 additions & 0 deletions authorize_helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,46 @@ func TestDoesClientWhiteListRedirect(t *testing.T) {
url: "https://bar.com/cb123",
isError: true,
},
{
client: &DefaultClient{RedirectURIs: []string{"http://[::1]"}},
url: "http://[::1]:1024",
isError: false,
expected: "http://[::1]:1024",
},
{
client: &DefaultClient{RedirectURIs: []string{"http://[::1]"}},
url: "http://[::1]:1024/cb",
isError: false,
expected: "http://[::1]:1024/cb",
},
{
client: &DefaultClient{RedirectURIs: []string{"http://[::1]"}},
url: "http://foo.bar/bar",
isError: true,
},
{
client: &DefaultClient{RedirectURIs: []string{"http://127.0.0.1"}},
url: "http://127.0.0.1:1024",
isError: false,
expected: "http://127.0.0.1:1024",
},
{
client: &DefaultClient{RedirectURIs: []string{"http://127.0.0.1"}},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that this should fail because the whitelisted redirect URL does not have the /cb path segment. The spec only talks about randomly assigned ports but does not leave room for variable paths, if I understand correctly. Otherwise, this looks pretty good!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds right. I had to change some bits because of that. Using url parse now to extract the paths and compare them.

url: "http://127.0.0.1:64000/cb",
isError: false,
expected: "http://127.0.0.1:64000/cb",
},
{
client: &DefaultClient{RedirectURIs: []string{"http://127.0.0.1"}},
url: "http://127.0.0.1",
isError: false,
expected: "http://127.0.0.1",
},
{
client: &DefaultClient{RedirectURIs: []string{"http://127.0.0.1"}},
url: "http://foo.bar/bar",
isError: true,
},
} {
redir, err := MatchRedirectURIWithClientRedirectURIs(c.url, c.client)
assert.Equal(t, c.isError, err != nil, "%d: %s", k, err)
Expand Down