mirror of
https://github.com/Hestia-Homes/assessment-model.git
synced 2026-07-27 22:45:03 +00:00
fix(ara-projects): require user onboarding before /projects
Code review: /onboarding is *user* onboarding, not Portfolio onboarding.
It captures the user's details and their required privacy-policy
acceptance (plus marketing opt-in) and touches no Portfolio at all —
confirmed by the form schema in src/app/onboarding/page.tsx, which has
no portfolio coupling and makes acceptedPrivacy mandatory.
The exemption added in e34f166f was therefore wrong: it let contractor
users into the app without the consent we are required to capture. A
contractor can complete onboarding perfectly well, so nobody needs the
bypass.
Removes the /projects exemption and isProjectsPath(). /projects stays in
the matcher, so the tree is still auth-guarded. Unit and Cypress tests
inverted to assert the corrected behaviour, plus a new case covering an
onboarded contractor reaching /projects.
Refs #406
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
e34f166f9c
commit
c482f2d6e8
4 changed files with 44 additions and 42 deletions
|
|
@ -46,9 +46,10 @@ describe("Ara Projects route shell", function () {
|
|||
cy.contains("h1", "Projects").should("be.visible");
|
||||
});
|
||||
|
||||
it("does not force a contractor through portfolio onboarding", function () {
|
||||
// A contractor org member has no Portfolio and so is never `onboarded`.
|
||||
// Middleware must let them into /projects anyway (issue #406).
|
||||
it("sends a non-onboarded user to onboarding", function () {
|
||||
// /onboarding captures user details and the required privacy-policy
|
||||
// acceptance — it is not Portfolio onboarding, so contractors complete it
|
||||
// too and nobody reaches /projects without it.
|
||||
cy.login({
|
||||
email: "site@contractor.example",
|
||||
name: "Contractor User",
|
||||
|
|
@ -58,6 +59,19 @@ describe("Ara Projects route shell", function () {
|
|||
|
||||
cy.visit("/projects");
|
||||
|
||||
cy.location("pathname").should("eq", "/onboarding");
|
||||
});
|
||||
|
||||
it("lets an onboarded contractor into /projects", function () {
|
||||
cy.login({
|
||||
email: "site@contractor.example",
|
||||
name: "Contractor User",
|
||||
onboarded: true,
|
||||
sub: "cypress-contractor",
|
||||
});
|
||||
|
||||
cy.visit("/projects");
|
||||
|
||||
cy.location("pathname").should("eq", "/projects");
|
||||
cy.get("[data-testid=projects-shell]").should("exist");
|
||||
});
|
||||
|
|
|
|||
|
|
@ -12,7 +12,8 @@ const TOP_LEVEL_NAV: ProjectsNavItem[] = [
|
|||
* Deliberately top-level rather than nested under `/portfolio/[slug]`: Ara
|
||||
* Projects are organisation-scoped (`project.organisation_id`) and are reached
|
||||
* by contractor users from external organisations, who have no Portfolio at
|
||||
* all. See CONTEXT.md § Ara Projects and ADR-0018.
|
||||
* all. (They still complete user onboarding — see `src/middleware.ts`.)
|
||||
* See CONTEXT.md § Ara Projects and ADR-0018.
|
||||
*/
|
||||
export default async function ProjectsLayout({
|
||||
children,
|
||||
|
|
|
|||
|
|
@ -1,36 +1,33 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import { isProjectsPath, routeAuthenticatedRequest } from "./middleware";
|
||||
import { routeAuthenticatedRequest } from "./middleware";
|
||||
|
||||
const contractor = { email: "site@contractor.example", onboarded: false };
|
||||
const onboardedContractor = { email: "site@contractor.example", onboarded: true };
|
||||
const clientOrgMember = { email: "asset@landlord.example", onboarded: true };
|
||||
const internal = { email: "dev@domna.homes", onboarded: false };
|
||||
|
||||
describe("isProjectsPath", () => {
|
||||
it("matches the tree root and its descendants", () => {
|
||||
expect(isProjectsPath("/projects")).toBe(true);
|
||||
expect(isProjectsPath("/projects/42")).toBe(true);
|
||||
expect(isProjectsPath("/projects/42/work-orders")).toBe(true);
|
||||
});
|
||||
|
||||
it("does not match unrelated paths that merely share the prefix", () => {
|
||||
expect(isProjectsPath("/projects-archive")).toBe(false);
|
||||
expect(isProjectsPath("/portfolio/7/your-projects")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("routeAuthenticatedRequest", () => {
|
||||
it("lets a contractor reach /projects without portfolio onboarding", () => {
|
||||
// The whole point of the exemption: contractors have no Portfolio, so
|
||||
// onboarding would be a loop with no exit.
|
||||
it("sends a non-onboarded contractor to onboarding, including on /projects", () => {
|
||||
// /onboarding is *user* onboarding — it captures the required
|
||||
// privacy-policy acceptance and touches no Portfolio. Nobody skips it.
|
||||
expect(
|
||||
routeAuthenticatedRequest({ pathname: "/projects", ...contractor })
|
||||
.redirectTo
|
||||
).toBeNull();
|
||||
).toBe("/onboarding");
|
||||
expect(
|
||||
routeAuthenticatedRequest({
|
||||
pathname: "/projects/42/work-orders",
|
||||
...contractor,
|
||||
}).redirectTo
|
||||
).toBe("/onboarding");
|
||||
});
|
||||
|
||||
it("lets a contractor into /projects once onboarded", () => {
|
||||
expect(
|
||||
routeAuthenticatedRequest({
|
||||
pathname: "/projects",
|
||||
...onboardedContractor,
|
||||
}).redirectTo
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -2,20 +2,16 @@ import { NextResponse } from "next/server";
|
|||
import type { NextRequest } from "next/server";
|
||||
import { getToken } from "next-auth/jwt";
|
||||
|
||||
/**
|
||||
* Ara Projects is organisation-scoped and reached by contractor users from
|
||||
* external organisations, who have no Portfolio and therefore never complete
|
||||
* the Portfolio onboarding flow. Forcing them through `/onboarding` would trap
|
||||
* them in a loop they cannot exit, so `/projects` is exempt from the onboarding
|
||||
* redirect. Authorization within the tree is enforced per-Project — see #408.
|
||||
*/
|
||||
export function isProjectsPath(pathname: string): boolean {
|
||||
return pathname === "/projects" || pathname.startsWith("/projects/");
|
||||
}
|
||||
|
||||
/**
|
||||
* Pure routing decision for an authenticated request, split out so the
|
||||
* onboarding/contractor rules are unit-testable without a real JWT.
|
||||
* onboarding rules are unit-testable without a real JWT.
|
||||
*
|
||||
* Note that `/onboarding` is *user* onboarding, not Portfolio onboarding: it
|
||||
* captures the user's details and their required privacy-policy acceptance
|
||||
* (plus marketing opt-in), and touches no Portfolio at all. Every non-internal
|
||||
* user must therefore complete it — including contractor users from external
|
||||
* organisations reaching `/projects`. Exempting the Projects tree would let a
|
||||
* user into the app without the consent we are required to capture.
|
||||
*/
|
||||
export function routeAuthenticatedRequest({
|
||||
pathname,
|
||||
|
|
@ -29,14 +25,8 @@ export function routeAuthenticatedRequest({
|
|||
// Internal users bypass onboarding entirely.
|
||||
const isInternal = email.endsWith("@domna.homes");
|
||||
|
||||
// Not onboarded and not internal: send to onboarding, unless they are
|
||||
// already there or are heading into the contractor-reachable Projects tree.
|
||||
if (
|
||||
onboarded === false &&
|
||||
pathname !== "/onboarding" &&
|
||||
!isInternal &&
|
||||
!isProjectsPath(pathname)
|
||||
) {
|
||||
// Not onboarded and not internal: send to onboarding, unless already there.
|
||||
if (onboarded === false && pathname !== "/onboarding" && !isInternal) {
|
||||
return { redirectTo: "/onboarding" };
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue