Repository navigation
feat(fx-api): add POST /user/breaches/resolutions for the Firefox API (MNTOR-5345) #6874
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 8 commits
fca39c1
a3fdfac
c39a34b
1b65cb3
b8c4df9
5494aab
e3ccc85
9b23cbe
9a3e90e
0f8085f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,33 +39,19 @@ components: | |
| - message | ||
| properties: | ||
| code: | ||
| description: Stable, machine-readable cause, kebab-case. Today `token-invalid`, `missing-scope`, `no-monitor-account`, `invalid-request`, `not-found`, `rate-limited`, `internal-error`, `unavailable`. We may add values in the future, so fall back to the status code for one you do not recognise. | ||
| description: Stable, machine-readable cause, kebab-case. Today `token-invalid`, `missing-scope`, `no-monitor-account`, `invalid-request`, `rate-limited`, `internal-error`, `unavailable`. We may add values in the future, so fall back to the status code for one you do not recognise. | ||
| type: string | ||
| message: | ||
| type: string | ||
| DataClass: | ||
| description: A data class you can mark resolved. Closed set, we reject anything else with a `400`. Remote Settings serves the same values in HIBP's Title Case, `Passwords` and `Email addresses`. | ||
| type: string | ||
| enum: | ||
| - passwords | ||
| - email-addresses | ||
| ResolvedDataClass: | ||
| description: A data class we report as resolved. `passwords` and `email-addresses` today, but this is an open set, so accept a value you do not recognise instead of rejecting the response. | ||
| type: string | ||
| BreachResolution: | ||
| description: One entry of a `POST /user/breaches/resolutions` body. Names a breach and the data classes to mark resolved. | ||
| description: One entry of a `POST /user/breaches/resolutions` body. Names a breach to mark resolved. We ignore unknown properties, so older servers accept newer clients. | ||
| type: object | ||
| additionalProperties: false | ||
| required: | ||
| - dataClasses | ||
| - breachId | ||
| properties: | ||
| dataClasses: | ||
| type: array | ||
| minItems: 1 | ||
| description: Data classes to mark resolved for every monitored address the breach affected. Each must be one the referenced breach actually leaked, so a breach that leaked only email addresses takes `email-addresses`, not `passwords`. Sending one it did not leak is a `400`, so refresh your Remote Settings copy before retrying. | ||
| items: | ||
| $ref: "#/components/schemas/DataClass" | ||
| breachId: | ||
| description: The HIBP breach name, as returned by `GET /user/breaches` in `id`. | ||
| type: string | ||
|
|
@@ -278,15 +264,17 @@ paths: | |
| /user/breaches/resolutions: | ||
| post: | ||
| operationId: createBreachResolutions | ||
| summary: Mark breach data classes resolved | ||
| summary: Mark breaches resolved | ||
| description: | | ||
| Mark data classes resolved for one or more breaches. Resolutions only move one way, this API cannot unresolve a data class. | ||
| Mark one or more breaches resolved, like "Mark as done" in the privacy panel. Resolutions only move one way, this API cannot unresolve a breach. | ||
|
|
||
| Each entry resolves every data class this API serves that the breach leaked, `passwords` and `email-addresses`, for every monitored address the breach affected. There is no way to target one address or data class. | ||
|
|
||
| Each entry applies to every monitored address the breach affected. There is no way to target one address, as the privacy panel resolves a breach for the whole account. | ||
| We skip a `breachId` that matches no breach affecting this user, for example one HIBP renamed or retired, and still apply the rest. There is nothing to resolve for it, so there is nothing to retry. | ||
|
|
||
| Idempotent per `(breachId, dataClass)`. Repeated `breachId` entries merge, and order does not matter. | ||
| Idempotent per `breachId`. Repeated entries merge, and order does not matter. | ||
|
|
||
| Atomic: every affected address and data class updates, or none does. | ||
| Atomic: one write covers every matched breach, so a request never applies halfway. | ||
|
|
||
| requestBody: | ||
| required: true | ||
|
|
@@ -298,62 +286,36 @@ paths: | |
| items: | ||
| $ref: "#/components/schemas/BreachResolution" | ||
| examples: | ||
| resolveOneBreach: | ||
| summary: Resolve one breach | ||
| value: | ||
| - breachId: "Adobe" | ||
| resolveMultipleBreaches: | ||
| summary: Resolve multiple breaches | ||
| value: | ||
| - breachId: "Adobe" | ||
| dataClasses: ["passwords"] | ||
| - breachId: "LinkedIn" | ||
| dataClasses: ["passwords"] | ||
| resolveEmailOnlyBreach: | ||
| summary: A breach that leaked no passwords takes email-addresses | ||
| value: | ||
| - breachId: "Facebook" | ||
| dataClasses: ["email-addresses"] | ||
| responses: | ||
| "204": | ||
| description: Resolution state updated successfully. | ||
| description: Every matched breach is resolved. Unmatched ones were skipped. | ||
| "400": | ||
| description: Invalid request, for example an empty batch, an empty `dataClasses` array, an unknown property, or a data class the referenced breach never leaked. | ||
| description: The body is not a non-empty JSON array of objects, each with a non-empty string `breachId`. Unknown properties and query parameters are ignored, not rejected. | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "#/components/schemas/Error" | ||
| examples: | ||
| unknownDataClass: | ||
| summary: Data class outside the accepted set | ||
| value: | ||
| code: invalid-request | ||
| message: "dataClasses[0] must be one of: passwords, email-addresses." | ||
| dataClassNotInBreach: | ||
| summary: Breach never leaked the requested data class | ||
| value: | ||
| code: invalid-request | ||
| message: The Facebook breach did not leak passwords. | ||
| example: | ||
| code: invalid-request | ||
| message: "[0].breachId must be a non-empty string." | ||
| "401": | ||
| $ref: "#/components/responses/Unauthorized" | ||
| "403": | ||
| $ref: "#/components/responses/Forbidden" | ||
| "404": | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question (non-blocking): are these cases being moved into a different response status code?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, they're gone rather than moved. We dropped dataClasses from the request, and I chatted with the frontend team about this change. As on their side of the house, they can only click "approve all" and don't have to provide us any data class value |
||
| description: | | ||
| Nothing here to resolve, for one of: | ||
|
|
||
| - an unknown `breachId`, or a breach this endpoint does not serve | ||
| - a breach that affected no address this user monitors | ||
|
|
||
| Both give the same code and message, so the response does not say which of them applied. | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "#/components/schemas/Error" | ||
| example: | ||
| code: not-found | ||
| message: Nothing matched for one or more entries. | ||
| "429": | ||
| $ref: "#/components/responses/TooManyRequests" | ||
| "500": | ||
| $ref: "#/components/responses/InternalError" | ||
| description: Something failed on our side. Treat the batch as not applied and retry it, repeats are safe. | ||
| description: Something failed on our side. The batch may or may not be applied, so retry it, repeats are safe. | ||
| "503": | ||
| $ref: "#/components/responses/ServiceUnavailable" | ||
| description: Monitor is unavailable, or FxA could not verify the token. Treat the batch as not applied and retry it, after the interval in `Retry-After` when it is present. Repeats are safe. | ||
| description: FxA could not verify the token, we could not check this user against HIBP, or Monitor is unavailable. Nothing was written. Retry after the interval in `Retry-After` when it is present, otherwise later. Repeats are safe. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,223 @@ | ||
| /* This Source Code Form is subject to the terms of the Mozilla Public | ||
| * License, v. 2.0. If a copy of the MPL was not distributed with this | ||
| * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ | ||
|
|
||
| import { | ||
| describe, | ||
| it, | ||
| expect, | ||
| vi, | ||
| beforeEach, | ||
| afterEach, | ||
| afterAll, | ||
| } from "vitest"; | ||
| import { NextRequest } from "next/server"; | ||
| import MockRedis from "ioredis-mock"; | ||
| import createDbConnection from "../../../../../../../db/connect"; | ||
| import { seeds } from "../../../../../../../test/db"; | ||
| import { FXA_MONITOR_SCOPE } from "../../../../../../../constants"; | ||
|
|
||
| // Only FxA, HIBP and Redis are fake; Postgres is real | ||
| vi.mock("../../../../../../../utils/fxa", async (importOriginal) => ({ | ||
| ...(await importOriginal<typeof import("../../../../../../../utils/fxa")>()), | ||
| introspectToken: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock("../../../../../../../utils/hibp", async (importOriginal) => ({ | ||
| ...(await importOriginal<typeof import("../../../../../../../utils/hibp")>()), | ||
| searchHibpRange: vi.fn(), | ||
| })); | ||
|
|
||
| const redis = new MockRedis(); | ||
| vi.mock("../../../../../../../db/redis/client", () => ({ | ||
| redisClient: () => redis, | ||
| REDIS_ALL_BREACHES_KEY: "breaches", | ||
| BREACHES_EXPIRY_SECONDS: 43200, | ||
| })); | ||
|
|
||
| vi.mock("../../../../../../functions/server/logging", () => ({ | ||
| logger: { error: vi.fn(), info: vi.fn(), debug: vi.fn(), warn: vi.fn() }, | ||
| })); | ||
|
|
||
| import { introspectToken, getSha1 } from "../../../../../../../utils/fxa"; | ||
| import { searchHibpRange } from "../../../../../../../utils/hibp"; | ||
| import { POST } from "./route"; | ||
|
|
||
| const conn = createDbConnection(); | ||
|
|
||
| const FXA_UID = "integration-test-fxa-uid"; | ||
| const PRIMARY = "Primary@Example.com"; | ||
| const SECONDARY = "secondary@example.com"; | ||
| const UNVERIFIED = "unverified@example.com"; | ||
|
|
||
| /** A breach every filter lets through */ | ||
| function breachSeed(name: string, addedDate: string) { | ||
| return seeds.breaches({ | ||
| name, | ||
| domain: `${name.toLowerCase()}.com`, | ||
| added_date: new Date(addedDate), | ||
| data_classes: ["passwords", "email-addresses"], | ||
| is_verified: true, | ||
| is_fabricated: false, | ||
| is_sensitive: false, | ||
| is_retired: false, | ||
| is_spam_list: false, | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Seeds a subscriber, their emails, breaches and the | ||
| * FirefoxApi flag. Returns the breach ids by name. | ||
| */ | ||
| async function seedUser(flagAllowList: string[]) { | ||
| const breaches = await conn("breaches") | ||
| .insert([ | ||
| breachSeed("Adobe", "2020-01-01"), | ||
| breachSeed("LinkedIn", "2026-01-01"), | ||
| ]) | ||
| .returning(["id", "name"]); | ||
| const ids = Object.fromEntries(breaches.map((b) => [b.name, b.id])); | ||
|
|
||
| const [subscriber] = await conn("subscribers") | ||
| .insert( | ||
| seeds.subscribers({ | ||
| fxa_uid: FXA_UID, | ||
| primary_email: PRIMARY, | ||
| primary_verified: true, | ||
| // Stored as the website's resolve route writes it | ||
| breach_resolution: { | ||
| useBreachId: true, | ||
| [PRIMARY]: { [ids.Adobe]: { resolutionsChecked: ["passwords"] } }, | ||
| }, | ||
| }), | ||
| ) | ||
| .returning("*"); | ||
|
|
||
| await conn("email_addresses").insert([ | ||
| seeds.emails(subscriber.id, { email: SECONDARY, verified: true }), | ||
| seeds.emails(subscriber.id, { email: UNVERIFIED, verified: false }), | ||
| ]); | ||
|
|
||
| await conn("feature_flag_events").insert({ | ||
| name: "FirefoxApi", | ||
| is_enabled: false, | ||
| allow_list: flagAllowList, | ||
| created_by_subscriber_id: subscriber.id, | ||
| }); | ||
|
|
||
| return ids; | ||
| } | ||
|
|
||
| /** HIBP lists these breach names, per email */ | ||
| function mockHibp(namesByEmail: Record<string, string[]>) { | ||
| vi.mocked(searchHibpRange).mockImplementation(async (sha1) => { | ||
| for (const [email, names] of Object.entries(namesByEmail)) { | ||
| if (getSha1(email.toLowerCase()) === sha1) return names; | ||
| } | ||
| return []; | ||
| }); | ||
| } | ||
|
|
||
| function authedRequest(body: unknown) { | ||
| return new NextRequest( | ||
| "http://localhost/api/firefox/v1/user/breaches/resolutions", | ||
| { | ||
| method: "POST", | ||
| headers: { authorization: "Bearer fxa-access-token" }, | ||
| body: JSON.stringify(body), | ||
| }, | ||
| ); | ||
| } | ||
|
|
||
| /** The subscriber row's stored resolution state */ | ||
| async function storedRow() { | ||
| return conn("subscribers") | ||
| .where("fxa_uid", FXA_UID) | ||
| .first("breach_resolution", "updated_at"); | ||
| } | ||
|
|
||
| const RESOLVE_LINKEDIN = [{ breachId: "LinkedIn" }]; | ||
|
|
||
| beforeEach(async () => { | ||
| // E2E "local" reads headers(), needing a request | ||
| vi.stubEnv("E2E_TEST_ENV", "ci"); | ||
| await redis.flushall(); | ||
| vi.mocked(introspectToken).mockResolvedValue({ | ||
| active: true, | ||
| sub: FXA_UID, | ||
| scope: FXA_MONITOR_SCOPE, | ||
| client_id: "integration-client", | ||
| token_type: "access_token", | ||
| }); | ||
| }); | ||
|
|
||
| afterEach(async () => { | ||
| vi.clearAllMocks(); | ||
| vi.unstubAllEnvs(); | ||
| await conn.raw("TRUNCATE TABLE subscribers, breaches CASCADE"); | ||
| }); | ||
|
|
||
| afterAll(async () => { | ||
| await conn.destroy(); | ||
| }); | ||
|
|
||
| describe("POST /api/firefox/v1/user/breaches/resolutions (integration)", () => { | ||
|
GregHilstonMozilla marked this conversation as resolved.
|
||
| it("resolves the breach for every affected email in Postgres", async () => { | ||
| const ids = await seedUser([PRIMARY]); | ||
| mockHibp({ | ||
| [PRIMARY]: ["Adobe", "LinkedIn"], | ||
| [SECONDARY]: ["LinkedIn"], | ||
| }); | ||
|
|
||
| const res = await POST(authedRequest(RESOLVE_LINKEDIN)); | ||
|
|
||
| expect(res.status).toBe(204); | ||
| expect(await res.text()).toBe(""); | ||
| expect((await storedRow())?.breach_resolution).toEqual({ | ||
| useBreachId: true, | ||
| [PRIMARY]: { | ||
| [ids.Adobe]: { resolutionsChecked: ["passwords"] }, | ||
| [ids.LinkedIn]: { | ||
| resolutionsChecked: ["passwords", "email-addresses"], | ||
| }, | ||
| }, | ||
| [SECONDARY]: { | ||
| [ids.LinkedIn]: { | ||
| resolutionsChecked: ["passwords", "email-addresses"], | ||
| }, | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| it("leaves the row unchanged on a retry", async () => { | ||
| await seedUser([PRIMARY]); | ||
| mockHibp({ [PRIMARY]: ["LinkedIn"] }); | ||
| await POST(authedRequest(RESOLVE_LINKEDIN)); | ||
| const before = await storedRow(); | ||
|
|
||
| const res = await POST(authedRequest(RESOLVE_LINKEDIN)); | ||
|
|
||
| expect(res.status).toBe(204); | ||
| expect(await storedRow()).toEqual(before); | ||
| }); | ||
|
|
||
| it("resolves the matched breaches in a mixed batch", async () => { | ||
| const ids = await seedUser([PRIMARY]); | ||
| mockHibp({ [PRIMARY]: ["LinkedIn"] }); | ||
|
|
||
| const res = await POST( | ||
| authedRequest([{ breachId: "Renamed" }, { breachId: "LinkedIn" }]), | ||
| ); | ||
|
|
||
| expect(res.status).toBe(204); | ||
| expect((await storedRow())?.breach_resolution).toEqual({ | ||
| useBreachId: true, | ||
| [PRIMARY]: { | ||
| [ids.Adobe]: { resolutionsChecked: ["passwords"] }, | ||
| [ids.LinkedIn]: { | ||
| resolutionsChecked: ["passwords", "email-addresses"], | ||
| }, | ||
| }, | ||
| }); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've chatted with Kaitlyn and Jonathan on the frontend team about these changes. It should honestly make their efforts even easier...