From 9882ea453f2537468b3f88cca2138e7a2de19369 Mon Sep 17 00:00:00 2001 From: justin-slowik Date: Wed, 13 May 2020 15:38:43 -0400 Subject: [PATCH] better support for /device/callback redirect uris with public clients. Signed-off-by: justin-slowik --- examples/config-dev.yaml | 3 ++- server/deviceflowhandlers.go | 2 +- server/deviceflowhandlers_test.go | 8 ++++---- server/oauth2.go | 9 ++++++++- server/server.go | 2 +- server/server_test.go | 6 +++--- 6 files changed, 19 insertions(+), 11 deletions(-) diff --git a/examples/config-dev.yaml b/examples/config-dev.yaml index 27e37589..75126692 100644 --- a/examples/config-dev.yaml +++ b/examples/config-dev.yaml @@ -98,8 +98,9 @@ staticClients: secret: ZXhhbXBsZS1hcHAtc2VjcmV0 # - id: example-device-client # redirectURIs: -# - /dex/device/callback +# - /device/callback # name: 'Static Client for Device Flow' +# public: true connectors: - type: mockCallback id: mock diff --git a/server/deviceflowhandlers.go b/server/deviceflowhandlers.go index 39ead503..d7bb11c5 100644 --- a/server/deviceflowhandlers.go +++ b/server/deviceflowhandlers.go @@ -374,7 +374,7 @@ func (s *Server) verifyUserCode(w http.ResponseWriter, r *http.Request) { q.Set("client_secret", deviceRequest.ClientSecret) q.Set("state", deviceRequest.UserCode) q.Set("response_type", "code") - q.Set("redirect_uri", path.Join(s.issuerURL.Path, "/device/callback")) + q.Set("redirect_uri", "/device/callback") q.Set("scope", strings.Join(deviceRequest.Scopes, " ")) u.RawQuery = q.Encode() diff --git a/server/deviceflowhandlers_test.go b/server/deviceflowhandlers_test.go index 35306971..6d1ed7e3 100644 --- a/server/deviceflowhandlers_test.go +++ b/server/deviceflowhandlers_test.go @@ -128,7 +128,7 @@ func TestDeviceCallback(t *testing.T) { baseAuthCode := storage.AuthCode{ ID: "somecode", ClientID: "testclient", - RedirectURI: "/device/callback", + RedirectURI: deviceCallbackURI, Nonce: "", Scopes: []string{"openid", "profile", "email"}, ConnectorID: "mock", @@ -194,7 +194,7 @@ func TestDeviceCallback(t *testing.T) { testAuthCode: storage.AuthCode{ ID: "somecode", ClientID: "testclient", - RedirectURI: "/device/callback", + RedirectURI: deviceCallbackURI, Nonce: "", Scopes: []string{"openid", "profile", "email"}, ConnectorID: "pic", @@ -210,7 +210,7 @@ func TestDeviceCallback(t *testing.T) { testAuthCode: storage.AuthCode{ ID: "somecode", ClientID: "testclient", - RedirectURI: "/device/callback", + RedirectURI: deviceCallbackURI, Nonce: "", Scopes: []string{"openid", "profile", "email"}, ConnectorID: "pic", @@ -336,7 +336,7 @@ func TestDeviceCallback(t *testing.T) { client := storage.Client{ ID: "testclient", Secret: "", - RedirectURIs: []string{"/device/callback"}, + RedirectURIs: []string{deviceCallbackURI}, } if err := s.storage.CreateClient(client); err != nil { t.Fatalf("failed to create client: %v", err) diff --git a/server/oauth2.go b/server/oauth2.go index 59e132bc..09402c91 100644 --- a/server/oauth2.go +++ b/server/oauth2.go @@ -114,6 +114,10 @@ const ( scopeCrossClientPrefix = "audience:server:client_id:" ) +const ( + deviceCallbackURI = "/device/callback" +) + const ( redirectURIOOB = "urn:ietf:wg:oauth:2.0:oob" ) @@ -433,6 +437,9 @@ func (s *Server) parseAuthorizationRequest(r *http.Request) (*storage.AuthReques description := fmt.Sprintf("Unregistered redirect_uri (%q).", redirectURI) return nil, &authErr{"", "", errInvalidRequest, description} } + if redirectURI == deviceCallbackURI && client.Public { + redirectURI = s.issuerURL.Path + deviceCallbackURI + } // From here on out, we want to redirect back to the client with an error. newErr := func(typ, format string, a ...interface{}) *authErr { @@ -574,7 +581,7 @@ func validateRedirectURI(client storage.Client, redirectURI string) bool { return false } - if redirectURI == redirectURIOOB { + if redirectURI == redirectURIOOB || redirectURI == deviceCallbackURI{ return true } diff --git a/server/server.go b/server/server.go index 661fc835..f4d139d1 100644 --- a/server/server.go +++ b/server/server.go @@ -309,7 +309,7 @@ func newServer(ctx context.Context, c Config, rotationStrategy rotationStrategy) handleFunc("/device/auth/verify_code", s.verifyUserCode) handleFunc("/device/code", s.handleDeviceCode) handleFunc("/device/token", s.handleDeviceToken) - handleFunc("/device/callback", s.handleDeviceCallback) + handleFunc(deviceCallbackURI, s.handleDeviceCallback) r.HandleFunc(path.Join(issuerURL.Path, "/callback"), func(w http.ResponseWriter, r *http.Request) { // Strip the X-Remote-* headers to prevent security issues on // misconfigured authproxy connector setups. diff --git a/server/server_test.go b/server/server_test.go index edc00832..7f0dc4dc 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -1317,8 +1317,8 @@ func TestOAuth2DeviceFlow(t *testing.T) { //Add the Clients to the test server client := storage.Client{ ID: clientID, - //Secret: "testclientsecret", - RedirectURIs: []string{"/non-root-path/device/callback"}, + RedirectURIs: []string{deviceCallbackURI}, + Public: true, } if err := s.storage.CreateClient(client); err != nil { t.Fatalf("failed to create client: %v", err) @@ -1421,7 +1421,7 @@ func TestOAuth2DeviceFlow(t *testing.T) { ClientSecret: client.Secret, Endpoint: p.Endpoint(), Scopes: requestedScopes, - RedirectURL: "/non-root-path/device/callback", + RedirectURL: deviceCallbackURI, } if len(tc.scopes) != 0 { oauth2Config.Scopes = tc.scopes