Merge pull request #941 from ericchiang/fix-public-client-localhost

server: fix localhost redirect validation for public clients

Documentation/custom-scopes-claims-clients.md

@@ -67,6 +67,23 @@ The ID token claims will then include the following audience and authorized part
 }
 ``` 
 
+## Public clients
+
+Public clients are inspired by Google's [_"Installed Applications"_][installed-apps] and are meant to impose restrictions on applications that don't intend to keep their client secret private. Clients can be declared as public using the `public` config option.
+
+```yaml
+staticClients:
+- id: cli-app
+  public: true
+  name: 'CLI app'
+  secret: cli-app-secret
+```
+
+Instead of traditional redirect URIs, public clients are limited to either redirects that begin with "http://localhost" or a special "out-of-browser" URL "urn:ietf:wg:oauth:2.0:oob". The latter triggers dex to display the OAuth2 code in the browser, prompting the end user to manually copy it to their app. It's the client's responsibility to either create a screen or a prompt to receive the code, then perform a code exchange for a token response.
+
+When using the "out-of-browser" flow, an ID Token nonce is strongly recommended.
+
 [saml-connector]: saml-connector.md
 [core-claims]: https://openid.net/specs/openid-connect-core-1_0.html#IDToken
 [standard-claims]: https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims
+[installed-apps]: https://developers.google.com/api-client-library/python/auth/installed-app

server/oauth2.go

@@ -12,6 +12,7 @@ import (
 	"fmt"
 	"hash"
 	"io"
+	"net"
 	"net/http"
 	"net/url"
 	"strconv"
@@ -518,9 +519,18 @@ func validateRedirectURI(client storage.Client, redirectURI string) bool {
 	if redirectURI == redirectURIOOB {
 		return true
 	}
-	if !strings.HasPrefix(redirectURI, "http://localhost:") {
+
+	// verify that the host is of form "http://localhost:(port)(path)" or "http://localhost(path)"
+	u, err := url.Parse(redirectURI)
+	if err != nil {
+		return false
+	}
+	if u.Scheme != "http" {
 		return false
 	}
-	n, err := strconv.Atoi(strings.TrimPrefix(redirectURI, "https://localhost:"))
-	return err == nil && n <= 0
+	if u.Host == "localhost" {
+		return true
+	}
+	host, _, err := net.SplitHostPort(u.Host)
+	return err == nil && host == "localhost"
 }

server/oauth2_test.go

@@ -195,3 +195,67 @@ func TestAccessTokenHash(t *testing.T) {
 		t.Errorf("expected %q got %q", googleAccessTokenHash, atHash)
 	}
 }
+
+func TestValidRedirectURI(t *testing.T) {
+	tests := []struct {
+		client      storage.Client
+		redirectURI string
+		wantValid   bool
+	}{
+		{
+			client: storage.Client{
+				RedirectURIs: []string{"http://foo.com/bar"},
+			},
+			redirectURI: "http://foo.com/bar",
+			wantValid:   true,
+		},
+		{
+			client: storage.Client{
+				RedirectURIs: []string{"http://foo.com/bar"},
+			},
+			redirectURI: "http://foo.com/bar/baz",
+		},
+		{
+			client: storage.Client{
+				Public: true,
+			},
+			redirectURI: "urn:ietf:wg:oauth:2.0:oob",
+			wantValid:   true,
+		},
+		{
+			client: storage.Client{
+				Public: true,
+			},
+			redirectURI: "http://localhost:8080/",
+			wantValid:   true,
+		},
+		{
+			client: storage.Client{
+				Public: true,
+			},
+			redirectURI: "http://localhost:991/bar",
+			wantValid:   true,
+		},
+		{
+			client: storage.Client{
+				Public: true,
+			},
+			redirectURI: "http://localhost",
+			wantValid:   true,
+		},
+		{
+			client: storage.Client{
+				Public: true,
+			},
+			redirectURI: "http://localhost.localhost:8080/",
+			wantValid:   false,
+		},
+	}
+	for _, test := range tests {
+		got := validateRedirectURI(test.client, test.redirectURI)
+		if got != test.wantValid {
+			t.Errorf("client=%#v, redirectURI=%q, wanted valid=%t, got=%t",
+				test.client, test.redirectURI, test.wantValid, got)
+		}
+	}
+}