feat(auth-server): add accountActivity table for per-client liveness by nshirley · Pull Request #20620 · mozilla/fxa

nshirley · 2026-05-18T19:46:55Z

Because:

FxA lacks a durable per-(userId, clientId, service) activity signal needed for inactive-account deletion, per-RP liveness, and support tooling.
Existing token tables either disappear on revoke, miss Firefox Desktop's online fxa-credentials service-token mints, or are sampled and Redis-biased and therefore under-detect active users.

This commit:

Adds a new accountActivity table to the fxa_oauth schema, keyed by (userId, clientId, service) with secondary indexes for time-window and per-service cohort queries.
Adds a fire-and-forget recordActivity helper that resolves the service tag from the requested scopes (scopeToServiceName) with a clientId fallback (oauthServer.clientIdToServiceNames), then writes via a SQL-throttled upsert. The throttle predicate uses VALUES(x) > x + ? to avoid BIGINT UNSIGNED underflow on out-of-order timestamps.
Wires a single write site at the OAuth grant convergence point in tokenHandler so all four grant types (authorization_code, fxa-credentials, refresh_token, token-exchange) are recorded uniformly.
Extends _removeTokensAndCodes so accountActivity rows are cleaned up alongside tokens on account deletion.
Stores scopeToServiceName as a JSON string
Emits accountActivity.recorded / accountActivity.write_failed StatsD metrics with service + grantType tags. Unclassified ('') rows trigger a deduped log.warn once per process per clientId.

Closes FXA-13662

Put an x in the boxes that apply

My commit is GPG signed.
If applicable, I have modified or added tests which pass locally.
I have added necessary documentation (if appropriate).
I have verified that my changes render correctly in RTL (if appropriate).
I have manually reviewed all AI generated code.

How to review (Optional)

Key files/areas to focus on:
Suggested review order:
Risky or complex parts:

Screenshots (Optional)

Here's a screenshot of the table after I signed in/up through 123Done, then signed in with scopeKeys

Other information (Optional)

Any other information that is important to this pull request.

Copilot

Pull request overview

Adds a durable per-(userId, clientId, service) “liveness” signal to the auth-server OAuth flow by introducing a new accountActivity table and recording activity on token issuance, with cleanup on account deletion and basic metrics/logging.

Changes:

Add accountActivity MySQL table + schema migration (patch level bump) and MySQL upsert/delete queries.
Add lib/oauth/account-activity.ts helper to resolve service tags from scopes/clientId and fire-and-forget writes with StatsD + deduped warnings.
Wire the activity recording into the /oauth/token grant convergence point and update tests/config accordingly.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
packages/fxa-auth-server/lib/routes/oauth/token.spec.ts	Adjust mocks and assertions to accommodate new StatsD emissions.
packages/fxa-auth-server/lib/routes/oauth/token.js	Call `recordActivity` after token issuance (all grant types converge here).
packages/fxa-auth-server/lib/oauth/db/mysql/index.js	Add SQL upsert/delete queries and `_recordAccountActivity` DB method; delete activity rows on account deletion.
packages/fxa-auth-server/lib/oauth/db/index.js	Expose `recordAccountActivity` on the OAuth DB wrapper.
packages/fxa-auth-server/lib/oauth/account-activity.ts	Implement service resolution, fire-and-forget write wrapper, metrics, and deduped warning logic.
packages/fxa-auth-server/lib/oauth/account-activity.spec.ts	Unit tests for parsing, service resolution, metrics/logging, and error handling.
packages/fxa-auth-server/config/index.ts	Add `oauthServer.accountActivity` config (throttle + scope→service JSON string).
packages/db-migrations/databases/fxa_oauth/target-patch.json	Bump patch level to 34.
packages/db-migrations/databases/fxa_oauth/patches/patch-034-033.sql	Down migration stub for patch 34.
packages/db-migrations/databases/fxa_oauth/patches/patch-033-034.sql	Create `accountActivity` table + indexes and bump schema-patch-level.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

nshirley · 2026-05-19T23:32:05Z

+        default: '24 hours',
+        env: 'FXA_ACCOUNT_ACTIVITY_UPDATE_AFTER',
+      },
+      scopeToServiceName: {


I'll be honest, I don't love this since it means we need to keep this in sync with RPs as we add/modify them in admin panel. It doesn't happen often, but we can quickly lose sight of this

One solution too would be to parse out the service from the scope url... it's not great either, but might be better so we don't have to remember to update configs

// account-activity.ts // First path segment after /apps/ in a canonical FXA OAuth scope URL. // e.g. https://identity.mozilla.com/apps/oldsync → 'oldsync' // https://identity.mozilla.com/apps/oldsync/keys → 'oldsync' // https://identity.mozilla.com/apps/relay#metadata → 'relay' const SCOPE_URL_SERVICE_PATTERN = /^https?:\/\/[^/]+\/apps\/([a-z0-9_-]+)/i; /** * Derive a service tag from a canonical OAuth scope URL. * * Short-name scopes like `profile` or `openid` return '' — they're capabilities, * not services. URL-format scopes return the first path segment after `/apps/`. * The `oldsync` → `sync` rename matches the existing tag convention applied in * token.js:918-922 for the `account.signed` metric event. */ export function deriveServiceFromScope(scopeUrl: string): string { const match = scopeUrl.match(SCOPE_URL_SERVICE_PATTERN); if (!match) return ''; const segment = match[1].toLowerCase(); return segment === 'oldsync' ? 'sync' : segment; } // then the deriveServiceFromScope gets used in the resolveService

One solution too would be to parse out the service from the scope url... it's not great either, but might be better so we don't have to remember to update configs

I don't think we should record 'service' unless it's given to us in the URL. Otherwise I feel like that gets really confusing - plus, if some other RP requests one of these scopes, it doesn't mean we consider that the same "project" or "service".

I personally think this is fine to keep as is at least for now. The correct solution is to have the service name be on the scope column in the database. I lean towards keeping this simple for the time being.

You can populate this with defaults

{ "https://identity.mozilla.com/apps/oldsync": "sync", "https://identity.mozilla.com/apps/relay": "relay", "https://identity.mozilla.com/apps/vpn": "vpn", "https://identity.mozilla.com/apps/smartwindow": "smartwindow" }

Per Slack convo, we are going to gate this on fxa-credentials grant-type + client_id + above scopes.

We already have exchange.serviceScopes config with exactly the same thing - maybe we can repurpose it.

LZoog · 2026-05-20T15:22:12Z


+// Resolve once at module load — the /token endpoint is hot and convict's
+// per-request lookup + JSON.parse of scopeToServiceName would otherwise run
+// on every grant. See AccountActivityConfig in lib/oauth/account-activity.ts.


I don't think we are throttling the authorization table authorizedAt writes - after looking at this, do you think we should be?

This wasn't so much the db writes that were the concern, rather the fact that the code initially was loading the config on every request, and running json parse as a result it could create unnecessary overhead - came from a comment from Copilot:

tokenHandler calls config.get('oauthServer') on every /token request. Since this endpoint is hot, consider hoisting the oauthServer config (or just the accountActivity subset) to module scope and passing it in, to avoid per-request config lookups and repeated JSON parsing done in recordActivity.

LZoog · 2026-05-20T15:24:15Z

+        default: '24 hours',
+        env: 'FXA_ACCOUNT_ACTIVITY_UPDATE_AFTER',
+      },
+      scopeToServiceName: {


One solution too would be to parse out the service from the scope url... it's not great either, but might be better so we don't have to remember to update configs

I don't think we should record 'service' unless it's given to us in the URL. Otherwise I feel like that gets really confusing - plus, if some other RP requests one of these scopes, it doesn't mean we consider that the same "project" or "service".

dschom · 2026-05-20T16:35:19Z

+-- Account activity / liveness signal per (userId, clientId, service).
+-- See FXA-13662. Written from the OAuth grant path; consumers include
+-- inactive-account deletion, support tooling, and per-service MAU.
+CREATE TABLE `accountActivity` (


Shouldn't there be an assertion that the current patch level is 36?

I thought there was, I had a bunch of conflicts so it probably got dropped - I'll add it back!

dschom · 2026-05-20T16:36:46Z

+CREATE TABLE `accountActivity` (
+  `userId` BINARY(16) NOT NULL,
+  `clientId` BINARY(8) NOT NULL,
+  `service` VARCHAR(32) NOT NULL DEFAULT '',


I don't love having an arbitrary string value here...

dschom · 2026-05-20T16:37:33Z

+    return services;
+  }
+
+  services.add('');


If there's no explicit mapping, and no fallback we can discern then we still want a value in the set so it gets recorded to the db. There's activity from the user of some kind and we should still record it... that said, maybe there's a better way to do that

dschom · 2026-05-20T16:51:51Z

+    // the grant response. All four grant types (authorization_code,
+    // fxa-credentials, refresh_token, token-exchange) converge here with a
+    // populated grant.scope, so one call site covers everything.
+    accountActivity.recordActivity(


We need a kill switch here in case the DB call ends up being more load than anticipated. The write throttle is effective on a populated table, but initially the table will be unpopulated, which will incur a ton of writes out the gate. Perhaps even better than a kill switch is a secondary throttle / sampler of some sort.

Until this is addressed, I can't r+.

@nshirley Lets add the feature flag just in case.

Yep, agreed, adding it meow

dschom · 2026-05-20T16:58:31Z

+// the unclassified ('') bucket. The metric tag is the durable signal.
+const warnedUnclassifiedClientIds = new Set<string>();
+
+/**


We should really hold this map in the DB. There's a several reasons:

It stops from having to use a string as part of key material, cause you can foreign key the mapping table.

It allows us to make changes without having sync configs or cooridante rollouts.

We don't have to parse JSON config on a hot call.

It eventually allows us a path to remove all these hardcode mappings which are hard to maintain and create multiple sources of truth.

dschom · 2026-05-20T17:02:44Z

+}
+
+// Per-process dedupe so we only warn once per clientId when activity lands in
+// the unclassified ('') bucket. The metric tag is the durable signal.


I feel like we should be passing in these config values. They really only need to be looked up / parsed once, and this could be done at the route handler layer.

dschom · 2026-05-20T17:05:06Z

+  warn(event: string, data?: Record<string, unknown>): void;
+}
+
+/**


This isn't really typesafe. Make the config take a concrete type, or just drop it and pass values to the function.

dschom

service

vbudhram

@nshirley Thanks, please add the feature flag and few comments to consider.

vbudhram · 2026-05-20T15:33:33Z

 const QUERY_REFRESH_TOKEN_DELETE_USER =
  'DELETE FROM refreshTokens WHERE userId=?';
 const QUERY_CODE_DELETE_USER = 'DELETE FROM codes WHERE userId=?';
+// Throttle is applied SQL-side: if the existing row was touched within the


Lets make this comment more concise, try /fxa-simplify

vbudhram · 2026-05-20T16:58:46Z

+    // the grant response. All four grant types (authorization_code,
+    // fxa-credentials, refresh_token, token-exchange) converge here with a
+    // populated grant.scope, so one call site covers everything.
+    accountActivity.recordActivity(


@nshirley Lets add the feature flag just in case.

vbudhram · 2026-05-20T17:02:37Z

+        default: '24 hours',
+        env: 'FXA_ACCOUNT_ACTIVITY_UPDATE_AFTER',
+      },
+      scopeToServiceName: {


I personally think this is fine to keep as is at least for now. The correct solution is to have the service name be on the scope column in the database. I lean towards keeping this simple for the time being.

You can populate this with defaults

{ "https://identity.mozilla.com/apps/oldsync": "sync", "https://identity.mozilla.com/apps/relay": "relay", "https://identity.mozilla.com/apps/vpn": "vpn", "https://identity.mozilla.com/apps/smartwindow": "smartwindow" }

vbudhram · 2026-05-20T17:04:46Z

+    scopeToServiceName: Record<string, unknown>;
+  };
+  /** clientId (hex) → service tag fallback for clients without a scope mapping. */
+  clientIdToServiceNames: Record<string, unknown>;


I would probably remove this. The service is the browser service, not the oauth client service name. If we wanted to get the other we could JOIN the table when doing analysis.

nshirley · 2026-05-20T20:19:21Z

After some further discussion, we landed on using a junction-like table between the accountActivity and scopes. However, the scopes table is not currently up to date so we're not FK onto the scope id, rather just storing the string for now and then we can update/migrate at a later date.

Few reasons for this:

We have to capture the scopes, because of how desktop auth works. The client_id on the request is always desktop, but we can't infer the service being used without introspecting the scopes
To that, we also must capture the scopes individually with their own lastUsedAt because another request from desktop (or any same clientId/userId combo), say for smartwindow scope, would update the accountActivity.lastUsedAt, essentially giving a false positive that another existing scope that was prior authorized was recently used.

This gives us:

The ability to centrally know account activity and last active status; Inactive-account-deletion liveness
Liveness for lapsed Smart Window, Sync etc. users, regardless of desktop service or direct oauth
Support tooling / dashboards showing user-to-service relationships

Testing notes. Signing in locally to a few services with initial signin and then re-authing or re-using the service after a few minutes generated the following results

-- used to join/validate the results
SELECT 
	c.name, 
    aats.scope,
    FROM_UNIXTIME(aa.firstSeenAt/1000) AS accountFirstSeenAt,
    FROM_UNIXTIME(aa.lastSeenAt/1000) AS accountLastSeenAt,
    FROM_UNIXTIME(aats.firstSeenAt/1000) AS scopeFirstSeenAt,
    FROM_UNIXTIME(aats.lastSeenAt/1000) AS scopeLastSeenAt
    FROM accountActivity AS aa
	LEFT JOIN accountActivityToScope 
		AS aats 
        USING (userId, clientId)
    LEFT JOIN clients 
		AS c 
        ON c.id = aa.clientId;

~~Okay, I've made the suggested improvements:~~

~~Remove the clientIdToServiceNames mapping and just relying on the scopeToServiceName mapping with suggested defaults~~
~~Added a throttling around the write so we can tune how fast we write the records~~
~~Few other clean ups~~

~~Here's some testing I did locally too after changes,~~

~~Initial sign into sync (joined to clients table for reference)~~
[removed for brevity]

Then, waited 5+ minutes with adjust throttleMs to make sure the db level write worked as expected (note the updated lastSeenAt left most column) I also signed into 123Done locally to generate the additional activity rows here
[removed for brevity]

Because: - FxA lacks a durable per-(userId, clientId) activity signal needed for inactive-account deletion, per-RP liveness, lapsed-user analysis (Smart Window, Sync), and support tooling. - Existing token tables either disappear on revoke, miss Firefox Desktop's online fxa-credentials service-token mints, or are sampled and Redis-biased and therefore under-detect active users. This commit: - Adds two new tables to the fxa_oauth schema. accountActivity (patch 36->37, PK (userId, clientId)) holds per-client liveness timestamps. accountActivityScopes (patch 37->38, PK (userId, clientId, scope), FK ON DELETE CASCADE to the parent) records each granted scope with its own firstSeenAt/lastSeenAt so a fresh grant for one scope does not pollute the per-scope liveness of another. - Indexes the scope mapping on (scope, lastSeenAt) for per-scope cohort queries (e.g. "users who last used Smart Window > 90 days ago") and on (lastSeenAt) for global time-window scans. - Adds a fire-and-forget recordActivity helper (lib/oauth/account-activity.ts) that writes via a SQL-throttled upsert. The throttle predicate uses 'VALUES(x) > x + ?' to avoid BIGINT UNSIGNED underflow on out-of-order timestamps. Per-scope upserts use the same predicate so each scope row's lastSeenAt advances independently of the parent's. - Wires a single write site at the OAuth grant convergence point in tokenHandler so all four grant types (authorization_code, fxa-credentials, refresh_token, token-exchange) are recorded uniformly. Sample-gated by oauthServer.accountActivity.sampleRate (default 0) so the writer can be ramped per environment after stage validation. - Extends _removeTokensAndCodes so accountActivity rows are cleaned up alongside tokens on account deletion; accountActivityScopes rows follow via the FK CASCADE. - Emits accountActivity.recorded / accountActivity.write_failed StatsD metrics tagged with clientId (routed through getClientServiceTags for cardinality safety) and grantType. - Adds an integration spec (test/remote/account_activity.in.spec.ts) covering the throttle window, per-scope independence (the pollution case), and FK CASCADE on account deletion. Adds a unit-level test in token.spec.ts that forces sampleRate = 1 to exercise the route-handler wiring. Closes FXA-13662

nshirley commented May 18, 2026

View reviewed changes

Comment thread packages/db-migrations/databases/fxa_oauth/patches/patch-033-034.sql

nshirley commented May 19, 2026

View reviewed changes

Comment thread packages/fxa-auth-server/lib/oauth/account-activity.ts Outdated

nshirley commented May 19, 2026

View reviewed changes

Comment thread packages/fxa-auth-server/lib/oauth/account-activity.ts Outdated

nshirley commented May 19, 2026

View reviewed changes

Comment thread packages/fxa-auth-server/lib/oauth/account-activity.ts Outdated

nshirley commented May 19, 2026

View reviewed changes

Comment thread packages/fxa-auth-server/lib/oauth/account-activity.ts Outdated

nshirley commented May 19, 2026

View reviewed changes

Comment thread packages/fxa-auth-server/lib/oauth/account-activity.ts Outdated

nshirley force-pushed the FXA-13662 branch from 11c4aaf to bfa3e1c Compare May 19, 2026 17:47

nshirley commented May 19, 2026

View reviewed changes

Comment thread packages/fxa-auth-server/lib/oauth/account-activity.ts Outdated

nshirley commented May 19, 2026

View reviewed changes

Comment thread packages/fxa-auth-server/lib/oauth/account-activity.ts Outdated

nshirley marked this pull request as ready for review May 19, 2026 19:15

nshirley requested a review from a team as a code owner May 19, 2026 19:15

Copilot AI review requested due to automatic review settings May 19, 2026 19:15

Copilot started reviewing on behalf of nshirley May 19, 2026 19:16 View session

Copilot AI reviewed May 19, 2026

View reviewed changes

Comment thread packages/fxa-auth-server/lib/routes/oauth/token.js Outdated

Comment thread packages/fxa-auth-server/lib/oauth/account-activity.ts Outdated

Comment thread packages/fxa-auth-server/lib/oauth/account-activity.ts Outdated

nshirley commented May 19, 2026

View reviewed changes

LZoog reviewed May 20, 2026

View reviewed changes

nshirley force-pushed the FXA-13662 branch 2 times, most recently from cce2474 to fecb319 Compare May 20, 2026 16:29

dschom reviewed May 20, 2026

View reviewed changes

Comment thread packages/fxa-auth-server/lib/oauth/account-activity.ts

dschom requested changes May 20, 2026

View reviewed changes

dschom reviewed May 20, 2026

View reviewed changes

vbudhram reviewed May 20, 2026

View reviewed changes

nshirley force-pushed the FXA-13662 branch from fecb319 to 8ece14c Compare May 20, 2026 20:20

nshirley requested a review from dschom May 20, 2026 20:59

nshirley force-pushed the FXA-13662 branch from 8ece14c to c2ea34b Compare May 26, 2026 04:56

nshirley force-pushed the FXA-13662 branch from c2ea34b to 9308b71 Compare May 26, 2026 14:37

Conversation

nshirley commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to review (Optional)

Screenshots (Optional)

Other information (Optional)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dschom May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dschom left a comment

Choose a reason for hiding this comment

Uh oh!

vbudhram left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nshirley commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

nshirley commented May 18, 2026 •

edited

Loading

dschom May 20, 2026 •

edited

Loading

vbudhram left a comment •

edited

Loading

nshirley commented May 20, 2026 •

edited

Loading