Skip to content

Conversation

@vbudhram
Copy link
Contributor

@vbudhram vbudhram commented Oct 3, 2025

Because

  • We want to emit some metrics when users skip confirmation from a seen device

This pull request

  • Emits glean metrics, records security events, updates statsd

Issue that this pull request solves

Closes: https://mozilla-hub.atlassian.net/browse/FXA-12477

Checklist

  • 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).

Screenshots (Optional)

security event 50 is skipped via seen device
Screenshot 2025-10-03 at 12 56 56 PM

Other information (Optional)

@vbudhram vbudhram requested a review from a team as a code owner October 3, 2025 16:54
@vbudhram vbudhram self-assigned this Oct 3, 2025
@vbudhram vbudhram requested a review from nshirley October 3, 2025 16:57
Copy link
Contributor

@nshirley nshirley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Code looks good. I think the only thing is the failing tests. Otherwise :shipit: once those are fixed!

@@ -0,0 +1,5 @@
CALL assertPatchLevel('177');

INSERT INTO securityEventNames(name) VALUES ('account.signin_confirm_bypass_known_device');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

welp, I just realized that I messed up the naming where the security event is signin_ and the glean events are login_ 😅 Probably not a big deal

loginConfirmSkipFor: {
knownIp: createEventFn('login_confirm_skip_for_known_ip'),
newAccount: createEventFn('login_confirm_skip_for_new_account'),
knownDevice: createEventFn('login_confirm_skip_for_known_device'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw tests failed, I think it's since this new glean event isn't in the packages/fxa-auth-server/test/local/metrics/glean.ts tests.

});

describe('skip for seen user agent', () => {
describe('skip for known device', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@vbudhram vbudhram merged commit f3c957b into main Oct 3, 2025
19 checks passed
@vbudhram vbudhram deleted the fxa-12477 branch October 3, 2025 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants