No longer allow desktop/mobile redirect URIs implicitly if RedirectUR…

…Is is set Signed-off-by: Martin Heide <martin.heide@faro.com>
dexidp · Nov 2, 2020 · 162073b · 162073b
1 parent c15e288
commit 162073b
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 7 deletions.
diff --git a/server/oauth2.go b/server/oauth2.go
@@ -595,8 +595,9 @@ func validateRedirectURI(client storage.Client, redirectURI string) bool {
 			return true
 		}
 	}
-	// For non-public clients, only named RedirectURIs are allowed.
-	if !client.Public {
+	// For non-public clients or when RedirectURIs is set, we allow only explicitly named RedirectURIs.
+	// Otherwise, we check below for special URIs used for desktop or mobile apps.
+	if !client.Public || len(client.RedirectURIs) > 0 {
 		return false
 	}
 

diff --git a/server/oauth2_test.go b/server/oauth2_test.go
@@ -395,45 +395,86 @@ func TestValidRedirectURI(t *testing.T) {
 			redirectURI: "http://foo.com/bar/baz",
 			wantValid:   false,
 		},
-		// These special desktop + device + localhost URIs are allowed even when RedirectURIs is non-empty.
+		// These special desktop + device + localhost URIs are not allowed implicitly when RedirectURIs is non-empty.
 		{
 			client: storage.Client{
 				Public:       true,
 				RedirectURIs: []string{"http://foo.com/bar"},
 			},
 			redirectURI: "urn:ietf:wg:oauth:2.0:oob",
-			wantValid:   true,
+			wantValid:   false,
 		},
 		{
 			client: storage.Client{
 				Public:       true,
 				RedirectURIs: []string{"http://foo.com/bar"},
 			},
 			redirectURI: "/device/callback",
-			wantValid:   true,
+			wantValid:   false,
 		},
 		{
 			client: storage.Client{
 				Public:       true,
 				RedirectURIs: []string{"http://foo.com/bar"},
 			},
 			redirectURI: "http://localhost:8080/",
-			wantValid:   true,
+			wantValid:   false,
 		},
 		{
 			client: storage.Client{
 				Public:       true,
 				RedirectURIs: []string{"http://foo.com/bar"},
 			},
 			redirectURI: "http://localhost:991/bar",
-			wantValid:   true,
+			wantValid:   false,
 		},
 		{
 			client: storage.Client{
 				Public:       true,
 				RedirectURIs: []string{"http://foo.com/bar"},
 			},
 			redirectURI: "http://localhost",
+			wantValid:   false,
+		},
+		// These special desktop + device + localhost URIs can still be specified explicitly.
+		{
+			client: storage.Client{
+				Public:       true,
+				RedirectURIs: []string{"http://foo.com/bar", "urn:ietf:wg:oauth:2.0:oob"},
+			},
+			redirectURI: "urn:ietf:wg:oauth:2.0:oob",
+			wantValid:   true,
+		},
+		{
+			client: storage.Client{
+				Public:       true,
+				RedirectURIs: []string{"http://foo.com/bar", "/device/callback"},
+			},
+			redirectURI: "/device/callback",
+			wantValid:   true,
+		},
+		{
+			client: storage.Client{
+				Public:       true,
+				RedirectURIs: []string{"http://foo.com/bar", "http://localhost:8080/"},
+			},
+			redirectURI: "http://localhost:8080/",
+			wantValid:   true,
+		},
+		{
+			client: storage.Client{
+				Public:       true,
+				RedirectURIs: []string{"http://foo.com/bar", "http://localhost:991/bar"},
+			},
+			redirectURI: "http://localhost:991/bar",
+			wantValid:   true,
+		},
+		{
+			client: storage.Client{
+				Public:       true,
+				RedirectURIs: []string{"http://foo.com/bar", "http://localhost"},
+			},
+			redirectURI: "http://localhost",
 			wantValid:   true,
 		},
 		// Non-localhost URIs are not allowed implicitly.