Repository: dexidp/dex
Pull request #1822 (2020-10-05 18:22:59)

Allow public clients (e.g. SPAs using implicit flow or PKCE) to have redirect URLs other than localhost
Relates to:
* https://github.com/dexidp/dex/pull/1722 (also allow usual IP addresses)
* https://github.com/dexidp/dex/pull/1784 (PKCE)

heidemn-faro: @tkleczek I saw your comment in an e-mail, can't find it here any more:
> I am curious - why don't you configure the app as confidential client if it is a web app and can keep the client secret secure?

What about SPAs without server component, hosted as static websites?
These can use Implicit Flow, or Auth Code + PKCE, and don't need a secret then.

Read more here: https://developer.okta.com/blog/2019/08/22/okta-authjs-pkce
tkleczek: @heidemn-faro Yes, I realized this soon after I've written my comment and therefore deleted it.
logikone: Hey, thanks for this! Just wanted to ask quickly what the timeline is on getting this merged and releasing v2.26.0? Thanks in advance!
sagikazarmark: I'll try to get this merged and released this week.
logikone: > I'll try to get this merged and released this week.

@sagikazarmark awesome, thank you sir! 

Patch:
diff --git a/server/oauth2.go b/server/oauth2.go
index 79e54c32..18146d61 100644
--- a/server/oauth2.go
+++ b/server/oauth2.go
@@ -588,12 +588,16 @@ func (s *Server) validateCrossClientTrust(clientID, peerID string) (trusted bool
 }
 
 func validateRedirectURI(client storage.Client, redirectURI string) bool {
-	if !client.Public {
-		for _, uri := range client.RedirectURIs {
-			if redirectURI == uri {
-				return true
-			}
+	// Allow named RedirectURIs for both public and non-public clients.
+	// This is required make PKCE-enabled web apps work, when configured as public clients.
+	for _, uri := range client.RedirectURIs {
+		if redirectURI == uri {
+			return true
 		}
+	}
+	// 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
 	}
 


Test patch:
diff --git a/server/oauth2_test.go b/server/oauth2_test.go
index 8db9ea59..c926a3e1 100644
--- a/server/oauth2_test.go
+++ b/server/oauth2_test.go
@@ -340,7 +340,9 @@ func TestValidRedirectURI(t *testing.T) {
 				RedirectURIs: []string{"http://foo.com/bar"},
 			},
 			redirectURI: "http://foo.com/bar/baz",
+			wantValid:   false,
 		},
+		// These special desktop + device + localhost URIs are allowed by default.
 		{
 			client: storage.Client{
 				Public: true,
@@ -348,6 +350,13 @@ func TestValidRedirectURI(t *testing.T) {
 			redirectURI: "urn:ietf:wg:oauth:2.0:oob",
 			wantValid:   true,
 		},
+		{
+			client: storage.Client{
+				Public: true,
+			},
+			redirectURI: "/device/callback",
+			wantValid:   true,
+		},
 		{
 			client: storage.Client{
 				Public: true,
@@ -369,6 +378,113 @@ func TestValidRedirectURI(t *testing.T) {
 			redirectURI: "http://localhost",
 			wantValid:   true,
 		},
+		// Both Public + RedirectURIs configured: Could e.g. be a PKCE-enabled web app.
+		{
+			client: storage.Client{
+				Public:       true,
+				RedirectURIs: []string{"http://foo.com/bar"},
+			},
+			redirectURI: "http://foo.com/bar",
+			wantValid:   true,
+		},
+		{
+			client: storage.Client{
+				Public:       true,
+				RedirectURIs: []string{"http://foo.com/bar"},
+			},
+			redirectURI: "http://foo.com/bar/baz",
+			wantValid:   false,
+		},
+		// 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:   false,
+		},
+		{
+			client: storage.Client{
+				Public:       true,
+				RedirectURIs: []string{"http://foo.com/bar"},
+			},
+			redirectURI: "/device/callback",
+			wantValid:   false,
+		},
+		{
+			client: storage.Client{
+				Public:       true,
+				RedirectURIs: []string{"http://foo.com/bar"},
+			},
+			redirectURI: "http://localhost:8080/",
+			wantValid:   false,
+		},
+		{
+			client: storage.Client{
+				Public:       true,
+				RedirectURIs: []string{"http://foo.com/bar"},
+			},
+			redirectURI: "http://localhost:991/bar",
+			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.
+		{
+			client: storage.Client{
+				Public: true,
+			},
+			redirectURI: "http://foo.com/bar",
+			wantValid:   false,
+		},
 		{
 			client: storage.Client{
 				Public: true,