From c2cff930743b9e90fdac76564c6cbd1e2bad531f Mon Sep 17 00:00:00 2001 From: cmaddox5 Date: Tue, 15 Oct 2024 13:27:53 -0400 Subject: [PATCH 1/8] Return 403 for api endpoints instead of redirecting. --- lib/screenplay_web/auth_manager/error_handler.ex | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/screenplay_web/auth_manager/error_handler.ex b/lib/screenplay_web/auth_manager/error_handler.ex index 266b0b59..eb80b70c 100644 --- a/lib/screenplay_web/auth_manager/error_handler.ex +++ b/lib/screenplay_web/auth_manager/error_handler.ex @@ -9,8 +9,12 @@ defmodule ScreenplayWeb.AuthManager.ErrorHandler do @impl Guardian.Plug.ErrorHandler def auth_error(conn, error, _opts) do - auth_params = auth_params_for_error(error) - Phoenix.Controller.redirect(conn, to: ~p"/auth/keycloak?#{auth_params}") + if Plug.Conn.get_session(conn, :previous_path) =~ "api" do + Plug.Conn.send_resp(conn, 403, "Session expired") + else + auth_params = auth_params_for_error(error) + Phoenix.Controller.redirect(conn, to: ~p"/auth/keycloak?#{auth_params}") + end end def auth_params_for_error({:invalid_token, {:auth_expired, sub}}) do From b5d9316dc97f965f8e8dd67e6126a31e9c1b16ae Mon Sep 17 00:00:00 2001 From: cmaddox5 Date: Tue, 15 Oct 2024 13:29:18 -0400 Subject: [PATCH 2/8] Stop interval and show modal when API fetches fail. --- assets/js/components/Dashboard/Dashboard.tsx | 62 +++++++++++++++----- assets/js/hooks/useInterval.tsx | 2 +- assets/js/utils/api.ts | 6 +- 3 files changed, 52 insertions(+), 18 deletions(-) diff --git a/assets/js/components/Dashboard/Dashboard.tsx b/assets/js/components/Dashboard/Dashboard.tsx index 83b214ea..f4548273 100644 --- a/assets/js/components/Dashboard/Dashboard.tsx +++ b/assets/js/components/Dashboard/Dashboard.tsx @@ -13,6 +13,7 @@ import AlertBanner from "Components/AlertBanner"; import LinkCopiedToast from "Components/LinkCopiedToast"; import ActionOutcomeToast from "Components/ActionOutcomeToast"; import { useLocation } from "react-router-dom"; +import { Button, Modal } from "react-bootstrap"; const Dashboard: ComponentType = () => { const { @@ -24,6 +25,8 @@ const Dashboard: ComponentType = () => { } = useScreenplayContext(); const dispatch = useScreenplayDispatchContext(); const [bannerDone, setBannerDone] = useState(false); + const [isAlertsIntervalRunning, setIsAlertsIntervalRunning] = useState(true); + const [showModal, setShowModal] = useState(false); useEffect(() => { fetchAlerts().then( @@ -52,23 +55,31 @@ const Dashboard: ComponentType = () => { }, []); // eslint-disable-line react-hooks/exhaustive-deps // Fetch alerts every 4 seconds. - useInterval(() => { - fetchAlerts().then( - ({ - all_alert_ids: allAPIalertIds, - alerts: newAlerts, - screens_by_alert: screensByAlertMap, - }) => { - findAndSetBannerAlert(alerts, newAlerts); - dispatch({ - type: "SET_ALERTS", - alerts: newAlerts, - allAPIAlertIds: allAPIalertIds, - screensByAlertMap: screensByAlertMap, + useInterval( + () => { + fetchAlerts() + .then( + ({ + all_alert_ids: allAPIalertIds, + alerts: newAlerts, + screens_by_alert: screensByAlertMap, + }) => { + findAndSetBannerAlert(alerts, newAlerts); + dispatch({ + type: "SET_ALERTS", + alerts: newAlerts, + allAPIAlertIds: allAPIalertIds, + screensByAlertMap: screensByAlertMap, + }); + }, + ) + .catch(() => { + setIsAlertsIntervalRunning(false); + setShowModal(true); }); - }, - ); - }, 4000); + }, + isAlertsIntervalRunning ? 4000 : null, + ); const findAndSetBannerAlert = (oldAlerts: Alert[], newAlerts: Alert[]) => { const now = new Date(); @@ -187,6 +198,25 @@ const Dashboard: ComponentType = () => { )} + + + Your session has expired, please refresh your browser. + + + + + + ); }; diff --git a/assets/js/hooks/useInterval.tsx b/assets/js/hooks/useInterval.tsx index 95d70119..4ad1e8d4 100644 --- a/assets/js/hooks/useInterval.tsx +++ b/assets/js/hooks/useInterval.tsx @@ -6,7 +6,7 @@ function noop() { // do nothing } -export function useInterval(callback: () => void, delay: number) { +export function useInterval(callback: () => void, delay: number | null) { const savedCallback = useRef<() => void>(noop); // Remember the latest callback. diff --git a/assets/js/utils/api.ts b/assets/js/utils/api.ts index 950eef3a..0ccb292a 100644 --- a/assets/js/utils/api.ts +++ b/assets/js/utils/api.ts @@ -19,7 +19,11 @@ interface AlertsResponse { export const fetchAlerts = async (): Promise => { const response = await fetch("/api/alerts"); - return await response.json(); + if (response.status === 200) { + return await response.json(); + } else { + throw response; + } }; export const fetchActiveAndFutureAlerts = async (): Promise => { From eca65d00379574cc53539d728c063135030a118f Mon Sep 17 00:00:00 2001 From: cmaddox5 Date: Tue, 15 Oct 2024 13:47:45 -0400 Subject: [PATCH 3/8] Created new ErrorModal component. --- assets/css/error-modal.scss | 40 +++++++++++++++++ assets/css/screenplay.scss | 1 + assets/css/workflow.scss | 43 ------------------ assets/js/components/Dashboard/Dashboard.tsx | 28 ++++-------- assets/js/components/Dashboard/ErrorModal.tsx | 44 +++++++++++++++++++ .../Workflows/GlEink/GlEinkWorkflow.tsx | 37 ++++------------ 6 files changed, 103 insertions(+), 90 deletions(-) create mode 100644 assets/css/error-modal.scss create mode 100644 assets/js/components/Dashboard/ErrorModal.tsx diff --git a/assets/css/error-modal.scss b/assets/css/error-modal.scss new file mode 100644 index 00000000..aff314b0 --- /dev/null +++ b/assets/css/error-modal.scss @@ -0,0 +1,40 @@ +.modal.error-modal { + color: $text-primary; + .modal-dialog { + margin-top: 362px; + } + + .modal-content { + background-color: $cool-gray-20; + + .modal-header { + border-bottom: none; + + .btn-close:hover { + background-color: transparent; + } + } + + .modal-footer { + background-color: $cool-gray-20; + border-top: none; + height: 62px; + padding: 0 12px 0 0; + + .btn { + height: 38px; + } + + .error-modal__refresh-button { + background-color: $button-primary; + color: $button-secondary; + } + + .error-modal__cancel-button { + background-color: $cool-gray-20; + color: #c1e4ff; + border: transparent; + } + } + } +} diff --git a/assets/css/screenplay.scss b/assets/css/screenplay.scss index febcdbde..8c6b3746 100644 --- a/assets/css/screenplay.scss +++ b/assets/css/screenplay.scss @@ -44,6 +44,7 @@ $form-feedback-invalid-color: $text-error; @import "search-bar.scss"; @import "dashboard/picker.scss"; @import "dashboard/error-toast.scss"; +@import "error-modal.scss"; html { font-size: 16px; diff --git a/assets/css/workflow.scss b/assets/css/workflow.scss index 54d79270..a5771dda 100644 --- a/assets/css/workflow.scss +++ b/assets/css/workflow.scss @@ -56,46 +56,3 @@ padding-left: 8px; } } - -.modal { - &.error-modal { - color: $text-primary; - .modal-dialog { - margin-top: 362px; - } - - .modal-content { - background-color: $cool-gray-20; - - .modal-header { - border-bottom: none; - - .btn-close:hover { - background-color: transparent; - } - } - - .modal-footer { - background-color: $cool-gray-20; - border-top: none; - height: 62px; - padding: 0 12px 0 0; - - .btn { - height: 38px; - } - - .error-modal__refresh-button { - background-color: $button-primary; - color: $button-secondary; - } - - .error-modal__cancel-button { - background-color: $cool-gray-20; - color: #c1e4ff; - border: transparent; - } - } - } - } -} diff --git a/assets/js/components/Dashboard/Dashboard.tsx b/assets/js/components/Dashboard/Dashboard.tsx index f4548273..d7995e76 100644 --- a/assets/js/components/Dashboard/Dashboard.tsx +++ b/assets/js/components/Dashboard/Dashboard.tsx @@ -14,6 +14,7 @@ import LinkCopiedToast from "Components/LinkCopiedToast"; import ActionOutcomeToast from "Components/ActionOutcomeToast"; import { useLocation } from "react-router-dom"; import { Button, Modal } from "react-bootstrap"; +import ErrorModal from "Components/ErrorModal"; const Dashboard: ComponentType = () => { const { @@ -198,25 +199,14 @@ const Dashboard: ComponentType = () => { )} - - - Your session has expired, please refresh your browser. - - - - - - + setShowModal(false)} + errorMessage="Your session has expired, please refresh your browser." + confirmButtonLabel="Refresh now" + onConfirm={() => window.location.reload()} + /> ); }; diff --git a/assets/js/components/Dashboard/ErrorModal.tsx b/assets/js/components/Dashboard/ErrorModal.tsx new file mode 100644 index 00000000..986dfdbc --- /dev/null +++ b/assets/js/components/Dashboard/ErrorModal.tsx @@ -0,0 +1,44 @@ +import React from "react"; +import { Button, Modal } from "react-bootstrap"; + +interface ErrorModalProps { + title: string; + showErrorModal: boolean; + onHide: () => void; + errorMessage: string; + confirmButtonLabel: string; + onConfirm: () => void; +} + +const ErrorModal = ({ + title, + showErrorModal, + onHide, + errorMessage, + confirmButtonLabel, + onConfirm, +}: ErrorModalProps) => { + return ( + + + {title && {title}} + + {errorMessage} + + + + + + ); +}; + +export default ErrorModal; diff --git a/assets/js/components/Dashboard/PermanentConfiguration/Workflows/GlEink/GlEinkWorkflow.tsx b/assets/js/components/Dashboard/PermanentConfiguration/Workflows/GlEink/GlEinkWorkflow.tsx index 45ba071f..e8811aac 100644 --- a/assets/js/components/Dashboard/PermanentConfiguration/Workflows/GlEink/GlEinkWorkflow.tsx +++ b/assets/js/components/Dashboard/PermanentConfiguration/Workflows/GlEink/GlEinkWorkflow.tsx @@ -18,6 +18,7 @@ import { } from "Hooks/useScreenplayContext"; import { putPendingScreens } from "Utils/api"; import { useScreenplayContext } from "Hooks/useScreenplayContext"; +import ErrorModal from "Components/ErrorModal"; interface EditNavigationState { place_id: string; @@ -275,35 +276,15 @@ const GlEinkWorkflow: ComponentType = () => { }; layout = ( <> - setShowErrorModal(false)} - > - - - Someone else is configuring these screens - - - - In order not to overwrite each others work, please refresh your - browser and fill-out the form again. - - - - - - + errorMessage="In order not to overwrite each others work, please refresh your + browser and fill-out the form again." + confirmButtonLabel="Refresh now" + onConfirm={() => window.location.reload()} + /> Date: Tue, 15 Oct 2024 13:47:58 -0400 Subject: [PATCH 4/8] Only show modal on 403. --- assets/js/components/Dashboard/Dashboard.tsx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/assets/js/components/Dashboard/Dashboard.tsx b/assets/js/components/Dashboard/Dashboard.tsx index d7995e76..1a92f763 100644 --- a/assets/js/components/Dashboard/Dashboard.tsx +++ b/assets/js/components/Dashboard/Dashboard.tsx @@ -74,9 +74,13 @@ const Dashboard: ComponentType = () => { }); }, ) - .catch(() => { - setIsAlertsIntervalRunning(false); - setShowModal(true); + .catch((response: Response) => { + if (response.status === 403) { + setIsAlertsIntervalRunning(false); + setShowModal(true); + } else { + throw response; + } }); }, isAlertsIntervalRunning ? 4000 : null, From d225f0d1aed22a79de6dc85de3cdc8a582f29f3b Mon Sep 17 00:00:00 2001 From: cmaddox5 Date: Tue, 15 Oct 2024 13:57:11 -0400 Subject: [PATCH 5/8] Tests. --- .../auth_manager/error_handler_test.exs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/screenplay_web/auth_manager/error_handler_test.exs b/test/screenplay_web/auth_manager/error_handler_test.exs index 518e16ad..7b407653 100644 --- a/test/screenplay_web/auth_manager/error_handler_test.exs +++ b/test/screenplay_web/auth_manager/error_handler_test.exs @@ -4,10 +4,19 @@ defmodule ScreenplayWeb.AuthManager.ErrorHandlerTest do alias ScreenplayWeb.AuthManager.ErrorHandler describe "auth_error/3" do + test "returns 403 for API endpoints if there's no refresh key", %{conn: conn} do + conn = + conn + |> init_test_session(%{previous_path: "api/test"}) + |> ErrorHandler.auth_error({:some_type, :reason}, []) + + assert %{status: 403} = conn + end + test "redirects to Keycloak login if there's no refresh key", %{conn: conn} do conn = conn - |> init_test_session(%{}) + |> init_test_session(%{previous_path: "test"}) |> ErrorHandler.auth_error({:some_type, :reason}, []) assert html_response(conn, 302) =~ "\"/auth/keycloak\?prompt%3Dlogin" From 8f4ecd1f92a8764cbda562a01a9d927f6e48d5bb Mon Sep 17 00:00:00 2001 From: cmaddox5 Date: Tue, 15 Oct 2024 14:50:29 -0400 Subject: [PATCH 6/8] Fix lint issues. --- assets/js/components/Dashboard/Dashboard.tsx | 1 - .../PermanentConfiguration/Workflows/GlEink/GlEinkWorkflow.tsx | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/assets/js/components/Dashboard/Dashboard.tsx b/assets/js/components/Dashboard/Dashboard.tsx index 1a92f763..59f20281 100644 --- a/assets/js/components/Dashboard/Dashboard.tsx +++ b/assets/js/components/Dashboard/Dashboard.tsx @@ -13,7 +13,6 @@ import AlertBanner from "Components/AlertBanner"; import LinkCopiedToast from "Components/LinkCopiedToast"; import ActionOutcomeToast from "Components/ActionOutcomeToast"; import { useLocation } from "react-router-dom"; -import { Button, Modal } from "react-bootstrap"; import ErrorModal from "Components/ErrorModal"; const Dashboard: ComponentType = () => { diff --git a/assets/js/components/Dashboard/PermanentConfiguration/Workflows/GlEink/GlEinkWorkflow.tsx b/assets/js/components/Dashboard/PermanentConfiguration/Workflows/GlEink/GlEinkWorkflow.tsx index e8811aac..54e56606 100644 --- a/assets/js/components/Dashboard/PermanentConfiguration/Workflows/GlEink/GlEinkWorkflow.tsx +++ b/assets/js/components/Dashboard/PermanentConfiguration/Workflows/GlEink/GlEinkWorkflow.tsx @@ -10,7 +10,7 @@ import ConfigureScreensWorkflowPage, { import BottomActionBar from "Components/PermanentConfiguration/BottomActionBar"; import { useLocation, useNavigate } from "react-router-dom"; import StationSelectPage from "Components/PermanentConfiguration/Workflows/GlEink/StationSelectPage"; -import { Alert, Button, Modal } from "react-bootstrap"; +import { Alert } from "react-bootstrap"; import { ExclamationCircleFill } from "react-bootstrap-icons"; import { useConfigValidationContext, From 9c32425a0a496b1ed78faaffbb92a1a2fa4a572c Mon Sep 17 00:00:00 2001 From: cmaddox5 Date: Tue, 15 Oct 2024 15:08:36 -0400 Subject: [PATCH 7/8] Fix client tests. --- assets/tests/setup.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/assets/tests/setup.ts b/assets/tests/setup.ts index f3e9381f..13097e3f 100644 --- a/assets/tests/setup.ts +++ b/assets/tests/setup.ts @@ -33,6 +33,7 @@ beforeEach(() => { .fn() .mockReturnValueOnce( Promise.resolve({ + status: 200, json: () => Promise.resolve({ all_alert_ids: allAPIAlertIds, From 1b6c9fd685ca6993a2e70abcf33cab7ab4bae4d5 Mon Sep 17 00:00:00 2001 From: cmaddox5 Date: Fri, 15 Nov 2024 09:09:28 -0500 Subject: [PATCH 8/8] Check current and previous path. --- lib/screenplay_web/auth_manager/error_handler.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/screenplay_web/auth_manager/error_handler.ex b/lib/screenplay_web/auth_manager/error_handler.ex index eb80b70c..e3e545f5 100644 --- a/lib/screenplay_web/auth_manager/error_handler.ex +++ b/lib/screenplay_web/auth_manager/error_handler.ex @@ -9,7 +9,7 @@ defmodule ScreenplayWeb.AuthManager.ErrorHandler do @impl Guardian.Plug.ErrorHandler def auth_error(conn, error, _opts) do - if Plug.Conn.get_session(conn, :previous_path) =~ "api" do + if conn.request_path =~ "api" or Plug.Conn.get_session(conn, :previous_path) =~ "api" do Plug.Conn.send_resp(conn, 403, "Session expired") else auth_params = auth_params_for_error(error)