Browse Source

fix(deviceflow): update redirect URIs to use absolute paths for non-root URLs (#4597)

Signed-off-by: maksim.nabokikh <max.nabokih@gmail.com>
pull/4602/head
Maksim Nabokikh 2 weeks ago committed by GitHub
parent
commit
a70f592589
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 4
      server/deviceflowhandlers.go
  2. 38
      server/deviceflowhandlers_test.go
  3. 4
      server/server_test.go

4
server/deviceflowhandlers.go

@ -431,7 +431,7 @@ func (s *Server) verifyUserCode(w http.ResponseWriter, r *http.Request) {
} }
// Redirect to Dex Auth Endpoint // Redirect to Dex Auth Endpoint
authURL := path.Join(s.issuerURL.Path, "/auth") authURL := s.absURL("/auth")
u, err := url.Parse(authURL) u, err := url.Parse(authURL)
if err != nil { if err != nil {
s.renderError(r, w, http.StatusInternalServerError, "Invalid auth URI.") s.renderError(r, w, http.StatusInternalServerError, "Invalid auth URI.")
@ -442,7 +442,7 @@ func (s *Server) verifyUserCode(w http.ResponseWriter, r *http.Request) {
q.Set("client_secret", deviceRequest.ClientSecret) q.Set("client_secret", deviceRequest.ClientSecret)
q.Set("state", deviceRequest.UserCode) q.Set("state", deviceRequest.UserCode)
q.Set("response_type", "code") q.Set("response_type", "code")
q.Set("redirect_uri", "/device/callback") q.Set("redirect_uri", s.absPath(deviceCallbackURI))
q.Set("scope", strings.Join(deviceRequest.Scopes, " ")) q.Set("scope", strings.Join(deviceRequest.Scopes, " "))
u.RawQuery = q.Encode() u.RawQuery = q.Encode()

38
server/deviceflowhandlers_test.go

@ -364,7 +364,7 @@ func TestDeviceCallback(t *testing.T) {
// Setup a dex server. // Setup a dex server.
httpServer, s := newTestServer(t, func(c *Config) { httpServer, s := newTestServer(t, func(c *Config) {
// c.Issuer = c.Issuer + "/non-root-path" c.Issuer = c.Issuer + "/non-root-path"
c.Now = now c.Now = now
}) })
defer httpServer.Close() defer httpServer.Close()
@ -752,7 +752,8 @@ func TestVerifyCodeResponse(t *testing.T) {
testDeviceRequest storage.DeviceRequest testDeviceRequest storage.DeviceRequest
userCode string userCode string
expectedResponseCode int expectedResponseCode int
expectedRedirectPath string expectedAuthPath string
shouldRedirectToAuth bool
}{ }{
{ {
testName: "Unknown user code", testName: "Unknown user code",
@ -765,7 +766,6 @@ func TestVerifyCodeResponse(t *testing.T) {
}, },
userCode: "CODE-TEST", userCode: "CODE-TEST",
expectedResponseCode: http.StatusBadRequest, expectedResponseCode: http.StatusBadRequest,
expectedRedirectPath: "",
}, },
{ {
testName: "Expired user code", testName: "Expired user code",
@ -778,7 +778,6 @@ func TestVerifyCodeResponse(t *testing.T) {
}, },
userCode: "ABCD-WXYZ", userCode: "ABCD-WXYZ",
expectedResponseCode: http.StatusBadRequest, expectedResponseCode: http.StatusBadRequest,
expectedRedirectPath: "",
}, },
{ {
testName: "No user code", testName: "No user code",
@ -791,10 +790,9 @@ func TestVerifyCodeResponse(t *testing.T) {
}, },
userCode: "", userCode: "",
expectedResponseCode: http.StatusBadRequest, expectedResponseCode: http.StatusBadRequest,
expectedRedirectPath: "",
}, },
{ {
testName: "Valid user code, expect redirect to auth endpoint", testName: "Valid user code, expect redirect to auth endpoint with device callback",
testDeviceRequest: storage.DeviceRequest{ testDeviceRequest: storage.DeviceRequest{
UserCode: "ABCD-WXYZ", UserCode: "ABCD-WXYZ",
DeviceCode: "f00bar", DeviceCode: "f00bar",
@ -804,7 +802,8 @@ func TestVerifyCodeResponse(t *testing.T) {
}, },
userCode: "ABCD-WXYZ", userCode: "ABCD-WXYZ",
expectedResponseCode: http.StatusFound, expectedResponseCode: http.StatusFound,
expectedRedirectPath: "/auth", expectedAuthPath: "/auth",
shouldRedirectToAuth: true,
}, },
} }
for _, tc := range tests { for _, tc := range tests {
@ -839,15 +838,24 @@ func TestVerifyCodeResponse(t *testing.T) {
t.Errorf("Unexpected Response Type. Expected %v got %v", tc.expectedResponseCode, rr.Code) t.Errorf("Unexpected Response Type. Expected %v got %v", tc.expectedResponseCode, rr.Code)
} }
u, err = url.Parse(s.issuerURL.String())
if err != nil {
t.Errorf("Could not parse issuer URL %v", err)
}
u.Path = path.Join(u.Path, tc.expectedRedirectPath)
location := rr.Header().Get("Location") location := rr.Header().Get("Location")
if rr.Code == http.StatusFound && !strings.HasPrefix(location, u.Path) { if rr.Code == http.StatusFound && tc.shouldRedirectToAuth {
t.Errorf("Invalid Redirect. Expected %v got %v", u.Path, location) // Parse the redirect location
redirectURL, err := url.Parse(location)
if err != nil {
t.Errorf("Could not parse redirect URL: %v", err)
return
}
// Check that the redirect path contains /auth
if !strings.Contains(redirectURL.Path, tc.expectedAuthPath) {
t.Errorf("Invalid Redirect Path. Expected to contain %q got %q", tc.expectedAuthPath, redirectURL.Path)
}
// Check that redirect_uri parameter contains /device/callback
if !strings.Contains(location, "redirect_uri=%2Fnon-root-path%2Fdevice%2Fcallback") {
t.Errorf("Invalid redirect_uri parameter. Expected to contain /device/callback (URL encoded), got %v", location)
}
} }
}) })
} }

4
server/server_test.go

@ -1640,7 +1640,7 @@ func TestOAuth2DeviceFlow(t *testing.T) {
// Add the Clients to the test server // Add the Clients to the test server
client := storage.Client{ client := storage.Client{
ID: clientID, ID: clientID,
RedirectURIs: []string{deviceCallbackURI}, RedirectURIs: []string{s.absPath(deviceCallbackURI)},
Public: true, Public: true,
} }
if err := s.storage.CreateClient(ctx, client); err != nil { if err := s.storage.CreateClient(ctx, client); err != nil {
@ -1751,7 +1751,7 @@ func TestOAuth2DeviceFlow(t *testing.T) {
ClientSecret: client.Secret, ClientSecret: client.Secret,
Endpoint: p.Endpoint(), Endpoint: p.Endpoint(),
Scopes: requestedScopes, Scopes: requestedScopes,
RedirectURL: deviceCallbackURI, RedirectURL: s.absURL(deviceCallbackURI),
} }
if len(tc.scopes) != 0 { if len(tc.scopes) != 0 {
oauth2Config.Scopes = tc.scopes oauth2Config.Scopes = tc.scopes

Loading…
Cancel
Save