From 540a7bd7be7237fb9cb703a0a1c5de36d7c54677 Mon Sep 17 00:00:00 2001 From: Livio Amstutz Date: Thu, 29 Apr 2021 12:43:21 +0200 Subject: [PATCH] improve Loopback check --- pkg/op/authrequest.go | 15 ++++++++------- pkg/op/authrequest_test.go | 33 +++++++++++++++++++++++++-------- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/pkg/op/authrequest.go b/pkg/op/authrequest.go index 765027f..8e6d8eb 100644 --- a/pkg/op/authrequest.go +++ b/pkg/op/authrequest.go @@ -3,6 +3,7 @@ package op import ( "context" "fmt" + "net" "net/http" "net/url" "strings" @@ -184,7 +185,7 @@ func ValidateAuthReqRedirectURI(client Client, uri string, responseType oidc.Res //ValidateAuthReqRedirectURINative validates the passed redirect_uri and response_type to the registered uris and client type func validateAuthReqRedirectURINative(client Client, uri string, responseType oidc.ResponseType) error { - parsedURL, isLoopback := LoopbackOrLocalhost(uri) + parsedURL, isLoopback := HTTPLoopbackOrLocalhost(uri) isCustomSchema := !strings.HasPrefix(uri, "http://") if utils.Contains(client.RedirectURIs(), uri) { if isLoopback || isCustomSchema { @@ -196,7 +197,7 @@ func validateAuthReqRedirectURINative(client Client, uri string, responseType oi return ErrInvalidRequestRedirectURI("The requested redirect_uri is missing in the client configuration. If you have any questions, you may contact the administrator of the application.") } for _, uri := range client.RedirectURIs() { - redirectURI, ok := LoopbackOrLocalhost(uri) + redirectURI, ok := HTTPLoopbackOrLocalhost(uri) if ok && equalURI(parsedURL, redirectURI) { return nil } @@ -208,16 +209,16 @@ func equalURI(url1, url2 *url.URL) bool { return url1.Path == url2.Path && url1.RawQuery == url2.RawQuery } -func LoopbackOrLocalhost(rawurl string) (*url.URL, bool) { +func HTTPLoopbackOrLocalhost(rawurl string) (*url.URL, bool) { parsedURL, err := url.Parse(rawurl) if err != nil { return nil, false } + if parsedURL.Scheme != "http" { + return nil, false + } hostName := parsedURL.Hostname() - return parsedURL, parsedURL.Scheme == "http" && - hostName == "localhost" || - hostName == "127.0.0.1" || - hostName == "::1" + return parsedURL, hostName == "localhost" || net.ParseIP(hostName).IsLoopback() } //ValidateAuthReqResponseType validates the passed response_type to the registered response types diff --git a/pkg/op/authrequest_test.go b/pkg/op/authrequest_test.go index 8427079..9bec1e7 100644 --- a/pkg/op/authrequest_test.go +++ b/pkg/op/authrequest_test.go @@ -316,16 +316,16 @@ func TestValidateAuthReqRedirectURI(t *testing.T) { false, }, { - "code flow registered http not confidential (user agent) fails", + "code flow registered http not confidential (native) fails", args{"http://registered.com/callback", - mock.NewClientWithConfig(t, []string{"http://registered.com/callback"}, op.ApplicationTypeUserAgent, nil, false), + mock.NewClientWithConfig(t, []string{"http://registered.com/callback"}, op.ApplicationTypeNative, nil, false), oidc.ResponseTypeCode}, true, }, { - "code flow registered http not confidential (native) fails", + "code flow registered http not confidential (user agent) fails", args{"http://registered.com/callback", - mock.NewClientWithConfig(t, []string{"http://registered.com/callback"}, op.ApplicationTypeNative, nil, false), + mock.NewClientWithConfig(t, []string{"http://registered.com/callback"}, op.ApplicationTypeUserAgent, nil, false), oidc.ResponseTypeCode}, true, }, @@ -344,7 +344,7 @@ func TestValidateAuthReqRedirectURI(t *testing.T) { false, }, { - "code flow registered http localhost native ok", + "code flow registered http loopback v6 native ok", args{"http://[::1]:4200/callback", mock.NewClientWithConfig(t, []string{"http://[::1]/callback"}, op.ApplicationTypeNative, nil, false), oidc.ResponseTypeCode}, @@ -420,6 +420,13 @@ func TestValidateAuthReqRedirectURI(t *testing.T) { oidc.ResponseTypeIDToken}, false, }, + { + "implicit flow registered http localhost web fails", + args{"http://localhost:9999/callback", + mock.NewClientWithConfig(t, []string{"http://localhost:9999/callback"}, op.ApplicationTypeWeb, nil, false), + oidc.ResponseTypeIDToken}, + true, + }, { "implicit flow registered http localhost user agent fails", args{"http://localhost:9999/callback", @@ -581,10 +588,15 @@ func Test_LoopbackOrLocalhost(t *testing.T) { true, }, { - "v6 no port ok", + "v6 short no port ok", args{url: "http://[::1]/test"}, true, }, + { + "v6 long no port ok", + args{url: "http://[0:0:0:0:0:0:0:1]/test"}, + true, + }, { "locahost no port ok", args{url: "http://localhost/test"}, @@ -596,10 +608,15 @@ func Test_LoopbackOrLocalhost(t *testing.T) { true, }, { - "v6 with port ok", + "v6 short with port ok", args{url: "http://[::1]:4200/test"}, true, }, + { + "v6 long with port ok", + args{url: "http://[0:0:0:0:0:0:0:1]:4200/test"}, + true, + }, { "localhost with port ok", args{url: "http://localhost:4200/test"}, @@ -608,7 +625,7 @@ func Test_LoopbackOrLocalhost(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if _, got := op.LoopbackOrLocalhost(tt.args.url); got != tt.want { + if _, got := op.HTTPLoopbackOrLocalhost(tt.args.url); got != tt.want { t.Errorf("loopbackOrLocalhost() = %v, want %v", got, tt.want) } })