diff --git a/README.md b/README.md index a56c9c8..a92133c 100644 --- a/README.md +++ b/README.md @@ -219,7 +219,7 @@ fetchMore({ variables: { after: data.searchCatalog.pageInfo.endCursor } }) - **Load more with `fetchMore`.** The cache keeps all the pages of one search in one list (the key is the search, filter and sort, not `first` or `after`). A query with `after` that is sent with `useQuery` or `client.query` is answered from the cache. `fetchMore` always asks the server, so use it for the next page. - **`totalCount` only with the first page:** the first-page query asks for the count, and later-page queries omit it; the cache keeps the count when those pages are loaded. -- **Authentication.** Session handling (`devLogin`, `logout`, and `restoreSession`) is in `src/lib/apollo/client.ts`. The development test login accepts any email address and is the only login flow planned; do not use real or sensitive data with it. See [Test login on the virtual machine](docs/VM_SETUP.md#6-test-login-on-the-virtual-machine) and [Login for development](docs/API.md#login-for-development). +- **Authentication.** Session handling (`devLogin`, `logout`, and `restoreSession`) is in `src/lib/apollo/client.ts`. The development test login accepts any email address and is the only login flow planned; do not use real or sensitive data with it. See [Test login on the virtual machine](docs/VM_SETUP.md#6-test-login-on-the-virtual-machine) and [Login for development](docs/API.md#login-for-development). The login page (`src/components/auth/LoginPage.tsx`) offers the demo coach and a test coach of your own. The app reads who is logged in in one place, `useAuth()` (`src/components/auth/useAuth.ts`), which the menu, the protected pages and the gymnasts page share. - **Errors.** `describeError(error)` (`src/lib/apollo/errors.ts`) turns an error from the API or the network into a message for the user, and says whether trying again can help. ## The Elements page: search, filters and sort @@ -467,6 +467,22 @@ node -e "console.log(require('node:crypto').randomBytes(48).toString('base64url' The server does not start if the secret is shorter than 32 characters. Without it the catalog works and `me` is always `null`. The development test login is enabled with `DEV_LOGIN=true` and a `SESSION_SECRET`; the VM setup documents the additional flag required to enable it in production. It accepts any email address, so it must only be used for testing with non-sensitive data. Send `Authorization: Bearer ` with the queries, or call `refreshSession` with the refresh token. +**Try the app with the demo coach.** The only login is a test login that needs no password (why: [docs/AUTHENTICATION.md](docs/AUTHENTICATION.md)). The demo coach has example gymnasts, and you can add and delete things as it. + +1. In `backend/.env` add `SESSION_SECRET` (see above) and `DEV_LOGIN=true`. In `.env.local` in the repository root add `VITE_DEV_LOGIN=true`. +2. Fill the database and make the demo coach, then start the backend and the frontend: + + ```bash + npm run db:seed # the catalog + npm run db:demo # the demo coach (demo@wag-planner.example) with three gymnasts + npm run backend + npm run dev + ``` + +3. Open the app, click **Log in**, and then **Log in as the demo coach**. "Your gymnasts" shows the example gymnasts. Run `npm run db:demo` again to start the demo coach over: it deletes what the demo coach has, and nothing else. Everyone who uses the demo coach shares its data, so run it before each review round. + +Everyone who uses the demo coach shares the same data, and anyone can log in as any test coach, so do not enter real data. The page says so. + ## Catalog search The catalog search is in `backend/src/catalog/search.ts`. It finds elements and vaults by the start of their words, with filters, sorting and cursor pagination, using Prisma only. How it works and why is described in "Search without raw SQL" in [docs/API.md](docs/API.md#search-without-raw-sql). diff --git a/backend/package.json b/backend/package.json index f445048..3f934ae 100644 --- a/backend/package.json +++ b/backend/package.json @@ -17,6 +17,7 @@ "prisma:migrate": "prisma migrate dev", "prisma:deploy": "prisma migrate deploy", "prisma:seed": "prisma db seed", + "demo": "tsx src/seed/demo.ts", "synthetic": "tsx src/seed/synthetic.ts", "prisma:studio": "prisma studio" }, diff --git a/backend/src/seed/demo.test.ts b/backend/src/seed/demo.test.ts new file mode 100644 index 0000000..81409c8 --- /dev/null +++ b/backend/src/seed/demo.test.ts @@ -0,0 +1,129 @@ +// Tests for the demo coach (see "The demo coach" in docs/AUTHENTICATION.md). +// They run against the local database from backend/.env, which must have the +// catalog (`npm run db:seed`), and they use an address of their own so the real +// demo coach is not touched. Without DATABASE_URL they are skipped. + +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { DEMO_GYMNASTS } from '../../../shared/demo.js' +import { createPrismaClient } from '../db.js' +import type { PrismaClient } from '../generated/prisma/client.js' +import { createApolloServer } from '../server/app.js' +import { createContext } from '../server/context.js' +import { resetDemoCoach } from './demo.js' + +const hasDatabase = Boolean(process.env['DATABASE_URL']) +const EMAIL = 'test-138-demo@example.com' +const OTHER = 'test-138-other@example.com' + +describe.runIf(hasDatabase)('resetDemoCoach', () => { + let prisma: PrismaClient + + const clean = () => + prisma.coach.deleteMany({ where: { email: { startsWith: 'test-138-' } } }) + const gymnastsOf = async (email: string) => + ( + await prisma.gymnast.findMany({ + where: { coach: { email } }, + include: { elements: true }, + orderBy: { name: 'asc' }, + }) + ).map((g) => ({ name: g.name, elements: g.elements.length })) + + beforeAll(async () => { + prisma = createPrismaClient() + await clean() + }) + afterAll(async () => { + await clean() + await prisma.$disconnect() + }) + + it('makes the coach with the example gymnasts, each with elements from the catalog', async () => { + const result = await resetDemoCoach(prisma, EMAIL) + + expect(result).toMatchObject({ + gymnasts: DEMO_GYMNASTS.length, + elements: 9, + }) + expect(await gymnastsOf(EMAIL)).toEqual( + [...DEMO_GYMNASTS].sort().map((name) => ({ name, elements: 3 })), + ) + const coach = await prisma.coach.findFirstOrThrow({ + where: { email: EMAIL }, + }) + expect(coach.googleId).toBe(`dev:${EMAIL}`) + }) + + it('is the coach that the test login finds, so the login page can log in as it', async () => { + const server = createApolloServer() + await server.start() + try { + const response = await server.executeOperation( + { + query: `mutation ($email: String!) { devLogin(email: $email) { coach { id email } } }`, + variables: { email: EMAIL }, + }, + { + contextValue: createContext( + prisma, + { + port: 3001, + imageBaseUrl: 'http://images.test/images', + corsOrigins: [], + sessionSecret: 'a-secret-that-is-at-least-32-characters-long', + devLogin: true, + }, + {}, + ), + }, + ) + if (response.body.kind !== 'single') + throw new Error('expected one result') + const coach = await prisma.coach.findFirstOrThrow({ + where: { email: EMAIL }, + }) + expect(response.body.singleResult.errors).toBeUndefined() + expect( + ( + response.body.singleResult.data as { + devLogin: { coach: { id: string } } + } + ).devLogin.coach.id, + ).toBe(coach.id) + expect(await prisma.coach.count({ where: { email: EMAIL } })).toBe(1) + } finally { + await server.stop() + } + }) + + it('puts the example data back after visitors have changed it, and touches no other coach', async () => { + const coach = await prisma.coach.findFirstOrThrow({ + where: { email: EMAIL }, + }) + await prisma.gymnast.deleteMany({ + where: { coachId: coach.id, name: DEMO_GYMNASTS[0]! }, + }) + await prisma.gymnast.create({ + data: { coachId: coach.id, name: 'Added by a visitor' }, + }) + const other = await prisma.coach.create({ + data: { + googleId: 'dev:' + OTHER, + email: OTHER, + name: 'Other', + gymnasts: { create: [{ name: 'Not the demo' }] }, + }, + }) + + await resetDemoCoach(prisma, EMAIL) + + expect((await gymnastsOf(EMAIL)).map((g) => g.name)).toEqual( + [...DEMO_GYMNASTS].sort(), + ) + expect(await gymnastsOf(OTHER)).toEqual([ + { name: 'Not the demo', elements: 0 }, + ]) + expect(await prisma.coach.count({ where: { id: other.id } })).toBe(1) + expect(await prisma.coach.count({ where: { email: EMAIL } })).toBe(1) + }) +}) diff --git a/backend/src/seed/demo.ts b/backend/src/seed/demo.ts new file mode 100644 index 0000000..0b652b0 --- /dev/null +++ b/backend/src/seed/demo.ts @@ -0,0 +1,100 @@ +// Makes the demo coach and puts its example data back (see "The demo coach" in +// docs/AUTHENTICATION.md). +// +// Run with `npm run db:demo` from the repository root, after `npm run db:seed` +// (the example elements come from the catalog). It can be run again whenever +// the demo coach should start over: everything the demo coach has is deleted +// first, so whatever visitors added or deleted is gone. Other coaches are never +// touched. +// +// The demo coach is a coach of the test login (`devLogin`, see "Login for +// development" in docs/API.md): its Google id is "dev:", so +// `devLogin` finds it. It has no password; this script only makes the coach +// and its data, and the login itself still needs DEV_LOGIN=true on the server. + +import { fileURLToPath } from 'node:url' +import { DEMO_EMAIL, DEMO_GYMNASTS, DEMO_NAME } from '../../../shared/demo.js' +import { createPrismaClient } from '../db.js' +import type { PrismaClient } from '../generated/prisma/client.js' + +/** The elements each demo gymnast "can do", from the catalog. */ +const ELEMENTS_PER_GYMNAST = 3 + +export type DemoResult = { + coachId: string + gymnasts: number + elements: number +} + +/** + * Makes the demo coach if there is none, and gives it exactly the example + * gymnasts. Each gymnast gets a few elements in the list of what they can do: + * real elements of the catalog, the first ones that are not removed, one after + * the other. If the catalog is empty the gymnasts get no elements. `email` is + * only different in the tests, which must not touch the real demo coach. + */ +export async function resetDemoCoach( + prisma: PrismaClient, + email: string = DEMO_EMAIL, +): Promise { + const googleId = `dev:${email}` + // Elements of the catalog and not the synthetic test rows (SYN-), which + // `npm run db:synthetic -- remove` deletes. + const elements = await prisma.element.findMany({ + where: { removedAt: null, NOT: { id: { startsWith: 'SYN-' } } }, + orderBy: { id: 'asc' }, + take: DEMO_GYMNASTS.length * ELEMENTS_PER_GYMNAST, + select: { id: true }, + }) + + return prisma.$transaction(async (tx) => { + const coach = await tx.coach.upsert({ + where: { googleId }, + create: { googleId, email, name: DEMO_NAME }, + update: { name: DEMO_NAME }, + }) + // Deleting the gymnasts deletes their routines and lists (ON DELETE CASCADE). + await tx.gymnast.deleteMany({ where: { coachId: coach.id } }) + + let added = 0 + for (const [index, name] of DEMO_GYMNASTS.entries()) { + const own = elements.slice( + index * ELEMENTS_PER_GYMNAST, + (index + 1) * ELEMENTS_PER_GYMNAST, + ) + await tx.gymnast.create({ + data: { + coachId: coach.id, + name, + elements: { create: own.map((e) => ({ elementId: e.id })) }, + }, + }) + added += own.length + } + return { + coachId: coach.id, + gymnasts: DEMO_GYMNASTS.length, + elements: added, + } + }) +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + const prisma = createPrismaClient() + resetDemoCoach(prisma) + .then((result) => { + console.log( + `The demo coach (${DEMO_EMAIL}) has ${result.gymnasts} gymnasts with ${result.elements} elements.`, + ) + if (result.elements === 0) { + console.log( + 'The catalog is empty, so the gymnasts have no elements. Run "npm run db:seed" first.', + ) + } + }) + .catch((error: unknown) => { + console.error(error instanceof Error ? error.message : error) + process.exitCode = 1 + }) + .finally(() => prisma.$disconnect()) +} diff --git a/docs/AI_USAGE.md b/docs/AI_USAGE.md index 5a70fb7..e63ae75 100644 --- a/docs/AI_USAGE.md +++ b/docs/AI_USAGE.md @@ -493,6 +493,12 @@ - **What the team did:** Decided that the page says "1000+" when there are more than 1000 results, and that the server must not count more than that. - **What Claude did:** Made the database stop counting at 1001 (`count` with `take`), kept the schema the same (a `totalCount` above 1000 means more than 1000), wrote the tests (the count stops at one more than the most, it is not capped below that, and a search is capped too) and broke the cap to check that they fail without it. +## The login, the demo coach and deleting a gymnast (`src/components/auth/`, `src/components/ProtectedRoute.tsx`, `backend/src/seed/demo.ts`, `docs/AUTHENTICATION.md`) + +- **Tool:** Claude Code +- **What the team did:** Asked for one source of login state after peer feedback (the menu and the gymnasts page disagreed), and for a clear way from the login to saving and seeing your own data. Decided to keep the test login and a seeded demo coach, and not to build passwords, when it was clear that the course server has only http (so a password would be sent as plain text) and that the assignment asks for saved user data and not for a login. Asked Claude to delete the work on password accounts, and to write down how much was tried and what a real system would do. Asked that the demo coach can save and delete things. +- **What Claude did:** Read the course text, the OWASP password and authentication cheat sheets, Google's guide for the client library and the Node documentation for `crypto.argon2`. First built password accounts (Argon2id, a lock after five wrong passwords, the same answer for every failure) with tests that were checked by breaking the lock, the same-answer rule and the lock-while-locked rule one at a time; then deleted them on the team's decision. Wrote `AuthProvider` and `useAuth` so the menu, `ProtectedRoute` and the gymnasts page read the same answer, the login page, the http warning, the confirmation before deleting a gymnast, and `npm run db:demo`. Wrote the tests (including one test of the whole path in `src/App.flow.test.tsx`) and broke four behaviors one at a time to check that a test fails: waiting while the server has not answered, the same state for the page and the menu, a `from` that leaves the site, and resetting the store after a login (this one was not caught by the flow test, which replaces the login functions, so `src/lib/apollo/client.test.ts` was added for it). Wrote `docs/AUTHENTICATION.md`. + ## The filters, the sort and the page titles of the Elements page (`src/components/elements/FilterMenu.tsx`, `SortMenu.tsx`, `src/lib/elementFilters.ts`, `src/components/PageTitle.tsx`, `backend/src/catalog/`) - **Tool:** Claude Code diff --git a/docs/API.md b/docs/API.md index 6479d9f..a736208 100644 --- a/docs/API.md +++ b/docs/API.md @@ -77,7 +77,7 @@ When a new Code of Points drops an element or vault, it is marked as removed in - The list is independent of the routines. A gymnast can practice an item that is in no routine, and a routine can contain an item that is not in the list. - Errors: `UNAUTHENTICATED`, `NOT_FOUND` (the gymnast or the item does not exist, or the gymnast belongs to another coach), `INVALID_ITEMS` (a removed item cannot be added) and `LIMIT_REACHED` (100 items per gymnast). - **`updatedAt` on a routine** changes when the routine is renamed and when its items change. The items are stored in a separate table, so `setRoutineItems` must update `updatedAt` explicitly, in the same transaction as the items. -- **Names.** The name of a gymnast and the name of a routine are normalized: written in Unicode normal form C (so an accented letter typed as one character, or as a letter and an accent, is the same name), with the spaces at the start and the end removed and every run of white space inside the name made into one space. The result must be 1–70 characters. A longer name, or a name with only spaces, throws `BAD_USER_INPUT`. The length is counted in Unicode characters (code points), not in bytes or UTF-16 units, so a name with accents or a simple emoji gets the same room as any other. A flag or a family of emoji is made of several characters and counts as several. 70 characters is the length that the UK Government Data Standards give for a full name in one field. The rule is one file, `shared/names.ts`, which the backend and the form in the frontend both import, so they cannot disagree about what a valid name is. The database does not check the limits yet. `NAME_TAKEN` is checked on the normalized name. +- **Names.** The name of a gymnast and the name of a routine are normalized: written in Unicode normal form C (so an accented letter typed as one character, or as a letter and an accent, is the same name), with the spaces at the start and the end removed and every run of white space inside the name made into one space. The result must be 1–70 characters. A longer name, or a name with only spaces, throws `BAD_USER_INPUT`. The length is counted in Unicode characters (code points), not in bytes or UTF-16 units, so a name with accents or a simple emoji gets the same room as any other. A flag or a family of emoji is made of several characters and counts as several. 70 characters is the length that the UK Government Data Standards give for a full name in one field. The rule is one file, `shared/names.ts`, which the backend and the form in the frontend both import, so they cannot disagree about what a valid name is. The database does not check the limits yet. `NAME_TAKEN` is checked on the normalized name. The form for a gymnast has the same rule: it does not let more than 70 characters be typed or pasted, and answers "Name is required" when there is no name. - **Two tabs on the same routine.** `setRoutineItems` replaces the whole list, and the last save wins. It does not check if the routine was changed since the client loaded it, so a change made in another tab can be overwritten. This is a choice for the prototype. If it becomes a problem, add a `CONFLICT` error and let the client send the `updatedAt` it loaded. - `calculateDScore(itemIds)` is for the calculator, which works without logging in. It calculates the D-score and saves nothing. The rule is the same as for a saved routine (constraint 13 in [DATA_MODEL.md](DATA_MODEL.md)): the sum of the eight highest values, each code counted once, and on vault the D-value of the single vault. The sum is made in tenths (whole numbers), so there are no rounding errors. An empty list gives `0`, and the same id twice counts once. It throws `INVALID_ITEMS` when the items do not fit together (items from more than one apparatus, a vault with elements, or more than one vault) or when an item has been removed from the Code of Points, and `BAD_USER_INPUT` for more than 50 ids, an id that does not exist or an id with odd characters. The function is in `backend/src/dscore.ts` and `Routine.dScore` will use the same one. - The same element may appear more than once in a routine. Removed elements stay in routines and lists with `removed: true`, and make the routine's D-score null (see "Removed elements" in [DATA_MODEL.md](DATA_MODEL.md)). @@ -243,6 +243,8 @@ Until the Google login is merged, and because Sign in with Google needs https (e The coach is stored with the Google id `dev:`, so a real Google account with the same address becomes another coach. The name is the part of the address before the `@`. Try it: set `SESSION_SECRET` and `DEV_LOGIN=true` in `backend/.env`, put `VITE_DEV_LOGIN=true` in `.env.local`, run the backend and the frontend, type an address and click the button. +**The demo coach.** `npm run db:demo` makes the coach `demo@wag-planner.example` with three gymnasts and a few elements in each list, so the staff and the students who review the project can look at the pages without typing an address; the login page has a button for it. It is a coach of this test login, so it needs `DEV_LOGIN=true` too, and everyone who uses it shares the same data. Running the command again deletes what the demo coach has and puts the example data back. Why the login is a test login, what it does not protect and what a real system would do is in [AUTHENTICATION.md](AUTHENTICATION.md). + ## Error handling Errors are thrown from the resolvers as GraphQL errors with a machine-readable code in `extensions.code`. The client reacts to the code and never to the text of the message. diff --git a/docs/AUTHENTICATION.md b/docs/AUTHENTICATION.md new file mode 100644 index 0000000..64e514d --- /dev/null +++ b/docs/AUTHENTICATION.md @@ -0,0 +1,134 @@ +# Authentication: the test login, the demo coach, and what we would do in production + +This document explains how a coach logs in, why the login is a **test login** and not Sign in with Google or passwords, what that does and does not protect, and what a real system would do instead. The technical details of the tokens are in "Identifying the user" in [API.md](API.md). + +**In short** + +- The only login in the app is the test login (`devLogin`): a coach is found or made from an e-mail address, and **nobody proves who they are**. It is guarded so that it cannot be turned on by accident (see [How the test login is guarded](#how-the-test-login-is-guarded)). +- There is a **demo coach** with example gymnasts, made by `npm run db:demo`. One click on the login page logs in as it, so the staff and the students who review the project can look at the pages and save and delete things without typing anything. +- The login state has **one source**: the server's answer to `me`. The menu, the protected pages and the gymnasts page all read it through `useAuth()`. +- We do not develop the login further. The rest of this document says why, and what we would build with https. + +## What the course asks for + +From the project description: + +- "Det skal inngå en eller annen form for brukergenererte data som skal lagres (persistent på databaseserveren) og presenteres": there must be user-generated data that is **saved on the database server** and shown. The examples are reviews, ratings, a search history or a shopping list. A login is not asked for. +- The project is served at `http://it2810-xx.idi.ntnu.no/project2`: **plain http**. NTNU cannot give the groups https, so there is no certificate for the address. +- The project must be **easy to install and run for others** (the teacher). Whatever the login needs must be possible to set up from the README. + +Our user data is the coach's gymnasts, their lists of elements and their routines. They belong to a coach, so the app needs to know which coach is asking. That is all the login has to do. + +## What we chose, and what we did not + +| Option | Why not (or why) | +| -------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Sign in with Google** | Google's guide for the client library says that One Tap only displays on https domains and tells you to add `http://localhost` as the origin for local development ([Google Identity: set up the client library](https://developers.google.com/identity/gsi/web/guides/client-library)); it documents no other http address. The course server has only http, so we treat it as not working there (we have not been able to try it on the server). The server side is written (PR #70, as a draft) and would be used with https. It also needs a Google client id, which makes it harder to run for someone who clones the project. | +| **Our own accounts with an e-mail address and a password** | We built this first, tested it, and **dropped it**. The password would be sent as plain text over http, so anyone on the network could read it, and people use the same password in many places. Hashing in the browser does not help (the hash would be the password), and an encryption scheme of our own over http is not safe either: the script that does the encryption is itself sent over http. Only TLS solves it. We did not want a login that collects passwords we cannot protect. | +| **A random key in the browser** (a guest identity in `localStorage`) | It needs no password, but the key is sent as plain text too, it is lost when the browser data is cleared, and it does not work on a second device. It would also throw away the session code we already have. | +| **Passkeys (WebAuthn) and magic links in an e-mail** | Passkeys need a secure context (https or `localhost`), and a magic link needs a mail server. We have neither. | +| **The test login** (chosen) | Sends no secret, because there is none. It gives the app what it needs (to know which coach is asking, with tokens that expire), it is simple to run, and it is honest about what it is. | + +The assignment asks for well-reasoned choices. This one is made with the trade-offs written down: we know what a login would need, and we say why it cannot be built on this server. + +## How much we tried before choosing + +The test login is not where we started. We wanted a real login over https, and we worked through the options in this order. Each step ended because of something we found out, and that is what this section records. + +1. **Sign in with Google.** This was the plan: no passwords for us to keep, and the user is already logged in to Google. We built the server side (`login(googleToken)`: the token is checked with Google's library for its signature, expiry, issuer and our client id, and the coach is made on the first login). It works on `localhost` and is kept as a draft pull request (#71). It cannot be used on the course server, because Google's login needs https (see the table above). +2. **https on the course server.** NTNU cannot give the groups https. We then looked at turning it on ourselves with a self-signed certificate (Apache `mod_ssl` and `openssl`). That was not possible on our virtual machine. A self-signed certificate would also have made every browser show a warning, and `http://…` and `https://…` are different origins, so a login on one is not a login on the other. +3. **A certificate the browser trusts.** Let's Encrypt can check a domain with a DNS record, so it does not need the server to be reachable from the internet, but it needs a **domain we control**, and the server's `idi.ntnu.no` name is not ours. A tunnel service that gives a public https address would open the virtual machine to the internet, which we did not want to do on a course server without asking. Whether the course staff would allow either is not settled. +4. **Our own accounts with a password.** We built this and tested it: Argon2id from Node's `crypto` with the minimum settings of the OWASP cheat sheet, a password of 15 to 128 characters with no rules about symbols, the same answer and the same time for a wrong address and a wrong password, and a lock for five minutes after five wrong passwords. 16 tests ran against a database, and we broke the lock, the same-answer and the lock-while-locked rules one at a time to see that the tests failed. Then we dropped all of it, because over http the password is sent as plain text. A login that collects passwords we cannot protect is worse than a login that asks for none. +5. **Protecting the password ourselves, over http.** We looked at two ways: encrypting the password in the browser with the server's public key (a "sealed box", for example libsodium), and OPAQUE, a standard (RFC 9807) where the password is never sent at all. Neither was built. Both only protect against someone who **listens**; someone who can **change** the page can change the script and the key too, because they are also sent over http. The tokens after the login are still readable. The browser's own cryptography (`crypto.subtle`) is only available in a secure context, so it would be a library in WebAssembly. And the result would be a protocol that we have to defend ourselves, where the usual and reviewed answer is TLS. Hashing the password in the browser does not help at all: the hash becomes the password. +6. **A random key in the browser** instead of an account (a guest identity). It sends no password, but the key is sent as plain text, it is lost when the browser data is cleared, and it does not work on a second device. +7. **Passkeys and magic links.** Passkeys need a secure context, and a magic link needs a mail server. + +**What we concluded.** With plain http there is no login that proves who the user is and keeps the proof secret on the way. The honest choice is a login that does not pretend to: it keeps each coach's data apart on the server, uses tokens that expire, says on the page that it is not encrypted, and is written down with what a real system would do (see the last section). The assignment asks for user data that is saved on the server and does not ask for a login, so this meets the requirement. If the course server gets https, the steps in "What we would do in a real production system" are the way on, and the Google login is already written. + +**What we did not verify.** We have not tried Sign in with Google on the course server. The claims about what needs a secure context (passkeys, `crypto.subtle`) are from the documentation and from what we know of the browsers; we did not test them on the server. + +## How the test login works + +1. The login page has two ways in: **"Log in as the demo coach"** (one click) and **"Or use your own test coach"** (an e-mail address, like `emma@example.com`). +2. The page calls `devLogin(email)`. The server finds the coach with that address, or makes one, and answers with the same tokens as every login would: a short-lived **access token** (15 minutes) and a **refresh token** (14 days, used once, replaced at every use). See "Identifying the user" in [API.md](API.md). +3. The coach's data (gymnasts, routines) is saved on the server for that coach, so it is there after a reload and after a new login with the same address. +4. Everyone who uses the same address is the same coach and sees the same data. Each person should use their own address, or the demo coach to look around. + +The coach is stored with the Google id `dev:`, so a real Google account with the same address would be another coach. + +### How the test login is guarded + +A login where nobody proves who they are must not be on by accident: + +- **Off by default.** The server answers `NOT_IMPLEMENTED` unless it was started with `DEV_LOGIN=true`. +- **Only on purpose in production.** The server does not start with `DEV_LOGIN=true` when `NODE_ENV=production`, unless `ALLOW_DEV_LOGIN_IN_PRODUCTION=true` is set as well. A flag set by mistake is found when the server starts, not by someone who logs in as another coach. The team decided to set it on the course server. +- **Only in a frontend built for it.** The buttons are shown only when the frontend was built with `VITE_DEV_LOGIN=true`. A build without it shows "Login is not turned on in this version of the app" and no form that cannot work. +- **Says what it is.** The form says "anyone can log in as anyone, so don’t enter real data". + +### What it does not protect + +- **Anyone can read and change the data of any test coach** by typing its address. Nothing in the app is private. Do not enter real data. +- **The tokens go over http.** Someone on the same network can read an access token or a refresh token and use it as the coach until it expires. The login page says so when it is served over http from another computer than the one you are on (`isInsecureConnection`, `src/lib/insecureConnection.ts`). The tokens are short-lived and a refresh token that is used twice ends the login, which limits the harm; it does not remove it. +- **An access token works for up to 15 minutes after logout.** It is only checked against its signature and expiry, not against the database (see "What the server guarantees" in [API.md](API.md)). + +## The demo coach + +For the staff and the students who review the project, a coach with data is better than an empty page. + +- `npm run db:demo` (after `npm run db:seed`) makes the coach `demo@wag-planner.example` and gives it three gymnasts, each with three elements in their list. It can be run again whenever the demo coach should start over: **everything the demo coach has is deleted first**, so what visitors added or deleted is gone. Other coaches are never touched. +- The demo coach is a coach of the test login, so it needs `DEV_LOGIN=true` on the server like any other test coach. It has no password. +- **Everyone who uses it shares the same data.** That is why the page tells the visitor, and why the data is example data. +- It can **save and delete things**: add a gymnast, and delete one after a confirmation. Deleting a gymnast also deletes its routines and list of elements (`ON DELETE CASCADE`), which the confirmation says. +- The values are in `shared/demo.ts` and the script in `backend/src/seed/demo.ts`. + +## One source for the login state + +Before, the menu kept `isLoggedIn` in a local `useState`, while the gymnasts page asked the server (`me`). They could disagree: after a reload the session was restored but the local value started as `false`, so a logged-in coach was sent away from `/gymnasts`; and the menu could say "Log out" while the page said "Log in". + +Now there is one question and one answer: + +``` +AuthProvider --asks--> me { id name email } (the server) + | + useAuth() -> { status: 'loading' | 'authenticated' | 'anonymous', coach } + | + +--> Navbar Log in / Log out / the Gymnasts link + +--> ProtectedRoute waits, lets the coach in, or sends them to /login + +--> YourGymnastsPage the "Log in" message if the session ends +``` + +- **`loading`** is the time before the first answer. The menu shows neither "Log in" nor "Log out", and `ProtectedRoute` shows "Checking your login…" and does not send anyone away. This is what keeps a coach with a saved session on `/gymnasts` after a reload. +- **Log in and log out** reset the Apollo store (`resetStore`), which asks `me` again, so the menu and the page change at once. (`clearStore` would only clear the cache, and the menu would keep its old answer.) +- A visitor who is not logged in is sent to `/login` with the page they wanted in the state of the navigation. After the login they go there, but only if it is a path on this site (a `from` such as `//evil.example` is ignored). +- Logging out goes to `/elements` first, so the page the coach leaves does not send them to the login page. + +The whole path (log in, save a gymnast, reload, log out, log in again, see the gymnast, delete it) is tested as one test in `src/App.flow.test.tsx`. + +## What we would do in a real production system + +With https and a real login, in this order of importance: + +1. **TLS everywhere.** A certificate from Let's Encrypt (`certbot`), a redirect from http to https, and `Strict-Transport-Security` so the browser never goes back to http. Without TLS nothing below protects the user. (Let's Encrypt can check a domain with a DNS record, so it does not need the server to be reachable from the internet, but it needs a domain we control, which the course server's name is not.) +2. **A login that proves who the user is.** Sign in with Google (the server side exists), or our own accounts. For passwords, follow the [OWASP Password Storage Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html): **Argon2id** with at least 19 MiB of memory, 2 passes and 1 lane (Node 24 has `crypto.argon2`), a salt for each password, stored as a PHC string so the settings can be raised later. Follow the [OWASP Authentication Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Authentication_Cheat_Sheet.html) for the rules: at least 15 characters (8 with a second factor), at most 64 or more, no rules about upper case or symbols, a check against leaked passwords, and **the same answer and the same time** for a wrong address and a wrong password. +3. **A second factor or passkeys.** The best defence against stolen or guessed passwords. +4. **Limit the attempts**, per account and per address, with a lock that ends, and log the failures. +5. **E-mail verification and a way to reset the password**, which needs a mail server. +6. **The refresh token in an `httpOnly`, `Secure`, `SameSite` cookie**, not in `localStorage`, so a script injected into the page cannot read it. The app and the API are on the same address (the Apache proxy), so this works without CORS; it needs CSRF protection for the cookie, and changes in `login`, `refreshSession` and `logout`. We kept the tokens in the response body for now (see "Identifying the user" in [API.md](API.md)). +7. **Check the access token against the database** if "log out" must act at once, at the cost of a lookup for every request. +8. **Remove the test login**, or keep it only for development. + +## Where the code is + +| What | Where | +| -------------------------------- | ------------------------------------------------------------------------ | +| The question `me` | `src/lib/authOperations.ts` | +| The provider and the hook | `src/components/auth/AuthProvider.tsx`, `src/components/auth/useAuth.ts` | +| The login page | `src/components/auth/LoginPage.tsx` | +| The test login form | `src/components/gymnasts/DevLoginButton.tsx` | +| The protected pages | `src/components/ProtectedRoute.tsx`, `src/App.tsx` | +| The menu | `src/components/navbar/Navbar.tsx` | +| Log in, log out, reset the store | `src/lib/apollo/client.ts` | +| The http warning | `src/lib/insecureConnection.ts` | +| Delete a gymnast | `src/components/gymnasts/DeleteGymnastButton.tsx` | +| The demo coach | `shared/demo.ts`, `backend/src/seed/demo.ts` (`npm run db:demo`) | +| `devLogin` on the server | `backend/src/server/resolvers/auth.ts`, `backend/src/server/config.ts` | diff --git a/docs/UI_PLANS.md b/docs/UI_PLANS.md index 6035dc2..08048a3 100644 --- a/docs/UI_PLANS.md +++ b/docs/UI_PLANS.md @@ -35,16 +35,16 @@ The search page is the start page for both user groups. Mockups of some of the pages are in [design/README.md](design/README.md). -| Page | Logged out | Logged in | Description | -| ------------------- | ---------- | --------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| **Search** | ✓ | ✓ | Search, filter and sort all elements and vaults. Logged-in coaches can add an element directly to one of their gymnasts' routines, or add it to a gymnast's list of elements. | -| **Element details** | ✓ | ✓ | All information about one element or vault: code, value, description, image, and its variants if it has any. | -| **Calculator** | ✓ | ✓ | Put together a routine on one apparatus and see its D-value. The routine is kept only while the page is open. Logged-in coaches can save it as a routine for one of their gymnasts. | -| **About** | ✓ | ✓ | What the website does, who it is for, and the source of the data. | -| **Log in** | ✓ | | Log in with Google. The first login creates the coach account. | -| **Gymnasts** | | ✓ | Overview of the coach's gymnasts, and a form to create a new gymnast. | -| **Gymnast** | | ✓ | One gymnast's routines on each apparatus with their D-values, and the elements she can do or is practicing. | -| **Your profile** | | ✓ | Account details from Google (name, email) and the date you joined. Log out. | +| Page | Logged out | Logged in | Description | +| ------------------- | ---------- | --------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Search** | ✓ | ✓ | Search, filter and sort all elements and vaults. Logged-in coaches can add an element directly to one of their gymnasts' routines, or add it to a gymnast's list of elements. | +| **Element details** | ✓ | ✓ | All information about one element or vault: code, value, description, image, and its variants if it has any. | +| **Calculator** | ✓ | ✓ | Put together a routine on one apparatus and see its D-value. The routine is kept only while the page is open. Logged-in coaches can save it as a routine for one of their gymnasts. | +| **About** | ✓ | ✓ | What the website does, who it is for, and the source of the data. | +| **Log in** | ✓ | | Log in. On the course server (plain http) this is the test login: the demo coach with one click, or a test coach made from an e-mail address (see [AUTHENTICATION.md](AUTHENTICATION.md)). A visitor who opens a page for coaches is sent here and goes back to it after the login. | +| **Gymnasts** | | ✓ | Overview of the coach's gymnasts, and a form to create a new gymnast. | +| **Gymnast** | | ✓ | One gymnast's routines on each apparatus with their D-values, and the elements she can do or is practicing. | +| **Your profile** | | ✓ | Account details from Google (name, email) and the date you joined. Log out. | ## Scope diff --git a/docs/VM_SETUP.md b/docs/VM_SETUP.md index 511dbb4..45e8b6d 100644 --- a/docs/VM_SETUP.md +++ b/docs/VM_SETUP.md @@ -167,6 +167,16 @@ The VM serves plain http, so Sign in with Google does not work there (Google onl VITE_API_URL=/project2/graphql VITE_DEV_LOGIN=true npm run build ``` -3. Open "Your gymnasts" without being logged in. The page shows the warning and asks for an e-mail address. Everyone should use their own address, otherwise they share the same gymnasts. +3. Make the demo coach, once the catalog is seeded (see "4. Run Migrations & Initial Seed"): + + ```bash + npm run db:demo + ``` + + It is a coach with three example gymnasts. Run it again whenever the demo coach should start over (it deletes what the demo coach has and nothing else). Everyone who reviews the project shares the demo coach, so one reviewer can empty it for the next: **run it before each review round**. + +4. Open the site and click "Log in". The login page has **"Log in as the demo coach"** (one click) and a form for a test coach of your own, with the warning that anyone can log in as anyone. Everyone who uses their own address should use a different one, otherwise they share the same gymnasts. The page also says that it is not encrypted, because the server uses plain http. + +Why this is a test login, what it does not protect and what we would do with https is in [`AUTHENTICATION.md`](AUTHENTICATION.md). To turn it off, remove the two lines from `backend/.env`, restart the backend, and build the frontend again without `VITE_DEV_LOGIN`. diff --git a/package.json b/package.json index 9353481..331bb02 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,7 @@ "db:deploy": "npm run prisma:deploy -w backend", "db:seed": "npm run prisma:seed -w backend", "db:studio": "npm run prisma:studio -w backend", + "db:demo": "npm run demo -w backend", "db:synthetic": "npm run synthetic -w backend --", "test:run": "vitest run", "test:ui": "vitest --ui", diff --git a/shared/demo.ts b/shared/demo.ts new file mode 100644 index 0000000..cb31913 --- /dev/null +++ b/shared/demo.ts @@ -0,0 +1,15 @@ +// The demo coach (see "The demo coach" in docs/AUTHENTICATION.md): a coach with +// example data, so that the staff and the students who review the project can +// look at the gymnast pages and save and delete things without typing an +// address. It is a coach of the test login (`devLogin`), so it needs no +// password. `npm run db:demo` makes it and puts the example data back +// (backend/src/seed/demo.ts), and the login page has a button for it. +// +// This file must not import anything: it is compiled by the backend and by +// the frontend. + +export const DEMO_EMAIL = 'demo@wag-planner.example' +export const DEMO_NAME = 'Demo Coach' + +/** The gymnasts the demo coach starts with, and gets back from `npm run db:demo`. */ +export const DEMO_GYMNASTS = ['Emma Berg', 'Sofie Hansen', 'Nora Lie'] diff --git a/src/App.flow.test.tsx b/src/App.flow.test.tsx new file mode 100644 index 0000000..f834e55 --- /dev/null +++ b/src/App.flow.test.tsx @@ -0,0 +1,211 @@ +// @vitest-environment jsdom +// The whole path of a coach, as one test: log in, save a gymnast, reload the page, +// log out, log in again and see the gymnast, delete it. The app and the real +// Apollo Client run as in the browser; only the server is replaced by a stub that +// keeps the state of the login and the gymnasts, as the real server does, so a +// reload (a new client with an empty cache) must ask it again. + +import { ApolloProvider } from '@apollo/client/react' +import { + cleanup, + fireEvent, + render, + screen, + waitFor, + within, + configure, +} from '@testing-library/react' +import { BrowserRouter } from 'react-router-dom' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createTestClient, ok, type Handler } from '@/tests/apollo' +import App from './App' + +const BASE = import.meta.env.BASE_URL + +/** + * The server: who is logged in and the gymnasts of the coach. The functions + * that the app calls to log in and out change this state and reset the client, + * as the real ones do (src/lib/apollo/client.ts). + */ +const server = vi.hoisted(() => ({ + loggedIn: false, + gymnasts: [] as { id: string; name: string }[], + nextId: 1, + client: null as null | { resetStore: () => Promise }, +})) + +vi.mock('@/lib/apollo/client', async (importOriginal) => ({ + ...(await importOriginal()), + devLogin: vi.fn(async () => { + server.loggedIn = true + await server.client?.resetStore() + return { id: 'coach-1', name: 'Demo Coach', email: 'demo@example.com' } + }), + logout: vi.fn(async () => { + server.loggedIn = false + await server.client?.resetStore() + }), +})) + +const coach = { + __typename: 'Coach', + id: 'coach-1', + name: 'Demo Coach', + email: 'demo@example.com', +} + +const handler: Handler = ({ operationName, variables }) => { + switch (operationName) { + case 'Me': + return ok({ me: server.loggedIn ? coach : null }) + case 'YourGymnasts': + return ok({ + me: server.loggedIn + ? { + __typename: 'Coach', + id: coach.id, + gymnasts: [...server.gymnasts] + .sort((a, b) => a.name.localeCompare(b.name)) + .map((g) => ({ __typename: 'Gymnast', ...g })), + } + : null, + }) + case 'CreateGymnast': { + const gymnast = { + id: `gymnast-${server.nextId++}`, + name: String(variables['name']), + } + server.gymnasts.push(gymnast) + return ok({ createGymnast: { __typename: 'Gymnast', ...gymnast } }) + } + case 'DeleteGymnast': + server.gymnasts = server.gymnasts.filter((g) => g.id !== variables['id']) + return ok({ deleteGymnast: variables['id'] }) + case 'CatalogStructure': + // The Elements page asks for the filters; this test is not about them. + return ok({ apparatus: [] }) + case 'SearchCatalog': + // Logging out leaves for the Elements page, which searches. It is not what + // this test is about, so the catalog is empty. + return ok({ + searchCatalog: { + __typename: 'CatalogConnection', + totalCount: 0, + pageInfo: { + __typename: 'PageInfo', + hasNextPage: false, + endCursor: null, + }, + edges: [], + }, + }) + default: + throw new Error(`The stub does not know ${operationName}.`) + } +} + +/** Opens the app as a new page load does: a new client with an empty cache. */ +function openApp(path: string) { + window.history.replaceState({}, '', `${BASE}${path}`) + const { client } = createTestClient(handler) + server.client = client + render( + + + + + , + ) +} + +const pathname = () => window.location.pathname.slice(BASE.length - 1) + +// One test walks through the whole app, so it needs more time than the default +// when all the test files run at the same time. +vi.setConfig({ testTimeout: 30_000 }) + +beforeEach(() => { + configure({ asyncUtilTimeout: 8000 }) + vi.stubEnv('VITE_DEV_LOGIN', 'true') + server.loggedIn = false + server.gymnasts = [] + server.nextId = 1 + window.localStorage.clear() +}) + +afterEach(() => { + configure({ asyncUtilTimeout: 1000 }) + cleanup() + vi.unstubAllEnvs() + window.history.replaceState({}, '', BASE) +}) + +describe('a coach’s way through the app', () => { + it('logs in, saves a gymnast, keeps it after a reload and after a new login, and deletes it', async () => { + // 1. A visitor who is not logged in is asked to log in, and the menu agrees. + openApp('gymnasts') + expect(await screen.findByRole('heading', { name: 'Log in' })).toBeTruthy() + expect(pathname()).toBe('/login') + expect(screen.getByRole('link', { name: 'Log in' })).toBeTruthy() + expect(screen.queryByRole('link', { name: 'Gymnasts' })).toBeNull() + + // 2. One click logs in as the demo coach, and lands on the page they wanted. + fireEvent.click( + screen.getByRole('button', { name: 'Log in as the demo coach' }), + ) + expect( + await screen.findByRole('heading', { name: 'Your gymnasts' }), + ).toBeTruthy() + expect(pathname()).toBe('/gymnasts') + expect(screen.getByRole('button', { name: 'Log out' })).toBeTruthy() + expect(screen.getByText('You haven’t added any gymnasts yet.')).toBeTruthy() + + // 3. A gymnast is saved. + fireEvent.click(screen.getByRole('button', { name: 'Add gymnast' })) + fireEvent.change(screen.getByLabelText('Gymnast name'), { + target: { value: 'Anna Lie' }, + }) + fireEvent.click(screen.getByRole('button', { name: 'Save gymnast' })) + expect( + await screen.findByRole('heading', { name: 'Anna Lie' }), + ).toBeTruthy() + expect(server.gymnasts.map((g) => g.name)).toEqual(['Anna Lie']) + + // 4. A reload: a new client that knows nothing, a saved login. The coach stays + // on the page, the menu still says Log out, and the gymnast is there. + cleanup() + openApp('gymnasts') + expect( + await screen.findByRole('heading', { name: 'Anna Lie' }), + ).toBeTruthy() + expect(pathname()).toBe('/gymnasts') + expect(screen.getByRole('button', { name: 'Log out' })).toBeTruthy() + + // 5. Logging out leaves the page and the menu says Log in. + fireEvent.click(screen.getByRole('button', { name: 'Log out' })) + expect(await screen.findByRole('link', { name: 'Log in' })).toBeTruthy() + expect(pathname()).toBe('/elements') + expect(screen.queryByRole('link', { name: 'Gymnasts' })).toBeNull() + + // 6. Logging in again shows the same gymnast: it is saved for the coach, on the server. + fireEvent.click(screen.getByRole('link', { name: 'Log in' })) + fireEvent.click( + await screen.findByRole('button', { name: 'Log in as the demo coach' }), + ) + expect( + await screen.findByRole('heading', { name: 'Anna Lie' }), + ).toBeTruthy() + expect(pathname()).toBe('/gymnasts') + + // 7. The gymnast is deleted after a confirmation, and the page says so. + fireEvent.click(screen.getByRole('button', { name: 'Delete Anna Lie' })) + const dialog = await screen.findByRole('alertdialog') + fireEvent.click(within(dialog).getByRole('button', { name: 'Yes, delete' })) + await waitFor(() => + expect(screen.queryByRole('heading', { name: 'Anna Lie' })).toBeNull(), + ) + expect(screen.getByText('Anna Lie deleted.')).toBeTruthy() + expect(screen.getByText('You haven’t added any gymnasts yet.')).toBeTruthy() + expect(server.gymnasts).toEqual([]) + }) +}) diff --git a/src/App.test.tsx b/src/App.test.tsx index 52730c2..74e8b3a 100644 --- a/src/App.test.tsx +++ b/src/App.test.tsx @@ -3,7 +3,7 @@ import { ApolloProvider } from '@apollo/client/react'; import { cleanup, fireEvent, render, screen } from '@testing-library/react'; import { BrowserRouter } from 'react-router-dom'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { createTestClient } from '@/tests/apollo'; +import { createTestClient, ok, type Handler } from '@/tests/apollo'; import App from './App'; /** @@ -11,8 +11,8 @@ import App from './App'; * client is used; only the network is replaced by a request that never * answers, so the pages stay in their loading state. */ -const renderApp = () => { - const { client } = createTestClient(() => new Promise(() => {})); +const renderApp = (handler: Handler = () => new Promise(() => {})) => { + const { client } = createTestClient(handler); return render( @@ -118,18 +118,51 @@ describe('App', () => { } }); - it('guards the Gymnasts route and makes it available after login', () => { + it('sends a visitor who is not logged in from the Gymnasts route to the login page', async () => { window.history.replaceState({}, '', `${import.meta.env.BASE_URL}gymnasts`); - renderApp(); + renderApp(() => ok({ me: null })); - // Logged out, /gymnasts goes to the start page, which is the Elements page. - expect(window.location.pathname).toBe(`${import.meta.env.BASE_URL}elements`); + expect(await screen.findByRole('heading', { name: 'Log in' })).toBeTruthy(); + expect(window.location.pathname).toBe(`${import.meta.env.BASE_URL}login`); expect(screen.queryByRole('link', { name: 'Gymnasts' })).toBeNull(); + expect(screen.getByRole('link', { name: 'Log in' })).toBeTruthy(); + }); - fireEvent.click(screen.getByRole('button', { name: 'Log in' })); + it('keeps a logged-in coach on the Gymnasts route after a reload, and the menu agrees with the page', async () => { + window.history.replaceState({}, '', `${import.meta.env.BASE_URL}gymnasts`); + renderApp((operation) => + ok({ + me: { + __typename: 'Coach', + id: 'coach-1', + name: 'Anna', + email: 'anna@example.com', + ...(operation.operationName === 'YourGymnasts' && { + gymnasts: [{ __typename: 'Gymnast', id: 'g1', name: 'Emma Berg' }], + }), + }, + }), + ); - fireEvent.click(screen.getByRole('link', { name: 'Gymnasts' })); + expect( + await screen.findByRole('heading', { name: 'Your gymnasts' }), + ).toBeTruthy(); + expect(window.location.pathname).toBe(`${import.meta.env.BASE_URL}gymnasts`); + // The menu and the page read the same answer. + expect(screen.getByRole('button', { name: 'Log out' })).toBeTruthy(); + expect(screen.queryByRole('link', { name: 'Log in' })).toBeNull(); + expect(screen.getByRole('link', { name: 'Gymnasts' })).toBeTruthy(); + expect(await screen.findByText('Emma Berg')).toBeTruthy(); + }); + + it('does not send a visitor away before the server has said who is logged in', async () => { + window.history.replaceState({}, '', `${import.meta.env.BASE_URL}gymnasts`); + renderApp(); // the server never answers - expect(screen.getByRole('heading', { name: 'Your gymnasts' })).toBeTruthy(); + expect(await screen.findByText('Checking your login…')).toBeTruthy(); + expect(window.location.pathname).toBe(`${import.meta.env.BASE_URL}gymnasts`); + // Neither "Log in" nor "Log out" is true yet. + expect(screen.queryByRole('link', { name: 'Log in' })).toBeNull(); + expect(screen.queryByRole('button', { name: 'Log out' })).toBeNull(); }); }); diff --git a/src/App.tsx b/src/App.tsx index 34da14f..fdc7b72 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -3,6 +3,8 @@ import { Navigate, Route, Routes } from 'react-router-dom'; import { Navbar } from './components/navbar/Navbar'; import { SkipLink } from './components/skipLink/SkipLink'; import { ProtectedRoute } from './components/ProtectedRoute'; +import { AuthProvider } from './components/auth/AuthProvider'; +import LoginPage from './components/auth/LoginPage'; import PageTitle from './components/PageTitle'; import ElementDetailPage from './components/elements/ElementDetailPage'; @@ -35,7 +37,6 @@ const NotFoundPage = () => ( ); export function App() { - const [isLoggedIn, setIsLoggedIn] = useState(false); const [isDarkMode, setIsDarkMode] = useState( () => window.localStorage.getItem('theme') === 'dark', ); @@ -50,11 +51,9 @@ export function App() { }, [isDarkMode]); return ( - <> + setIsDarkMode((current) => !current)} /> @@ -67,16 +66,17 @@ export function App() { } /> } /> + } /> {/* Protected routes requiring login */} - }> + }> } /> } /> - + ); } diff --git a/src/components/ProtectedRoute.test.tsx b/src/components/ProtectedRoute.test.tsx new file mode 100644 index 0000000..d45ab01 --- /dev/null +++ b/src/components/ProtectedRoute.test.tsx @@ -0,0 +1,73 @@ +// @vitest-environment jsdom +import { cleanup, render, screen } from '@testing-library/react' +import { MemoryRouter, Route, Routes, useLocation } from 'react-router-dom' +import { afterEach, describe, expect, it } from 'vitest' +import { AuthContext, type AuthState } from '@/components/auth/useAuth' +import { ProtectedRoute } from './ProtectedRoute' + +afterEach(cleanup) + +const states = { + loading: { status: 'loading', coach: null }, + anonymous: { status: 'anonymous', coach: null }, + authenticated: { + status: 'authenticated', + coach: { id: 'c1', name: 'Anna', email: 'anna@example.com' }, + }, +} satisfies Record + +/** Where the visitor ended up, and what the login page was told about where they came from. */ +function LoginStub() { + const location = useLocation() + const from = ( + location.state as { from?: { pathname: string; search: string } } | null + )?.from + return ( +

+ login, from {from?.pathname} + {from?.search} +

+ ) +} + +function renderAt(state: AuthState, entry = '/gymnasts?tab=1') { + render( + + + + } /> + }> + the gymnasts

} /> + + + + , + ) +} + +describe('ProtectedRoute', () => { + it('shows the page to a coach who is logged in', () => { + renderAt(states.authenticated) + + expect(screen.getByText('the gymnasts')).toBeTruthy() + }) + + it('sends a visitor who is not logged in to the login page, and remembers the page they wanted', () => { + renderAt(states.anonymous) + + expect(screen.getByTestId('login').textContent).toBe( + 'login, from /gymnasts?tab=1', + ) + expect(screen.queryByText('the gymnasts')).toBeNull() + }) + + it('waits while the server has not said who is logged in, and does not send anyone away', () => { + renderAt(states.loading) + + expect(screen.getByRole('status').textContent).toBe('Checking your login…') + // Also while it waits, the tab says what the page is. + expect(document.title).toBe('Checking your login – WAG CoP Planner') + expect(screen.queryByTestId('login')).toBeNull() + expect(screen.queryByText('the gymnasts')).toBeNull() + }) +}) diff --git a/src/components/ProtectedRoute.tsx b/src/components/ProtectedRoute.tsx index 45b3799..c4b2876 100644 --- a/src/components/ProtectedRoute.tsx +++ b/src/components/ProtectedRoute.tsx @@ -1,18 +1,36 @@ -import React from 'react'; -import { Navigate, Outlet } from 'react-router-dom'; +import { Navigate, Outlet, useLocation } from 'react-router-dom'; +import { useAuth } from '@/components/auth/useAuth'; +import PageTitle from '@/components/PageTitle'; interface ProtectedRouteProps { - isLoggedIn: boolean; + /** Where an anonymous visitor is sent. The page they wanted is kept, so the login can send them back. */ redirectPath?: string; } -export const ProtectedRoute: React.FC = ({ - isLoggedIn, - redirectPath = '/', -}) => { - if (!isLoggedIn) { - return ; +/** + * The pages inside are for a logged-in coach. It reads the same `useAuth()` as + * the menu and the pages, so they cannot disagree. While the answer from the + * server is on its way the visitor is not sent away: a reload of /gymnasts with + * a saved session must stay on /gymnasts. + */ +export const ProtectedRoute = ({ + redirectPath = '/login', +}: ProtectedRouteProps) => { + const { status } = useAuth(); + const location = useLocation(); + + if (status === 'loading') { + return ( +
+ Checking your login +

Checking your login…

+
+ ); + } + + if (status === 'anonymous') { + return ; } return ; -}; \ No newline at end of file +}; diff --git a/src/components/auth/AuthProvider.tsx b/src/components/auth/AuthProvider.tsx new file mode 100644 index 0000000..8af3140 --- /dev/null +++ b/src/components/auth/AuthProvider.tsx @@ -0,0 +1,26 @@ +import { useMemo, type ReactNode } from 'react' +import { useQuery } from '@apollo/client/react' +import { ME_QUERY } from '@/lib/authOperations' +import { AuthContext, type AuthState } from './useAuth' + +/** + * The one place that asks the server who is logged in (`me`). Everything that + * depends on the login reads `useAuth()`: the menu, the protected pages and the + * gymnasts page. Logging in and out resets the Apollo store (see + * src/lib/apollo/client.ts), which asks this question again, so all of them + * change at once. + * + * A network error is shown as "anonymous": the page then asks the visitor to + * log in, and the pages that need the server show their own error. + */ +export function AuthProvider({ children }: { children: ReactNode }) { + const { data, loading } = useQuery(ME_QUERY) + + const state = useMemo(() => { + if (data?.me) return { status: 'authenticated', coach: data.me } + if (loading && !data) return { status: 'loading', coach: null } + return { status: 'anonymous', coach: null } + }, [data, loading]) + + return {children} +} diff --git a/src/components/auth/LoginPage.test.tsx b/src/components/auth/LoginPage.test.tsx new file mode 100644 index 0000000..31d07da --- /dev/null +++ b/src/components/auth/LoginPage.test.tsx @@ -0,0 +1,231 @@ +// @vitest-environment jsdom +import { + cleanup, + fireEvent, + render, + screen, + waitFor, +} from '@testing-library/react' +import { MemoryRouter, Route, Routes, useLocation } from 'react-router-dom' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { AuthContext, type AuthState } from '@/components/auth/useAuth' +import { devLogin } from '@/lib/apollo/client' +import { ApiError } from '@/lib/apollo/api' +import { DEMO_EMAIL } from '../../../shared/demo' +import LoginPage from './LoginPage' + +// The real function would call the server. The page only has to call devLogin. +vi.mock('@/lib/apollo/client', () => ({ devLogin: vi.fn() })) + +const logIn = vi.mocked(devLogin) + +afterEach(() => { + cleanup() + vi.resetAllMocks() + vi.unstubAllEnvs() +}) + +const anonymous: AuthState = { status: 'anonymous', coach: null } +const coach = { id: 'c1', name: 'Demo Coach', email: DEMO_EMAIL } + +/** Shows the address, so a test can see where the visitor went. */ +function Address() { + const location = useLocation() + return ( + {location.pathname + location.search} + ) +} + +function renderPage({ + auth = anonymous, + entry = '/login' as string | { pathname: string; state: unknown }, +} = {}) { + render( + + + + } /> + somewhere else

} /> + +
+ + , + ) +} + +const address = () => screen.getByTestId('address').textContent + +describe('LoginPage: the title of the page', () => { + it('has a title that says what the page is, so the tab does not say the name of the project (WCAG 2.4.2)', () => { + vi.stubEnv('VITE_DEV_LOGIN', 'true') + renderPage() + + expect(document.title).toBe('Log in – WAG CoP Planner') + }) +}) + +describe('LoginPage: a build without the test login', () => { + it('says that login is not turned on, and shows no form that cannot work', () => { + vi.stubEnv('VITE_DEV_LOGIN', '') + renderPage() + + expect( + screen.getByRole('heading', { level: 1, name: 'Log in' }), + ).toBeTruthy() + expect(screen.getByText(/not turned on/)).toBeTruthy() + expect(screen.queryByRole('button', { name: /demo coach/ })).toBeNull() + expect(screen.queryByRole('form', { name: 'Test login' })).toBeNull() + }) +}) + +describe('LoginPage: a build with the test login', () => { + it('has the demo coach and a test coach of the visitor’s own, each under a heading', () => { + vi.stubEnv('VITE_DEV_LOGIN', 'true') + renderPage() + + expect( + screen.getByRole('heading', { + level: 2, + name: 'Try it with the demo coach', + }), + ).toBeTruthy() + expect( + screen.getByRole('button', { name: 'Log in as the demo coach' }), + ).toBeTruthy() + expect( + screen.getByRole('heading', { + level: 2, + name: 'Or use your own test coach', + }), + ).toBeTruthy() + expect(screen.getByRole('form', { name: 'Test login' })).toBeTruthy() + // The test login says what it is, so nobody enters anything private. + expect(screen.getByText(/anyone can log in as anyone/)).toBeTruthy() + }) + + it('logs in as the demo coach with one click and goes to the gymnasts', async () => { + vi.stubEnv('VITE_DEV_LOGIN', 'true') + logIn.mockResolvedValue(coach) + renderPage() + + fireEvent.click( + screen.getByRole('button', { name: 'Log in as the demo coach' }), + ) + + await waitFor(() => expect(address()).toBe('/gymnasts')) + expect(logIn).toHaveBeenCalledExactlyOnceWith(DEMO_EMAIL) + }) + + it('goes back to the page the visitor wanted, with its search, after the login', async () => { + vi.stubEnv('VITE_DEV_LOGIN', 'true') + logIn.mockResolvedValue(coach) + renderPage({ + entry: { + pathname: '/login', + state: { from: { pathname: '/gymnasts', search: '?x=1' } }, + }, + }) + + fireEvent.click( + screen.getByRole('button', { name: 'Log in as the demo coach' }), + ) + + await waitFor(() => expect(address()).toBe('/gymnasts?x=1')) + }) + + it('does not follow a "from" that leaves the site', async () => { + vi.stubEnv('VITE_DEV_LOGIN', 'true') + logIn.mockResolvedValue(coach) + renderPage({ + entry: { + pathname: '/login', + state: { from: { pathname: '//evil.example' } }, + }, + }) + + fireEvent.click( + screen.getByRole('button', { name: 'Log in as the demo coach' }), + ) + + await waitFor(() => expect(address()).toBe('/gymnasts')) + }) + + it('logs in as a test coach of the visitor’s own', async () => { + vi.stubEnv('VITE_DEV_LOGIN', 'true') + logIn.mockResolvedValue({ ...coach, email: 'emma@example.com' }) + renderPage() + + fireEvent.change(screen.getByLabelText('Your e-mail address'), { + target: { value: 'emma@example.com' }, + }) + fireEvent.click( + screen.getByRole('button', { name: 'Log in as test coach' }), + ) + + await waitFor(() => expect(address()).toBe('/gymnasts')) + expect(logIn).toHaveBeenCalledWith('emma@example.com') + }) + + it('tells the visitor when the server has the test login turned off, and stays on the page', async () => { + vi.stubEnv('VITE_DEV_LOGIN', 'true') + logIn.mockRejectedValue( + new ApiError('NOT_IMPLEMENTED', 'devLogin is not available yet.'), + ) + renderPage() + + fireEvent.click( + screen.getByRole('button', { name: 'Log in as the demo coach' }), + ) + + expect((await screen.findByRole('alert')).textContent).toBe( + 'This is not available yet.', + ) + expect(address()).toBe('/login') + // The button works again, for another try. + expect( + screen.getByRole('button', { + name: 'Log in as the demo coach', + }).disabled, + ).toBe(false) + }) + + it('makes only one login when the button is clicked twice', async () => { + vi.stubEnv('VITE_DEV_LOGIN', 'true') + logIn.mockReturnValue(new Promise(() => {})) + renderPage() + + const button = screen.getByRole('button', { + name: 'Log in as the demo coach', + }) + fireEvent.click(button) + fireEvent.click(button) + + expect(logIn).toHaveBeenCalledOnce() + expect( + screen.getByRole('button', { name: 'Logging in…' }) + .disabled, + ).toBe(true) + }) +}) + +describe('LoginPage: who is already logged in', () => { + it('sends a coach who is logged in on to the gymnasts, and shows no form', () => { + vi.stubEnv('VITE_DEV_LOGIN', 'true') + renderPage({ auth: { status: 'authenticated', coach } }) + + expect(address()).toBe('/gymnasts') + expect( + screen.queryByRole('button', { name: 'Log in as the demo coach' }), + ).toBeNull() + }) +}) + +describe('LoginPage: plain http', () => { + it('does not warn on localhost, where nothing leaves the computer', () => { + vi.stubEnv('VITE_DEV_LOGIN', 'true') + renderPage() + + // jsdom serves the page from http://localhost. + expect(screen.queryByTestId('insecure-connection')).toBeNull() + }) +}) diff --git a/src/components/auth/LoginPage.tsx b/src/components/auth/LoginPage.tsx new file mode 100644 index 0000000..c60e17d --- /dev/null +++ b/src/components/auth/LoginPage.tsx @@ -0,0 +1,142 @@ +import { useState } from 'react' +import { Navigate, useLocation, useNavigate } from 'react-router-dom' +import { TriangleAlert } from 'lucide-react' +import { useAuth } from '@/components/auth/useAuth' +import PageTitle from '@/components/PageTitle' +import DevLoginButton from '@/components/gymnasts/DevLoginButton' +import { Button } from '@/components/ui/button' +import { PILL } from '@/components/ui/pill' +import { devLogin } from '@/lib/apollo/client' +import { describeError } from '@/lib/apollo/errors' +import { isDevLoginEnabled } from '@/lib/devLogin' +import { isInsecureConnection } from '@/lib/insecureConnection' +import { DEMO_EMAIL } from '../../../shared/demo' + +/** Where the visitor goes after logging in when they did not come from a protected page. */ +const DEFAULT_TARGET = '/gymnasts' + +const HINT = 'text-sm text-(--text-color)' + +/** + * Where to go after the login: the page the visitor wanted (ProtectedRoute puts + * it in the state of the navigation), or the gymnasts. It is only used when it + * is a path on this site, so a link cannot send the visitor somewhere else. + */ +function targetOf(state: unknown): string { + const from = ( + state as { from?: { pathname?: string; search?: string } } | null + )?.from + const path = from?.pathname + if (!path || !path.startsWith('/') || path.startsWith('//')) { + return DEFAULT_TARGET + } + return path + (from?.search ?? '') +} + +/** + * The login page. The only login the app has is the test login (`devLogin`): + * Sign in with Google needs https, and the course server has only http. Why we + * chose this, how it is guarded, and what a real system would do instead is in + * docs/AUTHENTICATION.md. The page offers two ways in: the demo coach, which has + * example gymnasts, and a coach of the visitor's own, made from an e-mail + * address. When a build has no test login (VITE_DEV_LOGIN is not "true") the + * page says so, and does not show a form that cannot work. + */ +export default function LoginPage() { + const auth = useAuth() + const location = useLocation() + const navigate = useNavigate() + const target = targetOf(location.state) + const [isLoggingIn, setIsLoggingIn] = useState(false) + const [error, setError] = useState('') + + // Already logged in (also right after the login): go on to where the visitor + // was going. + if (auth.status === 'authenticated') { + return + } + + async function logInAsDemo() { + if (isLoggingIn) return + setIsLoggingIn(true) + setError('') + try { + await devLogin(DEMO_EMAIL) + void navigate(target, { replace: true }) + } catch (problem) { + setError(describeError(problem).message) + } finally { + setIsLoggingIn(false) + } + } + + return ( +
+ Log in +
+

Log in

+

+ Log in to see and change your gymnasts. +

+ + {isInsecureConnection() && ( +
+
+ )} + + {isDevLoginEnabled() ? ( + <> +
+

Try it with the demo coach

+

+ The demo coach has example gymnasts. You can add and delete + things; everyone who uses the demo coach shares the same data. +

+ + {error && ( +

+ {error} +

+ )} +
+ +
+

Or use your own test coach

+ { + void navigate(target, { replace: true }) + }} + /> +
+ + ) : ( +

+ Login is not turned on in this version of the app. +

+ )} +
+
+ ) +} diff --git a/src/components/auth/useAuth.ts b/src/components/auth/useAuth.ts new file mode 100644 index 0000000..e6bf3b4 --- /dev/null +++ b/src/components/auth/useAuth.ts @@ -0,0 +1,23 @@ +import { createContext, useContext } from 'react' +import type { Coach } from '@/lib/authOperations' + +/** + * Who is logged in, as the server says it: + * - `loading`: the first answer is on its way, so the page does not know yet. + * A page must wait and not send the visitor away or show "Log in". + * - `authenticated`: `coach` is the logged-in coach. + * - `anonymous`: nobody is logged in, or the session has ended. + */ +export type AuthState = + | { status: 'loading'; coach: null } + | { status: 'authenticated'; coach: Coach } + | { status: 'anonymous'; coach: null } + +export const AuthContext = createContext(null) + +/** The state of the login. Every part of the app that needs it reads it here, so there is one source. */ +export function useAuth(): AuthState { + const state = useContext(AuthContext) + if (!state) throw new Error('useAuth must be used inside an AuthProvider.') + return state +} diff --git a/src/components/gymnasts/DeleteGymnastButton.test.tsx b/src/components/gymnasts/DeleteGymnastButton.test.tsx new file mode 100644 index 0000000..54e44d3 --- /dev/null +++ b/src/components/gymnasts/DeleteGymnastButton.test.tsx @@ -0,0 +1,237 @@ +// @vitest-environment jsdom +// The button runs on the real Apollo Client, with a stub for `fetch` (see +// src/tests/apollo.ts). + +import { ApolloProvider } from '@apollo/client/react' +import { + cleanup, + fireEvent, + render, + screen, + waitFor, + within, +} from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { createTestClient, failure, ok, type Handler } from '@/tests/apollo' +import DeleteGymnastButton from './DeleteGymnastButton' + +afterEach(cleanup) + +const gymnast = { id: 'gymnast-1', name: 'Emma Berg' } + +function renderButton(handler: Handler) { + const onDeleted = vi.fn() + const onFailed = vi.fn() + const { client, calls } = createTestClient(handler) + render( + + + , + ) + return { + onDeleted, + onFailed, + deletes: () => calls.filter((c) => c.operationName === 'DeleteGymnast'), + } +} + +const trigger = () => screen.getByRole('button', { name: 'Delete Emma Berg' }) +const confirmButton = () => screen.getByRole('button', { name: 'Yes, delete' }) + +describe('DeleteGymnastButton', () => { + it('is named after the gymnast, so a list of them can be told apart', () => { + renderButton(() => ok({ deleteGymnast: gymnast.id })) + + expect(trigger()).toBeTruthy() + expect(screen.queryByRole('alertdialog')).toBeNull() + }) + + it('asks before it deletes, and says that the routines go too and that it cannot be undone', async () => { + const page = renderButton(() => ok({ deleteGymnast: gymnast.id })) + + fireEvent.click(trigger()) + + const dialog = await screen.findByRole('alertdialog') + expect(dialog.textContent).toContain('Delete gymnast') + expect(dialog.textContent).toContain('You’ll permanently lose Emma Berg’s:') + expect(page.deletes()).toHaveLength(0) + }) + + it('closes with “No, keep it” and deletes nothing', async () => { + const page = renderButton(() => ok({ deleteGymnast: gymnast.id })) + fireEvent.click(trigger()) + await screen.findByRole('alertdialog') + + fireEvent.click(screen.getByRole('button', { name: 'No, keep it' })) + + await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()) + expect(page.deletes()).toHaveLength(0) + expect(page.onDeleted).not.toHaveBeenCalled() + }) + + it('deletes the gymnast when it is confirmed, closes, and tells the page', async () => { + const page = renderButton(() => ok({ deleteGymnast: gymnast.id })) + fireEvent.click(trigger()) + await screen.findByRole('alertdialog') + + fireEvent.click(confirmButton()) + + await waitFor(() => + expect(page.onDeleted).toHaveBeenCalledWith('Emma Berg'), + ) + expect(page.deletes()).toHaveLength(1) + expect(page.deletes()[0]?.variables).toEqual({ id: 'gymnast-1' }) + await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()) + expect(page.onFailed).not.toHaveBeenCalled() + }) + + it('tells the page why when the server refuses, and does not say that it was deleted', async () => { + const page = renderButton(() => + failure('NOT_FOUND', 'The gymnast does not exist.'), + ) + fireEvent.click(trigger()) + await screen.findByRole('alertdialog') + + fireEvent.click(confirmButton()) + + await waitFor(() => + expect(page.onFailed).toHaveBeenCalledWith('We could not find that.'), + ) + expect(page.onDeleted).not.toHaveBeenCalled() + }) +}) + +describe('DeleteGymnastButton: how it looks', () => { + it('is 48 by 48 CSS pixels, so it can be pressed on a phone (WCAG 2.2 asks for 24 at AA and 44 at AAA)', () => { + renderButton(() => ok({ deleteGymnast: gymnast.id })) + + const classes = trigger().className.split(/\s+/) + expect(classes).toContain('size-12') // 3rem = 48px + expect(classes).not.toContain('size-8') // the smaller size of the button, merged away + }) + + it('has a red icon that is bigger than the text beside it, and the red is a theme value (so it works in dark mode)', () => { + renderButton(() => ok({ deleteGymnast: gymnast.id })) + + expect(trigger().className).toContain('text-(--danger)') + expect(trigger().querySelector('svg')?.getAttribute('class')).toContain( + 'size-7', + ) // 28px + }) +}) + +describe('DeleteGymnastButton: how the dialog looks', () => { + async function openDialog() { + renderButton(() => ok({ deleteGymnast: gymnast.id })) + fireEvent.click(trigger()) + return screen.findByRole('alertdialog') + } + + it('has a warning icon, the title “Delete gymnast”, and then the text', async () => { + const dialog = await openDialog() + + expect(dialog.querySelector('svg')).toBeTruthy() + expect(dialog.querySelector('svg')?.getAttribute('aria-hidden')).toBe( + 'true', + ) + expect( + screen.getByRole('alertdialog', { name: 'Delete gymnast' }), + ).toBeTruthy() + const order = [...dialog.querySelectorAll('svg, [id]')].map( + (el) => el.tagName, + ) + expect(order[0]).toBe('svg') // the icon comes first + }) + + it('has the same rounding as the other cards (rounded-lg), buttons as pills like the rest of the site', async () => { + const dialog = await openDialog() + + expect(dialog.className).toContain('rounded-lg') + for (const name of ['No, keep it', 'Yes, delete']) { + expect(screen.getByRole('button', { name }).className).toContain( + 'rounded-full', + ) + } + }) + + it('has “No, keep it” first, so the first thing the keyboard reaches does not delete', async () => { + const dialog = await openDialog() + + const buttons = [...dialog.querySelectorAll('button')].map( + (b) => b.textContent, + ) + expect(buttons).toEqual(['No, keep it', 'Yes, delete']) + }) + + it('has “No, keep it” with an edge in a theme color, so it can be seen on the card in both themes', async () => { + await openDialog() + const keep = screen.getByRole('button', { name: 'No, keep it' }) + + expect(keep.className).toContain('border-2') + expect(keep.className).toContain('border-(--secondary-button-border)') + expect(keep.className).toContain('bg-(--secondary-button-bg)') + expect(keep.className).toContain('text-(--secondary-button-text)') + // Not the outline variant, whose dark mode colors would win over these. + expect(keep.className).not.toContain('dark:bg-input/30') + expect(keep.className).not.toContain('dark:border-input') + }) + + it('keeps the red of the site on “Yes, delete” and the shadow behind both buttons', async () => { + await openDialog() + const yes = screen.getByRole('button', { name: 'Yes, delete' }) + const keep = screen.getByRole('button', { name: 'No, keep it' }) + + expect(yes.className).toContain('bg-(--danger)') + expect(yes.className).toContain('text-(--danger-contrast)') + expect(yes.className).toContain('shadow-(--box-shadow)') + expect(keep.className).toContain('shadow-(--box-shadow)') + }) + + it('dims the page behind the dialog', async () => { + await openDialog() + + expect(document.querySelector('[class*="bg-black/40"]')).toBeTruthy() + }) +}) + +describe('DeleteGymnastButton: what is lost', () => { + it('says whose they are, and lists everything that is deleted with the gymnast', async () => { + renderButton(() => ok({ deleteGymnast: gymnast.id })) + fireEvent.click(trigger()) + const dialog = await screen.findByRole('alertdialog') + + const items = within(dialog).getAllByRole('listitem') + expect(items.map((item) => item.textContent)).toEqual([ + 'Profile', + 'Routines', + 'List of elements', + ]) + }) + + it('puts the list in a div, because a list cannot be inside a paragraph', async () => { + renderButton(() => ok({ deleteGymnast: gymnast.id })) + fireEvent.click(trigger()) + const dialog = await screen.findByRole('alertdialog') + + const list = dialog.querySelector('ul')! + expect(list.closest('p')).toBeNull() + // The dialog is still described by that text, for a screen reader. + const describedBy = dialog.getAttribute('aria-describedby')! + expect(document.getElementById(describedBy)?.contains(list)).toBe(true) + }) + + it('keeps the list left-aligned in the middle of the card, so the bullets line up', async () => { + renderButton(() => ok({ deleteGymnast: gymnast.id })) + fireEvent.click(trigger()) + const dialog = await screen.findByRole('alertdialog') + + const classes = dialog.querySelector('ul')!.className + expect(classes).toContain('text-left') + expect(classes).toContain('list-disc') + expect(classes).toContain('mx-auto') + }) +}) diff --git a/src/components/gymnasts/DeleteGymnastButton.tsx b/src/components/gymnasts/DeleteGymnastButton.tsx new file mode 100644 index 0000000..6108f67 --- /dev/null +++ b/src/components/gymnasts/DeleteGymnastButton.tsx @@ -0,0 +1,124 @@ +import { useState } from 'react' +import { useMutation } from '@apollo/client/react' +import { AlertDialog } from '@base-ui/react/alert-dialog' +import { Trash2, TriangleAlert } from 'lucide-react' +import { Button } from '@/components/ui/button' +import { describeError } from '@/lib/apollo/errors' +import { + DELETE_GYMNAST_MUTATION, + GYMNASTS_QUERY, +} from '@/lib/gymnastOperations' +import type { GymnastSummary } from '@/lib/gymnasts' + +/** + * Deletes a gymnast, after the coach has confirmed it in a dialog. Deleting + * cannot be undone and takes the gymnast's routines and list of elements with + * it, so a click on the icon only opens the question. The dialog is the Base UI + * alert dialog: the focus stays inside it, Escape closes it, and it is read as + * a dialog with a title and a description. The button is named after the + * gymnast, so a list of them can be told apart by a screen reader. + * + * `onDeleted` is called with the name when the server has deleted the gymnast, + * and `onFailed` with the message when it has not. The page tells the visitor + * and moves the focus, because this button is gone by then. + */ +export default function DeleteGymnastButton({ + gymnast, + onDeleted, + onFailed, +}: { + gymnast: GymnastSummary + onDeleted: (name: string) => void + onFailed: (message: string) => void +}) { + const [open, setOpen] = useState(false) + // The list is fetched again: the gymnast is gone from it, and the API sorts it. + const [deleteGymnast, { loading }] = useMutation(DELETE_GYMNAST_MUTATION, { + refetchQueries: [GYMNASTS_QUERY], + }) + + async function confirm() { + try { + await deleteGymnast({ variables: { id: gymnast.id } }) + setOpen(false) + onDeleted(gymnast.name) + } catch (problem) { + setOpen(false) + onFailed(describeError(problem).message) + } + } + + return ( + + + } + > + + + {/* The page behind is dimmed. */} + + {/* A card like the others on the site (rounded-lg), with the warning, the + question and the two answers. The first thing in it that can be reached + is "No, keep it", so Enter right away does not delete. */} + + + + + ) +} diff --git a/src/components/gymnasts/DevLoginButton.tsx b/src/components/gymnasts/DevLoginButton.tsx index 7bda913..9efb942 100644 --- a/src/components/gymnasts/DevLoginButton.tsx +++ b/src/components/gymnasts/DevLoginButton.tsx @@ -1,5 +1,6 @@ import { useState, type FormEvent } from "react"; import { Button } from "@/components/ui/button"; +import { PILL } from "@/components/ui/pill"; import { devLogin } from "@/lib/apollo/client"; import { describeError } from "@/lib/apollo/errors"; @@ -40,12 +41,12 @@ export default function DevLoginButton({ onSubmit={(event) => void logIn(event)} className="mt-4 max-w-[24rem] space-y-2" > -

+

Test login: anyone can log in as anyone, so don’t enter real data.

@@ -62,16 +63,21 @@ export default function DevLoginButton({ }} aria-invalid={Boolean(error)} aria-describedby={error ? "dev-login-error" : undefined} - className="h-11 w-full rounded-md border border-(--box-stroke) bg-(--background-color-element) px-3 text-(--text-color-dark) focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-(--text-color-dark)" + className="h-11 w-full rounded-md border border-(--field-border) bg-(--field-bg) px-3 text-(--text-color) focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-(--text-color)" /> - {error && ( diff --git a/src/components/gymnasts/GymnastList.test.tsx b/src/components/gymnasts/GymnastList.test.tsx index e3b933b..6d59d1d 100644 --- a/src/components/gymnasts/GymnastList.test.tsx +++ b/src/components/gymnasts/GymnastList.test.tsx @@ -1,17 +1,24 @@ +import { ApolloProvider } from "@apollo/client/react"; import { renderToString } from "react-dom/server"; import { describe, expect, it } from "vitest"; +import { createTestClient } from "@/tests/apollo"; import GymnastList from "./GymnastList"; +// Each row has a delete button, which uses the client for its mutation. +const { client } = createTestClient(() => new Promise(() => {})); +const renderList = (gymnasts: { id: string; name: string }[]) => + renderToString( + + {}} onFailed={() => {}} /> + , + ); + describe("GymnastList", () => { it("renders names with initials", () => { - const markup = renderToString( - , - ); + const markup = renderList([ + { id: "one", name: "Ingrid Haugen" }, + { id: "two", name: "Emma Berg" }, + ]); expect(markup).toContain("Ingrid Haugen"); expect(markup).toContain("IH"); @@ -20,15 +27,26 @@ describe("GymnastList", () => { }); it("has a heading for the list", () => { - const markup = renderToString(); + const markup = renderList([]); expect(markup).toMatch(/]*>Gymnast list<\/h2>/); }); it("announces the empty list state", () => { - const markup = renderToString(); + const markup = renderList([]); expect(markup).toContain('role="status"'); expect(markup).toContain("You haven’t added any gymnasts yet."); }); -}); \ No newline at end of file +}); + +describe("GymnastList: the cards show that they can be pressed", () => { + it("has a tint and a deeper shadow on hover, a pointer, and no movement for a visitor who asks for less", () => { + const markup = renderList([{ id: "one", name: "Ingrid Haugen" }]); + + expect(markup).toContain("cursor-pointer"); + expect(markup).toContain("hover:bg-[color-mix(in_srgb,var(--surface),var(--text-color)_8%)]"); + expect(markup).toContain("hover:shadow-[0_6px_16px_rgba(3,2,2,0.22)]"); + expect(markup).toContain("motion-reduce:transition-none"); + }); +}); diff --git a/src/components/gymnasts/GymnastList.tsx b/src/components/gymnasts/GymnastList.tsx index 39f56d0..20c5f8f 100644 --- a/src/components/gymnasts/GymnastList.tsx +++ b/src/components/gymnasts/GymnastList.tsx @@ -3,8 +3,12 @@ import type { GymnastSummary } from "@/lib/gymnasts"; export default function GymnastList({ gymnasts, + onDeleted, + onFailed, }: { gymnasts: GymnastSummary[]; + onDeleted: (name: string) => void; + onFailed: (message: string) => void; }) { return (
{gymnasts.length === 0 ? ( -

+

You haven’t added any gymnasts yet.

) : (
    {gymnasts.map((gymnast) => ( - + ))}
)} diff --git a/src/components/gymnasts/GymnastRow.tsx b/src/components/gymnasts/GymnastRow.tsx index 2a955f8..8b8b26f 100644 --- a/src/components/gymnasts/GymnastRow.tsx +++ b/src/components/gymnasts/GymnastRow.tsx @@ -1,18 +1,32 @@ -import type { GymnastSummary } from "@/lib/gymnasts"; -import { getGymnastInitials } from "@/lib/gymnasts"; +import DeleteGymnastButton from '@/components/gymnasts/DeleteGymnastButton' +import type { GymnastSummary } from '@/lib/gymnasts' +import { getGymnastInitials } from '@/lib/gymnasts' -export default function GymnastRow({ gymnast }: { gymnast: GymnastSummary }) { +export default function GymnastRow({ + gymnast, + onDeleted, + onFailed, +}: { + gymnast: GymnastSummary + onDeleted: (name: string) => void + onFailed: (message: string) => void +}) { return ( -
  • -
    +
  • +
    -

    {gymnast.name}

    +

    {gymnast.name}

    +
    • - ); -} \ No newline at end of file + ) +} diff --git a/src/components/gymnasts/YourGymnastsPage.test.tsx b/src/components/gymnasts/YourGymnastsPage.test.tsx index 9ba9a24..76f0ef7 100644 --- a/src/components/gymnasts/YourGymnastsPage.test.tsx +++ b/src/components/gymnasts/YourGymnastsPage.test.tsx @@ -3,8 +3,16 @@ // src/tests/apollo.ts). import { ApolloProvider } from '@apollo/client/react' -import { cleanup, fireEvent, render, screen } from '@testing-library/react' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { + cleanup, + fireEvent, + render, + screen, + within, +} from '@testing-library/react' +import { MemoryRouter } from 'react-router-dom' +import { afterEach, describe, expect, it } from 'vitest' +import { AuthProvider } from '@/components/auth/AuthProvider' import { createTestClient, deferred, @@ -29,11 +37,32 @@ const coachWith = (names: string[]) => }, }) -function renderPage(handler: Handler) { - const { client, calls } = createTestClient(handler) +/** + * The page inside the same providers as in the app. `Me` (who is logged in) is + * answered here and the gymnasts by `handler`, as the server does. + */ +function renderPage(handler: Handler, { loggedIn = true } = {}) { + const withMe: Handler = (operation) => + operation.operationName === 'Me' + ? ok({ + me: loggedIn + ? { + __typename: 'Coach', + id: 'coach-1', + name: 'Anna', + email: 'anna@example.com', + } + : null, + }) + : handler(operation) + const { client, calls } = createTestClient(withMe) render( - + + + + + , ) return { @@ -43,7 +72,9 @@ function renderPage(handler: Handler) { } const listedNames = () => - screen.getAllByRole('listitem').map((item) => item.textContent) + screen + .getAllByRole('listitem') + .map((item) => item.querySelector('h2')?.textContent) async function addGymnast(name: string) { fireEvent.click(await screen.findByRole('button', { name: 'Add gymnast' })) @@ -58,7 +89,7 @@ afterEach(cleanup) describe('YourGymnastsPage: loading the gymnasts', () => { it('has a title that says what the page is (WCAG 2.4.2)', async () => { renderPage(() => coachWith(['Emma Berg'])) - await screen.findByText(/Emma Berg/) + await screen.findByText('Emma Berg') expect(document.title).toBe('Your gymnasts – WAG CoP Planner') }) @@ -68,14 +99,14 @@ describe('YourGymnastsPage: loading the gymnasts', () => { expect(screen.getByText('Loading your gymnasts…')).toBeTruthy() answer.resolve(coachWith(['Emma Berg'])) - expect(await screen.findByText(/Emma Berg/)).toBeTruthy() + expect(await screen.findByText('Emma Berg')).toBeTruthy() expect(screen.queryByText('Loading your gymnasts…')).toBeNull() }) it('lists the gymnasts in the order the API gives them', async () => { renderPage(() => coachWith(['Zara Moe', 'Anna Lie', 'Mia Dahl'])) - await screen.findByText(/Zara Moe/) + await screen.findByText('Zara Moe') const items = listedNames() expect(items).toHaveLength(3) expect(items[0]).toContain('Zara Moe') @@ -105,12 +136,12 @@ describe('YourGymnastsPage: loading the gymnasts', () => { expect(screen.queryByRole('button', { name: 'Add gymnast' })).toBeNull() fireEvent.click(screen.getByRole('button', { name: 'Try again' })) - expect(await screen.findByText(/Emma Berg/)).toBeTruthy() + expect(await screen.findByText('Emma Berg')).toBeTruthy() expect(screen.queryByRole('alert')).toBeNull() }) it('does not show an empty list when nobody is logged in', async () => { - renderPage(() => ok({ me: null })) + renderPage(() => ok({ me: null }), { loggedIn: false }) expect( await screen.findByText('Log in to see and add your gymnasts.'), @@ -119,25 +150,12 @@ describe('YourGymnastsPage: loading the gymnasts', () => { expect(screen.queryByRole('button', { name: 'Add gymnast' })).toBeNull() }) - it('offers the test login only in a build made with VITE_DEV_LOGIN=true', async () => { - try { - vi.stubEnv('VITE_DEV_LOGIN', 'true') - renderPage(() => ok({ me: null })) - expect( - await screen.findByRole('button', { name: 'Log in as test coach' }), - ).toBeTruthy() - cleanup() - - for (const value of ['', 'false']) { - vi.stubEnv('VITE_DEV_LOGIN', value) - renderPage(() => ok({ me: null })) - await screen.findByText('Log in to see and add your gymnasts.') - expect(screen.queryByRole('button', { name: /test coach/ })).toBeNull() - cleanup() - } - } finally { - vi.unstubAllEnvs() - } + it('links to the login page when nobody is logged in, and has no login form of its own', async () => { + renderPage(() => ok({ me: null }), { loggedIn: false }) + + const link = await screen.findByRole('link', { name: 'Log in' }) + expect(link.getAttribute('href')).toBe('/login') + expect(screen.queryByRole('form', { name: 'Test login' })).toBeNull() }) }) @@ -163,7 +181,7 @@ describe('YourGymnastsPage: adding a gymnast', () => { it('sends the normalized name, closes the form and shows the gymnast in the API order', async () => { const page = renderPage(serverWith(['Emma Berg', 'Zara Moe'])) - await screen.findByText(/Emma Berg/) + await screen.findByText('Emma Berg') await addGymnast(' Anna Lie ') @@ -172,31 +190,81 @@ describe('YourGymnastsPage: adding a gymnast', () => { expect(page.created()).toHaveLength(1) expect(page.created()[0]?.variables).toEqual({ name: 'Anna Lie' }) const items = listedNames() - expect(items.map((text) => text?.slice(2))).toEqual([ - 'Anna Lie', - 'Emma Berg', - 'Zara Moe', - ]) + expect(items).toEqual(['Anna Lie', 'Emma Berg', 'Zara Moe']) expect(screen.getByText('Anna Lie added to your gymnasts.')).toBeTruthy() expect(screen.queryByLabelText('Gymnast name')).toBeNull() }) - it('checks the name before it sends anything', async () => { + it('says "Name is required" in red when the name is empty, and sends nothing', async () => { const page = renderPage(serverWith([])) await screen.findByText('You haven’t added any gymnasts yet.') await addGymnast(' ') + + const error = await screen.findByRole('alert') + expect(error.textContent).toBe('Name is required') + expect(error.className).toContain('text-(--danger)') + // The field says that it is wrong, and what is wrong. + const field = screen.getByLabelText('Gymnast name') + expect(field.getAttribute('aria-invalid')).toBe('true') + expect(field.getAttribute('aria-describedby')).toBe(error.id) + expect(page.created()).toHaveLength(0) + }) + + it('says it with Enter in the field too, and the error goes away when the visitor types', async () => { + renderPage(serverWith([])) + fireEvent.click(await screen.findByRole('button', { name: 'Add gymnast' })) + + fireEvent.submit(screen.getByRole('form', { name: 'Add gymnast' })) // what Enter does expect((await screen.findByRole('alert')).textContent).toBe( - 'Enter a gymnast name.', + 'Name is required', ) + fireEvent.change(screen.getByLabelText('Gymnast name'), { - target: { value: 'x'.repeat(71) }, + target: { value: 'A' }, }) + expect(screen.queryByRole('alert')).toBeNull() + }) + + it('takes a name of 1 character, like the API', async () => { + const page = renderPage(serverWith([])) + await screen.findByText('You haven’t added any gymnasts yet.') + + await addGymnast('A') + + await screen.findByText('A added to your gymnasts.') + expect(page.created()).toHaveLength(1) + }) + + it('is not possible to type or paste more than 70 characters', async () => { + const page = renderPage(serverWith([])) + await screen.findByText('You haven’t added any gymnasts yet.') + fireEvent.click(await screen.findByRole('button', { name: 'Add gymnast' })) + const field = screen.getByLabelText('Gymnast name') + + fireEvent.change(field, { target: { value: 'x'.repeat(71) } }) + expect(field.value).toBe('x'.repeat(70)) + + // A paste of a long text is cut as well, and the form never has an error for it. + fireEvent.change(field, { target: { value: 'y'.repeat(500) } }) + expect(field.value).toBe('y'.repeat(70)) fireEvent.click(screen.getByRole('button', { name: 'Save gymnast' })) - expect((await screen.findByRole('alert')).textContent).toBe( - 'Names must be 70 characters or fewer.', - ) - expect(page.created()).toHaveLength(0) + await screen.findByText('y'.repeat(70) + ' added to your gymnasts.') + expect(page.created()[0]?.variables).toEqual({ name: 'y'.repeat(70) }) + }) + + it('shows no text and no error before the visitor has tried to save', async () => { + renderPage(serverWith([])) + fireEvent.click(await screen.findByRole('button', { name: 'Add gymnast' })) + + expect(screen.queryByRole('alert')).toBeNull() + expect(screen.queryByText(/characters/)).toBeNull() + expect( + screen.getByLabelText('Gymnast name').getAttribute('aria-describedby'), + ).toBeNull() + expect( + screen.getByLabelText('Gymnast name').getAttribute('aria-invalid'), + ).toBe('false') }) it('accepts 70 emoji, which the server accepts too', async () => { @@ -253,7 +321,7 @@ describe('YourGymnastsPage: adding a gymnast', () => { ? answer.promise : coachWith(['Emma Berg']), ) - await screen.findByText(/Emma Berg/) + await screen.findByText('Emma Berg') await addGymnast('Anna Lie') const saving = await screen.findByRole('button', { @@ -271,3 +339,124 @@ describe('YourGymnastsPage: adding a gymnast', () => { expect(page.created()).toHaveLength(1) }) }) + +describe('YourGymnastsPage: the form to add a gymnast', () => { + async function openForm() { + // A server that accepts the new gymnast. + const page = renderPage(({ operationName, variables }) => + operationName === 'CreateGymnast' + ? ok({ + createGymnast: { + __typename: 'Gymnast', + id: 'gymnast-new', + name: String(variables['name']), + }, + }) + : coachWith([]), + ) + fireEvent.click(await screen.findByRole('button', { name: 'Add gymnast' })) + return page + } + + it('has the field with the save and the close beside it, and no heading and no Cancel button', async () => { + await openForm() + + const form = screen.getByRole('form', { name: 'Add gymnast' }) + expect(within(form).queryByRole('heading')).toBeNull() + expect(within(form).queryByRole('button', { name: 'Cancel' })).toBeNull() + expect(within(form).queryByText('Save gymnast')).toBeNull() // an icon, not text + + // In the order of the page: the field, the save (a tick), the close (a cross). + const field = within(form).getByLabelText('Gymnast name') + const save = within(form).getByRole('button', { name: 'Save gymnast' }) + const close = within(form).getByRole('button', { + name: 'Close add gymnast form', + }) + expect( + field.compareDocumentPosition(save) & Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy() + expect( + save.compareDocumentPosition(close) & Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy() + expect(save.getAttribute('type')).toBe('submit') + expect(save.querySelector('svg')).toBeTruthy() + expect(close.querySelector('svg')).toBeTruthy() + }) + + it('has buttons that are 48 by 48 CSS pixels, so they can be pressed on a phone', async () => { + await openForm() + + for (const name of ['Save gymnast', 'Close add gymnast form']) { + const classes = screen + .getByRole('button', { name }) + .className.split(/\s+/) + expect(classes).toContain('size-12') + } + }) + + it('has the label and the field aligned to the left', async () => { + await openForm() + + const label = screen.getByText('Gymnast name') + expect(label.className).toContain('text-left') + expect(screen.getByLabelText('Gymnast name').className).toContain( + 'text-left', + ) + expect( + screen.getByRole('form', { name: 'Add gymnast' }).className, + ).toContain('text-left') + }) + + it('closes with the cross and with Escape, and saves with Enter', async () => { + const page = await openForm() + fireEvent.click( + screen.getByRole('button', { name: 'Close add gymnast form' }), + ) + expect(screen.queryByLabelText('Gymnast name')).toBeNull() + + fireEvent.click(screen.getByRole('button', { name: 'Add gymnast' })) + fireEvent.keyDown(screen.getByLabelText('Gymnast name'), { key: 'Escape' }) + expect(screen.queryByLabelText('Gymnast name')).toBeNull() + + fireEvent.click(screen.getByRole('button', { name: 'Add gymnast' })) + fireEvent.change(screen.getByLabelText('Gymnast name'), { + target: { value: 'Ana' }, + }) + fireEvent.submit(screen.getByRole('form', { name: 'Add gymnast' })) // what Enter does + await screen.findByText('Ana added to your gymnasts.') + expect(page.created()).toHaveLength(1) + }) + + it('uses theme colors for the form, so it works in dark mode', async () => { + await openForm() + + const form = screen.getByRole('form', { name: 'Add gymnast' }) + expect(form.className).toContain('bg-(--surface)') + expect(screen.getByLabelText('Gymnast name').className).toContain( + 'bg-(--field-bg)', + ) + expect( + screen.getByRole('button', { name: 'Save gymnast' }).className, + ).toContain('bg-(--success-bg)') + }) +}) + +describe('YourGymnastsPage: the save button', () => { + it('is green with a tick that can be seen, not the color of the card', async () => { + renderPage(() => coachWith([])) + fireEvent.click(await screen.findByRole('button', { name: 'Add gymnast' })) + + const classes = screen.getByRole('button', { + name: 'Save gymnast', + }).className + expect(classes).toContain('bg-(--success-bg)') + expect(classes).toContain('text-(--success-contrast)') + // The colors of the card and of the page are not used for the button. + expect(classes).not.toContain('bg-(--surface)') + expect(classes).not.toContain('bg-(--button-bg)') + // The outline variant of the button has a background of its own in dark mode + // (dark:bg-input/30) that wins over the green, so the button must not use it. + expect(classes).not.toContain('dark:bg-input/30') + expect(classes).not.toContain('dark:hover:bg-input/50') + }) +}) diff --git a/src/components/gymnasts/YourGymnastsPage.tsx b/src/components/gymnasts/YourGymnastsPage.tsx index a9d1aff..c84213c 100644 --- a/src/components/gymnasts/YourGymnastsPage.tsx +++ b/src/components/gymnasts/YourGymnastsPage.tsx @@ -1,12 +1,13 @@ import { useEffect, useRef, useState } from "react"; import { useMutation, useQuery } from "@apollo/client/react"; -import { Plus, X } from "lucide-react"; +import { Link } from "react-router-dom"; +import { Check, Plus, X } from "lucide-react"; +import { useAuth } from "@/components/auth/useAuth"; import PageTitle from "@/components/PageTitle"; import { Button } from "@/components/ui/button"; -import DevLoginButton from "@/components/gymnasts/DevLoginButton"; +import { PILL } from "@/components/ui/pill"; import GymnastList from "@/components/gymnasts/GymnastList"; import { describeError } from "@/lib/apollo/errors"; -import { isDevLoginEnabled } from "@/lib/devLogin"; import { CREATE_GYMNAST_MUTATION, GYMNASTS_QUERY, @@ -14,6 +15,7 @@ import { import { GYMNAST_NAME_INPUT_MAX_LENGTH, getGymnastNameError, + limitGymnastNameInput, normalizeGymnastName, } from "@/lib/gymnasts"; @@ -36,9 +38,13 @@ export default function YourGymnastsPage() { // create a second gymnast. The state of the mutation changes one render too late. const isSubmitting = useRef(false); - // `me` is null when nobody is logged in or the session has ended. That is not - // the same as a coach with no gymnasts, so it must not look like an empty list. - const isLoggedOut = data?.me === null; + // The same answer as the menu reads. Nobody logged in, or a session that has + // ended, is not the same as a coach with no gymnasts, so it must not look like + // an empty list. (A visitor who is not logged in does not get here from the + // menu: ProtectedRoute sends them to the login page. This is for a session + // that ends while the page is open.) + const isLoggedOut = useAuth().status === "anonymous"; + const [deleteError, setDeleteError] = useState(""); const loadProblem = loadError ? describeError(loadError) : null; useEffect(() => { @@ -51,6 +57,14 @@ export default function YourGymnastsPage() { } }, [isFormOpen]); + // The button of the deleted gymnast is gone, so the focus goes to the button + // that adds one, and the visitor is told what happened. + function onDeleted(deletedName: string) { + setDeleteError(""); + setAnnouncement(`${deletedName} deleted.`); + addButton.current?.focus(); + } + function closeForm() { setIsFormOpen(false); setName(""); @@ -87,7 +101,7 @@ export default function YourGymnastsPage() { } return ( -
    +
    Your gymnasts

    @@ -99,7 +113,7 @@ export default function YourGymnastsPage() { -

    -
    - - { - setName(event.target.value); - setError(""); - }} - aria-invalid={Boolean(error)} - aria-describedby={error ? "gymnast-name-error" : undefined} - className="h-11 w-full rounded-md border border-(--box-stroke) bg-(--background-color-element) px-3 text-(--text-color-dark) focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-(--text-color-dark)" - /> - {error && ( - - )} -
    -
    - -
    )} {loading && !data && ( -

    +

    Loading your gymnasts…

    )} @@ -184,13 +210,14 @@ export default function YourGymnastsPage() { {loadProblem && (

    {loadProblem.message}

    {loadProblem.retryable && (
    )} - {data?.me && } + {deleteError && ( +

    + {deleteError} +

    + )} + + {data?.me && ( + + )}
    ); diff --git a/src/components/navbar/Navbar.test.tsx b/src/components/navbar/Navbar.test.tsx index 157b893..977c92a 100644 --- a/src/components/navbar/Navbar.test.tsx +++ b/src/components/navbar/Navbar.test.tsx @@ -1,9 +1,31 @@ // @vitest-environment jsdom -import { fireEvent, render, screen } from '@testing-library/react'; -import { BrowserRouter } from 'react-router-dom'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { BrowserRouter, useLocation } from 'react-router-dom'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { AuthContext, type AuthState } from '@/components/auth/useAuth'; +import { logout } from '@/lib/apollo/client'; import { Navbar } from './Navbar'; +// The real functions would call the server. The Navbar only has to call logout. +vi.mock('@/lib/apollo/client', () => ({ + logout: vi.fn(async () => undefined), +})); + +const states: Record = { + loading: { status: 'loading', coach: null }, + anonymous: { status: 'anonymous', coach: null }, + authenticated: { + status: 'authenticated', + coach: { id: 'c1', name: 'Anna', email: 'anna@example.com' }, + }, +}; + +/** Shows the address, so a test can see where the navigation went. */ +function Address() { + const location = useLocation(); + return {location.pathname}; +} + beforeEach(() => { window.history.replaceState({}, '', '/'); }); @@ -13,20 +35,24 @@ afterEach(() => { }); const renderNavbar = ({ - isLoggedIn = false, + auth = 'anonymous', isDarkMode = false, - setIsLoggedIn = vi.fn(), onToggleDarkMode = vi.fn(), +}: { + auth?: keyof typeof states; + isDarkMode?: boolean; + onToggleDarkMode?: () => void; } = {}) => render( - - - , + + + +
    + + , ); describe('Navbar', () => { @@ -51,7 +77,7 @@ describe('Navbar', () => { }); it('shows the Gymnasts link only when logged in', () => { - renderNavbar({ isLoggedIn: true }); + renderNavbar({ auth: 'authenticated' }); expect(screen.getByRole('link', { name: 'Gymnasts' })).toBeTruthy(); }); @@ -153,12 +179,37 @@ describe('Navbar', () => { expect(themeToggle.querySelector('span')?.style.transform).toBe('translateX(0)'); }); - it('requests the opposite login state when the button is clicked', () => { - const setIsLoggedIn = vi.fn(); - renderNavbar({ setIsLoggedIn }); + it('has a Log in link to the login page when nobody is logged in', () => { + renderNavbar({ auth: 'anonymous' }); + + expect(screen.getByRole('link', { name: 'Log in' }).getAttribute('href')).toBe( + '/login', + ); + expect(screen.queryByRole('button', { name: 'Log out' })).toBeNull(); + }); + + it('has a Log out button, and no Log in, when a coach is logged in', () => { + renderNavbar({ auth: 'authenticated' }); + + expect(screen.getByRole('button', { name: 'Log out' })).toBeTruthy(); + expect(screen.queryByRole('link', { name: 'Log in' })).toBeNull(); + }); + + it('says neither Log in nor Log out until the server has answered', () => { + renderNavbar({ auth: 'loading' }); + + expect(screen.queryByRole('link', { name: 'Log in' })).toBeNull(); + expect(screen.queryByRole('button', { name: 'Log out' })).toBeNull(); + expect(screen.queryByRole('link', { name: 'Gymnasts' })).toBeNull(); + }); + + it('logs out and leaves the page that is for coaches', async () => { + window.history.replaceState({}, '', '/gymnasts'); + renderNavbar({ auth: 'authenticated' }); - fireEvent.click(screen.getByRole('button', { name: 'Log in' })); + fireEvent.click(screen.getByRole('button', { name: 'Log out' })); - expect(setIsLoggedIn).toHaveBeenCalledWith(true); + await waitFor(() => expect(logout).toHaveBeenCalledOnce()); + expect(screen.getByTestId('address').textContent).toBe('/elements'); }); }); diff --git a/src/components/navbar/Navbar.tsx b/src/components/navbar/Navbar.tsx index a413049..ffe806e 100644 --- a/src/components/navbar/Navbar.tsx +++ b/src/components/navbar/Navbar.tsx @@ -1,23 +1,35 @@ import React, { useState } from 'react'; import { Menu, X } from 'lucide-react'; -import { NavLink } from 'react-router-dom'; +import { Link, NavLink, useNavigate } from 'react-router-dom'; +import { useAuth } from '@/components/auth/useAuth'; +import { logout } from '@/lib/apollo/client'; interface NavbarProps { - isLoggedIn: boolean; - setIsLoggedIn: (value: boolean) => void; isDarkMode: boolean; onToggleDarkMode: () => void; } export const Navbar: React.FC = ({ - isLoggedIn, - setIsLoggedIn, isDarkMode, onToggleDarkMode, }) => { const [menuOpen, setMenuOpen] = useState(false); + // The same answer as the pages read: who is logged in, as the server says it. + const { status } = useAuth(); + const isLoggedIn = status === 'authenticated'; + const navigate = useNavigate(); + + async function logOut() { + setMenuOpen(false); + // Leave a page that is for coaches first, so the visitor is not sent to the + // login page by the page they are leaving. + void navigate('/elements'); + await logout(); + } const focusRing = 'focus-visible:outline-2 focus-visible:outline-(--text-color) focus-visible:outline-offset-4'; + const authButton = + 'inline-flex min-h-6 min-w-6 shrink items-center whitespace-nowrap rounded-corner border border-border-color bg-transparent px-[clamp(11px,12px,13px)] py-xs font-text text-[clamp(var(--font-size-row-title),1.2vw,1rem)] text-text-color'; const navLinks = [ { path: '/elements', label: 'Search' }, { path: '/calculator', label: 'Calculator' }, @@ -81,13 +93,21 @@ export const Navbar: React.FC = ({ - + {/* While the server has not answered, neither "Log in" nor "Log out" is true. */} + {status !== 'loading' && + (isLoggedIn ? ( + + ) : ( + + Log in + + ))}