Bug 1635656 - add support for storing and fetching FxA ecosystem telemetry ids. r=rfkelly,markh

Differential Revision: https://phabricator.services.mozilla.com/D77762
This commit is contained in:
Ryan Kelly 2020-07-21 23:05:04 +00:00
parent 2d57d60bcc
commit fae3d55603
6 changed files with 296 additions and 17 deletions

View File

@ -278,6 +278,7 @@ exports.FXA_PWDMGR_PLAINTEXT_FIELDS = new Set([
"authAt",
"sessionToken",
"uid",
"ecosystemUserId",
"oauthTokens",
"profile",
"device",

View File

@ -163,6 +163,24 @@ FxAccountsProfile.prototype = {
return profileCache.profile;
},
// Get the user's profile data, fetching from the network if necessary.
// Most callers should instead use `getProfile()`; this methods exists to support
// callers who need to await the underlying network request.
async ensureProfile() {
const profileCache = await this._getProfileCache();
if (Date.now() > this._cachedAt + this.PROFILE_FRESHNESS_THRESHOLD) {
const profile = await this._fetchAndCacheProfile().catch(err => {
log.error("Background refresh of profile failed", err);
});
if (profile) {
return profile;
}
}
log.trace("not checking freshness of profile as it remains recent");
return profileCache.profile;
},
QueryInterface: ChromeUtils.generateQI([
"nsIObserver",
"nsISupportsWeakReference",

View File

@ -53,13 +53,6 @@ var FxAccountsProfileClient = function(options) {
}
this.fxai = options.fxai || fxAccounts._internal;
// This is a work-around for loop that manages its own oauth tokens.
// * If |token| is in options we use it and don't attempt any token refresh
// on 401. This is for loop.
// * If |token| doesn't exist we will fetch our own token. This is for the
// normal FxAccounts methods for obtaining the profile.
// We should nuke all |this.token| support once loop moves closer to FxAccounts.
this.token = options.token;
try {
this.serverURL = new URL(options.serverURL);
@ -99,21 +92,14 @@ FxAccountsProfileClient.prototype = {
* @private
*/
async _createRequest(path, method = "GET", etag = null) {
let token = this.token;
if (!token) {
// tokens are cached, so getting them each request is cheap.
token = await this.fxai.getOAuthToken(this.oauthOptions);
}
// tokens are cached, so getting them each request is cheap.
let token = await this.fxai.getOAuthToken(this.oauthOptions);
try {
return await this._rawRequest(path, method, token, etag);
} catch (ex) {
if (!(ex instanceof FxAccountsProfileClientError) || ex.code != 401) {
throw ex;
}
// If this object was instantiated with a token then we don't refresh it.
if (this.token) {
throw ex;
}
// it's an auth error - assume our token expired and retry.
log.info(
"Fetching the profile returned a 401 - revoking our token and retrying"

View File

@ -56,6 +56,61 @@ class FxAccountsTelemetry {
.slice(1, -1);
}
// Account Ecosystem Telemetry identifies the user by a secret id called their "ecosystemUserId".
// To maintain user privacy this value must never be shared with Mozilla servers in plaintext
// (although there may be some client-side-only features that use it in future).
//
// Instead, AET-related telemetry pings can identify the user by their "ecosystemAnonId",
// an encrypted bundle that can communicate the "ecosystemUserId" through to the telemetry
// backend without allowing it to be snooped on in transit.
// Get the user's ecosystemAnonId, or null if it's not available.
//
// This method is asynchronous because it may need to load data from storage, but it will not
// block on network access and will return null rather than throwing an error on failure. This is
// designed to simplify usage from telemetry-sending code, which may want to avoid making expensive
// network requests.
//
// If you want to ensure that a value is present then use `ensureEcosystemAnonId()` instead.
async getEcosystemAnonId() {
try {
// N.B. `getProfile()` may kick off a silent background update but won't await network requests.
const profile = await this._internal.profile.getProfile();
if (profile.hasOwnProperty("ecosystemAnonId")) {
return profile.ecosystemAnonId;
}
} catch (err) {
log.error("Getting ecosystemAnonId from profile failed", err);
}
// Calling `ensureEcosystemAnonId()` so the calling code doesn't have to do this when a call
// to `getProfile()` doesn't produce `ecosystemAnonId`.
this.ensureEcosystemAnonId().catch(err => {
log.error("Failed ensuring we have an anon-id in the background ", err);
});
return null;
}
// Get the user's ecosystemAnonId, fetching it from the server if necessary.
//
// This asynchronous method resolves with the "ecosystemAnonId" value on success, and rejects
// with an error if no user is signed in or if the value could not be obtained from the
// FxA server.
async ensureEcosystemAnonId() {
const profile = await this._internal.profile.ensureProfile();
if (!profile.hasOwnProperty("ecosystemAnonId")) {
// In a future iteration, we can synthesize a placeholder ecosystemAnonId and persist it
// back to the FxA server.
throw new Error("Profile data does not contain an 'ecosystemAnonId'");
}
return profile.ecosystemAnonId;
}
// Prior to Account Ecosystem Telemetry, FxA- and Sync-related metrics were submitted in
// a special-purpose "sync ping". This ping identified the user by a version of their FxA
// uid that was HMAC-ed with a server-side secret key, but this approach provides weaker
// privacy than "ecosystemAnonId" above. New metrics should prefer to use AET rather than
// the sync ping.
// Secret back-channel by which tokenserver client code can set the hashed UID.
// This value conceptually belongs to FxA, but we currently get it from tokenserver,
// so there's some light hackery to put it in the right place.

View File

@ -347,6 +347,119 @@ add_task(async function fetchAndCacheProfileAfterThreshold() {
Assert.equal(numFetches, 2);
});
add_task(async function test_ensureProfile() {
let client = new FxAccountsProfileClient({
serverURL: "http://127.0.0.1:1111/v1",
fxa: mockFxa(),
});
let profile = CreateFxAccountsProfile(null, client);
const testCases = [
// profile retrieval when there is no cached profile info
{
threshold: 1000,
hasRecentCachedProfile: false,
cachedProfile: null,
fetchedProfile: {
uid: ACCOUNT_UID,
email: ACCOUNT_EMAIL,
avatar: "myimg",
},
},
// profile retrieval when the cached profile is recent
{
// Note: The threshold for this test case is being set to an arbitrary value that will
// be greater than Date.now() so the retrieved cached profile will be deemed recent.
threshold: Date.now() + 5000,
hasRecentCachedProfile: true,
cachedProfile: {
uid: `${ACCOUNT_UID}2`,
email: `${ACCOUNT_EMAIL}2`,
avatar: "myimg2",
},
},
// profile retrieval when the cached profile is old and a new profile is fetched
{
threshold: 1000,
hasRecentCachedProfile: false,
cachedProfile: {
uid: `${ACCOUNT_UID}3`,
email: `${ACCOUNT_EMAIL}3`,
avatar: "myimg3",
},
fetchAndCacheProfileResolves: true,
fetchedProfile: {
uid: `${ACCOUNT_UID}4`,
email: `${ACCOUNT_EMAIL}4`,
avatar: "myimg4",
},
},
// profile retrieval when the cached profile is old and a null profile is fetched
{
threshold: 1000,
hasRecentCachedProfile: false,
cachedProfile: {
uid: `${ACCOUNT_UID}5`,
email: `${ACCOUNT_EMAIL}5`,
avatar: "myimg5",
},
fetchAndCacheProfileResolves: true,
fetchedProfile: null,
},
// profile retrieval when the cached profile is old and fetching a new profile errors
{
threshold: 1000,
hasRecentCachedProfile: false,
cachedProfile: {
uid: `${ACCOUNT_UID}6`,
email: `${ACCOUNT_EMAIL}6`,
avatar: "myimg6",
},
fetchAndCacheProfileResolves: false,
},
];
for (const tc of testCases) {
let mockProfile = sinon.mock(profile);
mockProfile
.expects("_getProfileCache")
.once()
.returns({
profile: tc.cachedProfile,
});
profile.PROFILE_FRESHNESS_THRESHOLD = tc.threshold;
if (tc.hasRecentCachedProfile) {
mockProfile.expects("_fetchAndCacheProfile").never();
let actualProfile = await profile.ensureProfile();
mockProfile.verify();
Assert.equal(actualProfile, tc.cachedProfile);
} else if (tc.fetchAndCacheProfileResolves) {
mockProfile
.expects("_fetchAndCacheProfile")
.once()
.resolves(tc.fetchedProfile);
let actualProfile = await profile.ensureProfile();
let expectedProfile = tc.fetchedProfile
? tc.fetchedProfile
: tc.cachedProfile;
mockProfile.verify();
Assert.equal(actualProfile, expectedProfile);
} else {
mockProfile
.expects("_fetchAndCacheProfile")
.once()
.rejects();
let actualProfile = await profile.ensureProfile();
mockProfile.verify();
Assert.equal(actualProfile, tc.cachedProfile);
}
}
});
// Check that a new profile request within PROFILE_FRESHNESS_THRESHOLD of the
// last one *does* kick off a new request if ON_PROFILE_CHANGE_NOTIFICATION
// is sent.

View File

@ -3,7 +3,7 @@
"use strict";
const { fxAccounts } = ChromeUtils.import(
const { fxAccounts, FxAccounts } = ChromeUtils.import(
"resource://gre/modules/FxAccounts.jsm"
);
@ -11,6 +11,14 @@ const { PREF_ACCOUNT_ROOT } = ChromeUtils.import(
"resource://gre/modules/FxAccountsCommon.js"
);
const { FxAccountsProfile } = ChromeUtils.import(
"resource://gre/modules/FxAccountsProfile.jsm"
);
const { FxAccountsTelemetry } = ChromeUtils.import(
"resource://gre/modules/FxAccountsTelemetry.jsm"
);
_("Misc tests for FxAccounts.telemetry");
const MOCK_HASHED_UID = "00112233445566778899aabbccddeeff";
@ -53,3 +61,101 @@ add_task(function test_sanitize_device_id() {
fxAccounts.telemetry._setHashedUID("");
Assert.equal(fxAccounts.telemetry.sanitizeDeviceId(MOCK_DEVICE_ID), null);
});
add_task(async function test_getEcosystemAnonId() {
let ecosystemAnonId = "aaaaaaaaaaaaaaa";
let testCases = [
{
// testing retrieving the ecosystemAnonId when the profile contains it
throw: false,
profileObj: { ecosystemAnonId },
expectedEcosystemAnonId: ecosystemAnonId,
},
{
// testing retrieving the ecosystemAnonId when the profile doesn't contain it
throw: false,
profileObj: {},
expectedEcosystemAnonId: null,
},
{
// testing retrieving the ecosystemAnonId when the profile is null
throw: true,
profileObj: null,
expectedEcosystemAnonId: null,
},
];
for (const tc of testCases) {
let profile = new FxAccountsProfile({ profileServerUrl: "http://testURL" });
let telemetry = new FxAccountsTelemetry({});
telemetry._internal = { profile };
const mockProfile = sinon.mock(profile);
const mockTelemetry = sinon.mock(telemetry);
if (tc.throw) {
mockProfile
.expects("getProfile")
.once()
.throws(Error);
} else {
mockProfile
.expects("getProfile")
.once()
.returns(tc.profileObj);
}
if (tc.expectedEcosystemAnonId) {
mockTelemetry.expects("ensureEcosystemAnonId").never();
} else {
mockTelemetry
.expects("ensureEcosystemAnonId")
.once()
.resolves("dddddddddd");
}
let actualEcoSystemAnonId = await telemetry.getEcosystemAnonId();
mockProfile.verify();
mockTelemetry.verify();
Assert.equal(actualEcoSystemAnonId, tc.expectedEcosystemAnonId);
}
});
add_task(async function test_ensureEcosystemAnonId() {
let ecosystemAnonId = "bbbbbbbbbbbbbb";
let testCases = [
{
profile: {
ecosystemAnonId,
},
willErr: false,
},
{
profile: {},
willErr: true,
},
];
for (const tc of testCases) {
let profile = new FxAccountsProfile({ profileServerUrl: "http://testURL" });
let telemetry = new FxAccountsTelemetry({});
telemetry._internal = { profile };
let mockProfile = sinon.mock(profile);
mockProfile
.expects("ensureProfile")
.once()
.returns(tc.profile);
if (tc.willErr) {
const expectedError = `Profile data does not contain an 'ecosystemAnonId'`;
try {
await telemetry.ensureEcosystemAnonId();
} catch (e) {
Assert.equal(e.message, expectedError);
}
} else {
let actualEcoSystemAnonId = await telemetry.ensureEcosystemAnonId();
Assert.equal(actualEcoSystemAnonId, ecosystemAnonId);
}
mockProfile.verify();
}
});