From 72fc86164c38d23fbe5a7982a574bebfb4f58777 Mon Sep 17 00:00:00 2001 From: Livio Amstutz Date: Mon, 26 Apr 2021 14:31:26 +0200 Subject: [PATCH] fix: allow loopback redirect_uri for native apps --- pkg/op/authrequest.go | 65 ++++++++++++---- pkg/op/authrequest_test.go | 151 ++++++++++++++++++++++++++++++++++--- 2 files changed, 192 insertions(+), 24 deletions(-) diff --git a/pkg/op/authrequest.go b/pkg/op/authrequest.go index 3a79b9b..765027f 100644 --- a/pkg/op/authrequest.go +++ b/pkg/op/authrequest.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "net/url" "strings" "time" @@ -157,32 +158,66 @@ func ValidateAuthReqRedirectURI(client Client, uri string, responseType oidc.Res if uri == "" { return ErrInvalidRequestRedirectURI("The redirect_uri is missing in the request. Please ensure it is added to the request. If you have any questions, you may contact the administrator of the application.") } + if strings.HasPrefix(uri, "https://") { + if !utils.Contains(client.RedirectURIs(), uri) { + 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.") + } + return nil + } + if client.ApplicationType() == ApplicationTypeNative { + return validateAuthReqRedirectURINative(client, uri, responseType) + } if !utils.Contains(client.RedirectURIs(), uri) { 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.") } - if client.DevMode() { - return nil - } - if strings.HasPrefix(uri, "https://") { - return nil - } - if responseType == oidc.ResponseTypeCode { - if strings.HasPrefix(uri, "http://") && IsConfidentialType(client) { + if strings.HasPrefix(uri, "http://") { + if client.DevMode() { return nil } - if !strings.HasPrefix(uri, "http://") && client.ApplicationType() == ApplicationTypeNative { + if responseType == oidc.ResponseTypeCode && IsConfidentialType(client) { return nil } return ErrInvalidRequest("This client's redirect_uri is http and is not allowed. If you have any questions, you may contact the administrator of the application.") - } else { - if client.ApplicationType() != ApplicationTypeNative { - return ErrInvalidRequestRedirectURI("Http is only allowed for native applications. Please change your redirect uri try again. If you have any questions, you may contact the administrator of the application.") + } + return ErrInvalidRequest("This client's redirect_uri is using a custom schema and is not allowed. If you have any questions, you may contact the administrator of the application.") +} + +//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) + isCustomSchema := !strings.HasPrefix(uri, "http://") + if utils.Contains(client.RedirectURIs(), uri) { + if isLoopback || isCustomSchema { + return nil } - if !(strings.HasPrefix(uri, "http://localhost:") || strings.HasPrefix(uri, "http://localhost/")) { - return ErrInvalidRequestRedirectURI("Http is only allowed for localhost uri. Please change your redirect uri try again. If you have any questions, you may contact the administrator of the application at:") + return ErrInvalidRequest("This client's redirect_uri is http and is not allowed. If you have any questions, you may contact the administrator of the application.") + } + if !isLoopback { + 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) + if ok && equalURI(parsedURL, redirectURI) { + return nil } } - return nil + 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.") +} + +func equalURI(url1, url2 *url.URL) bool { + return url1.Path == url2.Path && url1.RawQuery == url2.RawQuery +} + +func LoopbackOrLocalhost(rawurl string) (*url.URL, bool) { + parsedURL, err := url.Parse(rawurl) + if err != nil { + return nil, false + } + hostName := parsedURL.Hostname() + return parsedURL, parsedURL.Scheme == "http" && + hostName == "localhost" || + hostName == "127.0.0.1" || + hostName == "::1" } //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 3856acd..8427079 100644 --- a/pkg/op/authrequest_test.go +++ b/pkg/op/authrequest_test.go @@ -274,28 +274,112 @@ func TestValidateAuthReqRedirectURI(t *testing.T) { true, }, { - "unregistered fails", + "unregistered https fails", args{"https://unregistered.com/callback", mock.NewClientWithConfig(t, []string{"https://registered.com/callback"}, op.ApplicationTypeWeb, nil, false), oidc.ResponseTypeCode}, true, }, { - "code flow registered http not confidential fails", - args{"http://registered.com/callback", - mock.NewClientWithConfig(t, []string{"http://registered.com/callback"}, op.ApplicationTypeUserAgent, nil, false), + "unregistered http fails", + args{"http://unregistered.com/callback", + mock.NewClientWithConfig(t, []string{"http://registered.com/callback"}, op.ApplicationTypeWeb, nil, false), oidc.ResponseTypeCode}, true, }, { - "code flow registered http confidential ok", + "code flow registered https web ok", + args{"https://registered.com/callback", + mock.NewClientWithConfig(t, []string{"https://registered.com/callback"}, op.ApplicationTypeWeb, nil, false), + oidc.ResponseTypeCode}, + false, + }, + { + "code flow registered https native ok", + args{"https://registered.com/callback", + mock.NewClientWithConfig(t, []string{"https://registered.com/callback"}, op.ApplicationTypeNative, nil, false), + oidc.ResponseTypeCode}, + false, + }, + { + "code flow registered https user agent ok", + args{"https://registered.com/callback", + mock.NewClientWithConfig(t, []string{"https://registered.com/callback"}, op.ApplicationTypeUserAgent, nil, false), + oidc.ResponseTypeCode}, + false, + }, + { + "code flow registered http confidential (web) ok", args{"http://registered.com/callback", mock.NewClientWithConfig(t, []string{"http://registered.com/callback"}, op.ApplicationTypeWeb, nil, false), oidc.ResponseTypeCode}, false, }, { - "code flow registered custom not 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.ApplicationTypeUserAgent, nil, false), + oidc.ResponseTypeCode}, + true, + }, + { + "code flow registered http not confidential (native) fails", + args{"http://registered.com/callback", + mock.NewClientWithConfig(t, []string{"http://registered.com/callback"}, op.ApplicationTypeNative, nil, false), + oidc.ResponseTypeCode}, + true, + }, + { + "code flow registered http localhost native ok", + args{"http://localhost:4200/callback", + mock.NewClientWithConfig(t, []string{"http://localhost/callback"}, op.ApplicationTypeNative, nil, false), + oidc.ResponseTypeCode}, + false, + }, + { + "code flow registered http loopback v4 native ok", + args{"http://127.0.0.1:4200/callback", + mock.NewClientWithConfig(t, []string{"http://127.0.0.1/callback"}, op.ApplicationTypeNative, nil, false), + oidc.ResponseTypeCode}, + false, + }, + { + "code flow registered http localhost native ok", + args{"http://[::1]:4200/callback", + mock.NewClientWithConfig(t, []string{"http://[::1]/callback"}, op.ApplicationTypeNative, nil, false), + oidc.ResponseTypeCode}, + false, + }, + { + "code flow unregistered http native fails", + args{"http://unregistered.com/callback", + mock.NewClientWithConfig(t, []string{"http://locahost/callback"}, op.ApplicationTypeNative, nil, false), + oidc.ResponseTypeCode}, + true, + }, + { + "code flow unregistered custom native fails", + args{"unregistered://callback", + mock.NewClientWithConfig(t, []string{"registered://callback"}, op.ApplicationTypeNative, nil, false), + oidc.ResponseTypeCode}, + true, + }, + { + "code flow unregistered loopback native fails", + args{"http://[::1]:4200/unregistered", + mock.NewClientWithConfig(t, []string{"http://[::1]:4200/callback"}, op.ApplicationTypeNative, nil, false), + oidc.ResponseTypeCode}, + true, + }, + { + "code flow registered custom not native (web) fails", + args{"custom://callback", + mock.NewClientWithConfig(t, []string{"custom://callback"}, op.ApplicationTypeWeb, nil, false), + oidc.ResponseTypeCode}, + true, + }, + { + "code flow registered custom not native (user agent) fails", args{"custom://callback", mock.NewClientWithConfig(t, []string{"custom://callback"}, op.ApplicationTypeUserAgent, nil, false), oidc.ResponseTypeCode}, @@ -311,7 +395,7 @@ func TestValidateAuthReqRedirectURI(t *testing.T) { { "code flow dev mode http ok", args{"http://registered.com/callback", - mock.NewClientWithConfig(t, []string{"http://registered.com/callback"}, op.ApplicationTypeNative, nil, true), + mock.NewClientWithConfig(t, []string{"http://registered.com/callback"}, op.ApplicationTypeUserAgent, nil, true), oidc.ResponseTypeCode}, false, }, @@ -355,12 +439,12 @@ func TestValidateAuthReqRedirectURI(t *testing.T) { args{"custom://callback", mock.NewClientWithConfig(t, []string{"custom://callback"}, op.ApplicationTypeNative, nil, false), oidc.ResponseTypeIDToken}, - true, + false, }, { "implicit flow dev mode http ok", args{"http://registered.com/callback", - mock.NewClientWithConfig(t, []string{"http://registered.com/callback"}, op.ApplicationTypeNative, nil, true), + mock.NewClientWithConfig(t, []string{"http://registered.com/callback"}, op.ApplicationTypeUserAgent, nil, true), oidc.ResponseTypeIDToken}, false, }, @@ -481,3 +565,52 @@ func TestAuthResponse(t *testing.T) { }) } } + +func Test_LoopbackOrLocalhost(t *testing.T) { + type args struct { + url string + } + tests := []struct { + name string + args args + want bool + }{ + { + "v4 no port ok", + args{url: "http://127.0.0.1/test"}, + true, + }, + { + "v6 no port ok", + args{url: "http://[::1]/test"}, + true, + }, + { + "locahost no port ok", + args{url: "http://localhost/test"}, + true, + }, + { + "v4 with port ok", + args{url: "http://127.0.0.1:4200/test"}, + true, + }, + { + "v6 with port ok", + args{url: "http://[::1]:4200/test"}, + true, + }, + { + "localhost with port ok", + args{url: "http://localhost:4200/test"}, + true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if _, got := op.LoopbackOrLocalhost(tt.args.url); got != tt.want { + t.Errorf("loopbackOrLocalhost() = %v, want %v", got, tt.want) + } + }) + } +}