Skip to content

Commit

Permalink
Fix navigating directly to a plus route (#3003)
Browse files Browse the repository at this point in the history
  • Loading branch information
allisonking authored Apr 7, 2023
1 parent 4012567 commit 1c88fd9
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 91 deletions.
3 changes: 2 additions & 1 deletion clients/admin-ui/cypress/e2e/datamap.cy.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { stubDatamap } from "cypress/support/stubs";
import { stubDatamap, stubPlus } from "cypress/support/stubs";

describe("Datamap table and spatial view", () => {
beforeEach(() => {
cy.login();
stubDatamap();
stubPlus(true);
});

it("Can render only render one view at a time", () => {
Expand Down
3 changes: 3 additions & 0 deletions clients/admin-ui/cypress/e2e/privacy-notices.cy.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import { stubPlus } from "cypress/support/stubs";

import { PRIVACY_NOTICES_ROUTE } from "~/features/common/nav/v2/routes";
import { RoleRegistryEnum } from "~/types/api";

describe("Privacy notices", () => {
beforeEach(() => {
cy.login();
stubPlus(true);
});

describe("permissions", () => {
Expand Down
23 changes: 22 additions & 1 deletion clients/admin-ui/cypress/e2e/routes.cy.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { stubPlus } from "cypress/support/stubs";

import { ADD_SYSTEMS_ROUTE } from "~/features/common/nav/v2/routes";
import {
ADD_SYSTEMS_ROUTE,
DATAMAP_ROUTE,
PRIVACY_NOTICES_ROUTE,
} from "~/features/common/nav/v2/routes";
import { RoleRegistryEnum } from "~/types/api";

describe("Routes", () => {
Expand Down Expand Up @@ -64,4 +68,21 @@ describe("Routes", () => {
});
});
});

describe("plus", () => {
it("non-plus cannot access plus routes", () => {
stubPlus(false);
cy.visit(DATAMAP_ROUTE);
cy.getByTestId("home-content");
cy.getByTestId("cytoscape-graph").should("not.exist");
cy.visit(PRIVACY_NOTICES_ROUTE);
cy.getByTestId("home-content");
});

it("plus can access plus routes", () => {
stubPlus(true);
cy.visit(DATAMAP_ROUTE);
cy.getByTestId("cytoscape-graph");
});
});
});
15 changes: 7 additions & 8 deletions clients/admin-ui/src/features/auth/ProtectedRoute.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import { ReactNode } from "react";

import { useAppDispatch, useAppSelector } from "~/app/hooks";
import { LOGIN_ROUTE, VERIFY_AUTH_INTERVAL } from "~/constants";
import { canAccessRoute } from "~/features/common/nav/v2/nav-config";
import { useNav } from "~/features/common/nav/v2/hooks";
import { useGetHealthQuery } from "~/features/plus/plus.slice";
import { useGetUserPermissionsQuery } from "~/features/user-management";

import { logout, selectToken, selectUser } from "./auth.slice";
Expand All @@ -18,6 +19,8 @@ const useProtectedRoute = (redirectUrl: string) => {
pollingInterval: VERIFY_AUTH_INTERVAL,
skip: !userId,
});
const plusQuery = useGetHealthQuery();
const nav = useNav({ path: router.pathname });

if (!token || !userId || permissionsQuery.isError) {
// Reset the user information in redux only if we have stale information
Expand All @@ -30,16 +33,12 @@ const useProtectedRoute = (redirectUrl: string) => {
return { authenticated: false, hasAccess: false };
}

const path = router.pathname;
const userScopes = permissionsQuery.data
? permissionsQuery.data.total_scopes
: [];

const hasAccess = canAccessRoute({ path, userScopes });
const hasAccess = !!nav.active;
if (
!hasAccess &&
permissionsQuery.isSuccess &&
typeof window !== "undefined"
typeof window !== "undefined" &&
!plusQuery.isLoading
) {
router.push("/");
}
Expand Down
47 changes: 1 addition & 46 deletions clients/admin-ui/src/features/common/nav/v2/nav-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,7 @@ import { describe, expect, it } from "@jest/globals";

import { ScopeRegistryEnum } from "~/types/api";

import {
canAccessRoute,
configureNavGroups,
findActiveNav,
NAV_CONFIG,
} from "./nav-config";
import { configureNavGroups, findActiveNav, NAV_CONFIG } from "./nav-config";
import * as routes from "./routes";

const ALL_SCOPES = [
Expand Down Expand Up @@ -256,44 +251,4 @@ describe("findActiveNav", () => {
expect(activeNav?.path).toEqual(expected.path);
});
});

describe("canAccessRoute", () => {
const accessTestCases = [
{
path: "/",
expected: true,
userScopes: [],
},
{
path: routes.ADD_SYSTEMS_ROUTE,
expected: false,
userScopes: [],
},
{
path: routes.ADD_SYSTEMS_ROUTE,
expected: true,
userScopes: [ScopeRegistryEnum.SYSTEM_CREATE],
},
{
path: routes.PRIVACY_REQUESTS_ROUTE,
expected: false,
userScopes: [ScopeRegistryEnum.SYSTEM_CREATE],
},
{
path: routes.PRIVACY_REQUESTS_ROUTE,
expected: true,
userScopes: [ScopeRegistryEnum.PRIVACY_REQUEST_READ],
},
{
path: `${routes.PRIVACY_REQUESTS_ROUTE}?queryParam`,
expected: true,
userScopes: [ScopeRegistryEnum.PRIVACY_REQUEST_READ],
},
];
accessTestCases.forEach(({ path, expected, userScopes }) => {
it(`${path} is scope restricted`, () => {
expect(canAccessRoute({ path, userScopes })).toBe(expected);
});
});
});
});
35 changes: 0 additions & 35 deletions clients/admin-ui/src/features/common/nav/v2/nav-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,38 +282,3 @@ export const findActiveNav = ({
path: activePath,
};
};

/**
* Similar to findActiveNav, but using NavConfig instead of a NavGroup
* This may not be needed once we remove the progressive nav, since then we can
* just check what navs are available (they would all be restricted by scope)
*/
export const canAccessRoute = ({
path,
userScopes,
}: {
path: string;
userScopes: ScopeRegistryEnum[];
}) => {
let childMatch: NavConfigRoute | undefined;
const groupMatch = NAV_CONFIG.find((group) => {
childMatch = group.routes.find((child) =>
child.exact ? path === child.path : path.startsWith(child.path)
);
return childMatch;
});

if (!(groupMatch && childMatch)) {
return false;
}

// Special case of empty scopes
if (childMatch.scopes.length === 0) {
return true;
}

const scopeOverlaps = childMatch.scopes.filter((scope) =>
userScopes.includes(scope)
);
return scopeOverlaps.length > 0;
};

0 comments on commit 1c88fd9

Please sign in to comment.