diff --git a/server/deviceflowhandlers.go b/server/deviceflowhandlers.go index 3cbfbf16..ec5fb52b 100644 --- a/server/deviceflowhandlers.go +++ b/server/deviceflowhandlers.go @@ -11,8 +11,6 @@ import ( "strings" "time" - "golang.org/x/net/html" - "github.com/dexidp/dex/storage" ) @@ -300,9 +298,11 @@ func (s *Server) handleDeviceCallback(w http.ResponseWriter, r *http.Request) { // Authorization redirect callback from OAuth2 auth flow. if errMsg := r.FormValue("error"); errMsg != "" { - // escape the message to prevent cross-site scripting - msg := html.EscapeString(errMsg + ": " + r.FormValue("error_description")) - http.Error(w, msg, http.StatusBadRequest) + // Log the error details but don't expose them to the user + s.logger.ErrorContext(r.Context(), "OAuth2 authorization error", + "error", errMsg, + "error_description", r.FormValue("error_description")) + s.renderError(r, w, http.StatusBadRequest, "Authorization failed. Please try again.") return } @@ -392,7 +392,8 @@ func (s *Server) handleDeviceCallback(w http.ResponseWriter, r *http.Request) { } default: - http.Error(w, fmt.Sprintf("method not implemented: %s", r.Method), http.StatusBadRequest) + s.logger.ErrorContext(r.Context(), "unsupported method in device callback", "method", r.Method) + s.renderError(r, w, http.StatusBadRequest, ErrMsgMethodNotAllowed) return } } diff --git a/server/deviceflowhandlers_test.go b/server/deviceflowhandlers_test.go index 3f3ea81e..ec7bf29d 100644 --- a/server/deviceflowhandlers_test.go +++ b/server/deviceflowhandlers_test.go @@ -222,8 +222,9 @@ func TestDeviceCallback(t *testing.T) { code: "somecode", error: "Error Condition", }, - expectedResponseCode: http.StatusBadRequest, - expectedServerResponse: "Error Condition: \n", + expectedResponseCode: http.StatusBadRequest, + // Note: Error details should NOT be displayed to user anymore. + // Instead, a safe generic message is shown. }, { testName: "Expired Auth Code", @@ -352,8 +353,9 @@ func TestDeviceCallback(t *testing.T) { code: "somecode", error: "", }, - expectedResponseCode: http.StatusBadRequest, - expectedServerResponse: "<script>console.log(window);</script>: \n", + expectedResponseCode: http.StatusBadRequest, + // Note: XSS data should NOT be displayed to user anymore. + // Instead, a safe generic message is shown. }, } for _, tc := range tests { @@ -413,6 +415,29 @@ func TestDeviceCallback(t *testing.T) { t.Errorf("%s: Unexpected Response. Expected %q got %q", tc.testName, tc.expectedServerResponse, result) } } + + // Special check for error message safety tests + if tc.testName == "Prevent cross-site scripting" || tc.testName == "Error During Authorization" { + result, _ := io.ReadAll(rr.Body) + responseBody := string(result) + + // Error details should NOT be present in the response (for security) + if tc.testName == "Prevent cross-site scripting" { + if strings.Contains(responseBody, "