fix: validate the landing selection plan id before building URLs from it

.github/workflows/jest.yml

@@ -0,0 +1,15 @@
+name: jest
+
+on: [push, pull_request]
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
+      - uses: actions/setup-node@v3
+        with:
+          node-version: 22
+      - run: yarn install
+      - run: yarn test

babel.config.js

@@ -0,0 +1,14 @@
+// Babel config used by jest (babel-jest). It is scoped to the "test" env so the
+// webpack build is left untouched: webpack.common.js configures babel-loader
+// with inline presets, and for non-test envs this file contributes nothing.
+module.exports = {
+    env: {
+        test: {
+            presets: [
+                ['@babel/preset-env', { targets: { node: 'current' } }],
+                '@babel/preset-react',
+                '@babel/preset-flow'
+            ]
+        }
+    }
+};

jest.config.js

@@ -0,0 +1,4 @@
+module.exports = {
+    testEnvironment: 'node',
+    testMatch: ['**/src/**/*.test.js']
+};

package.json

@@ -7,6 +7,7 @@
     "build-dev": "./node_modules/.bin/webpack --config webpack.dev.js",
     "build": "./node_modules/.bin/webpack --config webpack.prod.js",
     "serve": "webpack-dev-server --open --config webpack.dev.js",
+    "test": "jest",
     "clean-branchs": "git branch | grep -v \"master\\|main\\|production\" | xargs git branch -D"
   },
   "author": "",
@@ -101,7 +102,9 @@
     "webpack-dev-server": "^4.7.4",
     "webpack-merge": "^5.8.0"
   },
-  "devDependencies": {},
+  "devDependencies": {
+    "jest": "30.4.2"
+  },
   "resolutions": {
     "lodash": "4.17.21"
   }

src/actions/speaker-actions.js

@@ -472,7 +472,7 @@ export const saveSpeakerProfile = (entity) => async (dispatch, getState) => {
             dispatch(getSpeakerInfo(null));
         })
         .then((payload) => {
-            const redirectUrl = summit ? `/app/${summit.slug}/${getSubmissionsPath()}` : '/app/start';
+            const redirectUrl = summit ? `/app/${summit.slug}/${getSubmissionsPath(summit)}` : '/app/start';
             success_message.html = T.translate("edit_profile.profile_saved");
             dispatch(showMessage(success_message, () => history.push(redirectUrl)));
         });

src/components/nav-menu/index.js

@@ -18,13 +18,15 @@ import MenuItem from './menu-item'
 import MenuItemsDefinitions from './menu-items-definition'
 import '../../styles/menu.less';
 import {connect} from "react-redux";
-import {getCurrentSelectionPlanId, getLandingSelectionPlanId} from "../../utils/methods";
+import {getCurrentSelectionPlanId, getAllowedLandingSelectionPlanId} from "../../utils/methods";
 import DocList from "./doc-list";
 
 
 const NavMenu = ({summit, active, user, exclusiveSections, presentation}) => {
     const [activeItem, setActiveItem] = useState(active);
-    const landingSP = getLandingSelectionPlanId();
+    // a landing plan the user cannot submit to would build menu links that the allowed-plan
+    // guard bounces back to /all-plans, so fall back to the global routes instead
+    const landingSP = getAllowedLandingSelectionPlanId(summit);
     const currentSP = getCurrentSelectionPlanId();
 
     const globalSummitDocs = summit.summit_documents.filter(sd => sd.selection_plan_id === 0);

src/layouts/summit-layout.js

@@ -19,7 +19,7 @@ import AllPlansLayout from "./all-plans-layout";
 import PlanSelectionPage from "../pages/plan-selection-page";
 import ProfilePage from "../pages/profile-page";
 import ClockComponent from '../components/clock';
-import {getLandingSelectionPlanId} from "../utils/methods";
+import {getAllowedLandingSelectionPlanId} from "../utils/methods";
 
 const SummitLayout = ({summit, loading, match, speaker, location, baseLoaded, ...props}) => {
   const urlSummitSlug = match.params.summit_slug;
@@ -39,7 +39,9 @@ const SummitLayout = ({summit, loading, match, speaker, location, baseLoaded, ..
 
   // check if speaker profile exists, if not redirect
   if ((!speaker || !speaker.id) && !location.pathname.includes('/profile') && !loading) {
-    const spLanding = getLandingSelectionPlanId();
+    // only keep the landing plan in the URL if the user can actually submit to it, otherwise
+    // SelectionPlanLayout's guard bounces us back here and we redirect again, forever
+    const spLanding = getAllowedLandingSelectionPlanId(summit);
 
     return (
       <Redirect exact to={{pathname: `/app/${summit.slug}/${spLanding ? `all-plans/${spLanding}/profile` : 'all-plans/profile'}`}}/>

src/pages/preview-presentation-page.js

@@ -41,7 +41,7 @@ class PreviewPresentationPage extends React.Component {
         ev.preventDefault();
 
         const {history, summit} = this.props;
-        const submissionsPath = getSubmissionsPath();
+        const submissionsPath = getSubmissionsPath(summit);
 
         history.push(`/app/${summit.slug}/${submissionsPath}`);
     }

src/pages/profile-page.js

@@ -20,7 +20,6 @@ import {saveSpeakerProfile, getOrganizationalRoles} from "../actions/speaker-act
 import {getSpeakerInfo} from "../actions/auth-actions";
 
 import '../styles/profile-page.less';
-import {getSubmissionsPath} from "../utils/methods";
 
 const ProfilePage = ({entity, speaker, orgRoles, loggedMember, errors, loading, summit, selectionPlanId, history, selectionPlansSettings, ...props}) => {
     const selectionPlanSettings = selectionPlansSettings?.[selectionPlanId];

src/pages/thankyou-presentation-page.js

@@ -30,7 +30,7 @@ class ThankYouPresentationPage extends React.Component {
         ev.preventDefault();
 
         const {history, summit} = this.props;
-        const submissionsPath = getSubmissionsPath();
+        const submissionsPath = getSubmissionsPath(summit);
 
         history.push(`/app/${summit.slug}/${submissionsPath}`);
     }

src/utils/methods.js

@@ -259,13 +259,32 @@ export const setDefaultColors = () => {
     setDocumentColors(defaultColors);
 };
 
-export const getSubmissionsPath = () => {
-    const selectionPlanLandingId = localStorage.getItem(SP_LANDING);
+export const getSubmissionsPath = (summit) => {
+    const selectionPlanLandingId = getAllowedLandingSelectionPlanId(summit);
     return selectionPlanLandingId ? `all-plans/${selectionPlanLandingId}` : 'all-plans';
 };
 
-export const getLandingSelectionPlanId = () => {
-    return localStorage.getItem(SP_LANDING);
+/**
+ * SP_LANDING is stored globally, so it can outlive the summit it was set on and point to a
+ * selection plan id that isn't among the member's allowed plans for this summit. Callers that
+ * build URLs from it must use this instead, or they send the user into a route the allowed-plan
+ * guard bounces back. This mirrors the guard in selection-plan-layout.js:SelectionPlanLayout
+ * (allowedSelectionPlans.some(sp => sp.id === selectionPlanId)) on purpose: it's what closes the
+ * redirect loop. If either check ever gains a condition the other lacks, the loop comes back.
+ *
+ * @param summit
+ * @returns {number|null} the landing selection plan id, or null when it isn't among the
+ *   member's allowed plans for this summit
+ */
+export const getAllowedLandingSelectionPlanId = (summit) => {
+    const storedId = localStorage.getItem(SP_LANDING);
+    if (!storedId) return null;
+
+    const selectionPlanLandingId = parseInt(storedId);
+    if (Number.isNaN(selectionPlanLandingId) || selectionPlanLandingId <= 0) return null;
+
+    const isAllowed = summit?.selection_plans?.some(sp => sp.id === selectionPlanLandingId);
+    return isAllowed ? selectionPlanLandingId : null;
 }
 
 /**

src/utils/methods.test.js

@@ -0,0 +1,92 @@
+/**
+ * Copyright 2018 OpenStack Foundation
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ **/
+
+import { getAllowedLandingSelectionPlanId, getSubmissionsPath } from './methods';
+import { SP_LANDING } from './constants';
+
+const originalLocalStorage = global.localStorage;
+
+afterEach(() => {
+    global.localStorage = originalLocalStorage;
+});
+
+const setLanding = (value) => {
+    global.localStorage = {
+        getItem: (key) => (key === SP_LANDING && value !== null ? String(value) : null)
+    };
+};
+
+const summitWithPlans = (...ids) => ({selection_plans: ids.map(id => ({id}))});
+
+describe('getAllowedLandingSelectionPlanId', () => {
+    it('returns the landing plan id when the user is allowed to submit to it', () => {
+        setLanding(123);
+
+        expect(getAllowedLandingSelectionPlanId(summitWithPlans(45, 123))).toBe(123);
+    });
+
+    it('returns null when the landing plan is not among the allowed plans', () => {
+        // stale SP_LANDING: id kept from another summit or a closed plan.
+        // this is the precondition of the /all-plans/{id}/profile redirect loop
+        setLanding(999);
+
+        expect(getAllowedLandingSelectionPlanId(summitWithPlans(45, 123))).toBeNull();
+    });
+
+    it('returns null when no landing plan was ever stored', () => {
+        setLanding(null);
+
+        expect(getAllowedLandingSelectionPlanId(summitWithPlans(45, 123))).toBeNull();
+    });
+
+    it('returns null when the stored value is not a usable plan id', () => {
+        setLanding('');
+        expect(getAllowedLandingSelectionPlanId(summitWithPlans(45, 123))).toBeNull();
+
+        setLanding('not-a-plan');
+        expect(getAllowedLandingSelectionPlanId(summitWithPlans(45, 123))).toBeNull();
+    });
+
+    it('returns null for a non-positive id even if a plan matches it', () => {
+        setLanding(0);
+
+        expect(getAllowedLandingSelectionPlanId(summitWithPlans(0, 123))).toBeNull();
+    });
+
+    it('returns null when the allowed plans are not loaded yet', () => {
+        setLanding(123);
+
+        expect(getAllowedLandingSelectionPlanId(undefined)).toBeNull();
+        expect(getAllowedLandingSelectionPlanId({})).toBeNull();
+    });
+});
+
+describe('getSubmissionsPath', () => {
+    it('keeps the landing plan in the path when the user can submit to it', () => {
+        setLanding(123);
+
+        expect(getSubmissionsPath(summitWithPlans(45, 123))).toBe('all-plans/123');
+    });
+
+    it('falls back to the global submissions path when the landing plan is stale', () => {
+        setLanding(999);
+
+        expect(getSubmissionsPath(summitWithPlans(45, 123))).toBe('all-plans');
+    });
+
+    it('falls back to the global submissions path when no landing plan was stored', () => {
+        setLanding(null);
+
+        expect(getSubmissionsPath(summitWithPlans(45, 123))).toBe('all-plans');
+    });
+});