feat(fx-api): api contract for firefox clients

[kschelonka]

References:

Jira: MNTOR-5192
Figma: Privacy Panel

Description

API for firefox clients to provide breach alerts in the privacy panel.

Usage pattern:

• periodic polling to update unresolved breaches in memory
• all data access is user-scoped, using FxA as auth server
• typical resolution pattern is per-breach, when a user clicks "mark as done" on the privacy panel

How to review

Since we currently don't have docs set up, it might be easiest to pull this branch and use an openapi previewer extension in your IDE

[kschelonka]

@Vinnl do you think it's ergonomic to keep the non-bulk endpoint as an option vs. always having to build an array? Or just ditch it? Most of the time I imagine the breach resolution will be a single breach (since the privacy panel is per-domain).

[Vinnl]

Yeah it might be premature optimisation, since for now it looks like it'll always be a single breach, but at the same time, as a client I don't really mind wrapping it in an array I don't think. And since adding endpoints is easier than removing them, I'd just ditch it.

(Come to think of it, I just realised that sometimes we have multiple breaches for the same domain. That's probably fine for the UI, since there's still just a single action for the user: change their password. But that would imply multiple breach resolutions. So yeah, let's ditch the single-breach resolution endpoint.)

[Vinnl]

Thanks! Looking good, main two comments are about the bulk resolution endpoint still listing an unused path parameter and (I think) the wrong response type. The rest are mere suggestions or thoughts, feel free to take 'm or leave 'm.

(I didn't really look at the individual breach resolution, because as mentioned there, I think we wouldn't use it.)

[Vinnl]

I'm not familiar with this field, but any reason not to include stage as well?

[kschelonka]

Just to avoid publishing it publicly. it's not really security by obscurity since we're not relying on obscurity to avoid access, but having it publicly listed probably will increase undesirable requests made to it

[Vinnl]

Suggestion: might be nice for clients to be able to make assumptions here. (Though on the other hand, since we can't encode it in the OpenAPI type, it might be best for callers to check.)

Suggested change

	description: The domain of the primary website the breach occurred on. This may be used for identifying other assets external systems may have for the site.
	description: The domain of the primary website the breach occurred on. This may be used for identifying other assets external systems may have for the site. Does not include protocol and trailing slash, e.g. `example.com`.

[kschelonka]

I'm not sure if that's guaranteed, since this value is coming from HIBP. How about I make a note about HIBP types instead (this is copied directly from their docs)

[Vinnl]

I think it's a good idea to keep track of the source at least, ideally from the start. Do we still store resolutions in a big JSON blob? If so, then that might be a bit too much to ask, but otherwise it'd be nice.

I don't think I follow why we need the object though - can't we just add new properties to UserBreachState later? I don't think I'd implement the client calls in such a way that it would break if additional fields show up later.

[kschelonka]

Yes we still store resolutions in the big JSON blob. UserBreachState is the parent object so it wouldn't work to just add fields there, but there is an alternative to use an array of resolutions instead of a key value object. Instead of something you'd express in typescript types like:

type ExposureResolutionMeta = {
  // source: string,       // eventually you might have fields like this
  // timestamp: date   // but the data model doesn't support it right now
}

type UserBreachState = {
email: string,
resolvedDataClasses: {
[dataClass: string]: ExposureResolutionMeta
}

Example:

{
"email": "me@example.com"
"resolvedDataClasses": {
    "passwords": { } // {"timestamp": 12323232, "source": "firefox"},
    "credit-cards": { } // {"timestamp": 123123232, "source": "monitor"}
}

And then if the client wants to check if a particular resolution exists, they just have to do resolvedDataClasses.passwords

One alternative is:

type ExposureResolution = {
   dataClass: string,
   // source: string,
   // timestamp: date
}
type UserBreachState = {
email: string,
resolvedDataClasses: Array<ExposureResolution>
}

then if the client wants to check they have to do an array filter/search: resolvedDataClasses.find((_) => _.dataClass === "password")

I went the object because I thought it was more ergonomic, and makes clear that dataClass is unique in the breach resolutions context

[Vinnl]

As suggested above:

Suggested change

	"404":
	description: Breach or email not found (or not visible to this user)

[groovecoder]

@kschelonka : did you use a tool to create this file? Is this going to be the source of truth for clients - i.e., is it going to be served at monitor.mozilla.org? Or is this just meant for documentation?

[kschelonka]

@kschelonka : did you use a tool to create this file? Is this going to be the source of truth for clients - i.e., is it going to be served at monitor.mozilla.org? Or is this just meant for documentation?

No I wrote it manually following the openapi specification, and I ran it through a swagger previewer to ensure it was correctly formatted. We could host it directly but it would probably make more sense to use readthedocs.io like remote settings or even host a github pages from this repo?