Merge pull request #96 from caos/redirect-uri

fix: add loopback for native apps redirect_uri

pkg/op/authrequest.go

@@ -3,7 +3,9 @@ package op
 import (
 	"context"
 	"fmt"
+	"net"
 	"net/http"
+	"net/url"
 	"strings"
 	"time"
 
@@ -157,32 +159,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() {
+	if strings.HasPrefix(uri, "http://") {
-		return nil
+		if client.DevMode() {
-	}
-	if strings.HasPrefix(uri, "https://") {
-		return nil
-	}
-	if responseType == oidc.ResponseTypeCode {
-		if strings.HasPrefix(uri, "http://") && IsConfidentialType(client) {
 			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 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.")
-			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.")
+}
+
+//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 := HTTPLoopbackOrLocalhost(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 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.")
-			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:")
+	}
+	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 := HTTPLoopbackOrLocalhost(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 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, hostName == "localhost" || net.ParseIP(hostName).IsLoopback()
 }
 
 //ValidateAuthReqResponseType validates the passed response_type to the registered response types

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",
+			"unregistered http fails",
-			args{"http://registered.com/callback",
+			args{"http://unregistered.com/callback",
-				mock.NewClientWithConfig(t, []string{"http://registered.com/callback"}, op.ApplicationTypeUserAgent, nil, false),
+				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 (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 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 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 loopback v6 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,
 		},
@@ -336,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",
@@ -355,12 +446,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 +572,62 @@ 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 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"},
+			true,
+		},
+		{
+			"v4 with port ok",
+			args{url: "http://127.0.0.1:4200/test"},
+			true,
+		},
+		{
+			"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"},
+			true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if _, got := op.HTTPLoopbackOrLocalhost(tt.args.url); got != tt.want {
+				t.Errorf("loopbackOrLocalhost() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}