Skip to content
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.

Sign up for GitHub

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

Jump to bottom

refactor(api): prefer async/await in device/session routes #2301

Merged
merged 2 commits into from
Aug 22, 2019
Merged

refactor(api): prefer async/await in device/session routes #2301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
86621f2
fix(tests): fix broken proxyquire path in redis test
philbooth Aug 22, 2019
36aee9e
refactor(api): prefer async/await in device/session routes
philbooth Aug 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
324 changes: 158 additions & 166 deletions packages/fxa-auth-server/lib/routes/devices-and-sessions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,10 +247,9 @@ module.exports = (
throw new error.featureNotEnabled();
}

return pushbox.retrieve(uid, deviceId, limit, index).then(resp => {
log.info('commands.fetch', { resp: resp });
return resp;
});
const response = await pushbox.retrieve(uid, deviceId, limit, index);
log.info('commands.fetch', { response });
return response;
},
},
{
Expand Down Expand Up @@ -293,45 +292,38 @@ module.exports = (
throw new error.featureNotEnabled();
}

return customs
.checkAuthenticated(request, uid, 'invokeDeviceCommand')
.then(() => db.device(uid, target))
.then(device => {
if (!device.availableCommands.hasOwnProperty(command)) {
throw error.unavailableDeviceCommand();
}
// 0 is perfectly acceptable TTL, hence the strict equality check.
if (ttl === undefined && DEFAULT_COMMAND_TTL.has(command)) {
ttl = DEFAULT_COMMAND_TTL.get(command);
}
const data = {
command,
payload,
sender,
};
return pushbox
.store(uid, device.id, data, ttl)
.then(({ index }) => {
const url = new URL(
'v1/account/device/commands',
config.publicUrl
);
url.searchParams.set('index', index);
url.searchParams.set('limit', 1);
return push.notifyCommandReceived(
uid,
device,
command,
sender,
index,
url.href,
ttl
);
});
})
.then(() => {
return {};
});
const [, device] = await Promise.all([
customs.checkAuthenticated(request, uid, 'invokeDeviceCommand'),
db.device(uid, target),
]);

if (!device.availableCommands.hasOwnProperty(command)) {
throw error.unavailableDeviceCommand();
}

// 0 is perfectly acceptable TTL, hence the strict equality check.
if (ttl === undefined && DEFAULT_COMMAND_TTL.has(command)) {
ttl = DEFAULT_COMMAND_TTL.get(command);
}

const data = { command, payload, sender };
const { index } = await pushbox.store(uid, device.id, data, ttl);

const url = new URL('v1/account/device/commands', config.publicUrl);
url.searchParams.set('index', index);
url.searchParams.set('limit', 1);

await push.notifyCommandReceived(
uid,
device,
command,
sender,
index,
url.href,
ttl
);

return {};
},
},
{
Expand Down Expand Up @@ -430,66 +422,62 @@ module.exports = (
pushOptions.TTL = body.TTL;
}

return customs
.checkAuthenticated(request, uid, endpointAction)
.then(() => request.app.devices)
.then(devices => {
if (body.to !== 'all') {
const include = new Set(body.to);
devices = devices.filter(device => include.has(device.id));

if (devices.length === 0) {
log.error('Account.devicesNotify', {
uid: uid,
error: 'devices empty',
});
return;
}
} else if (body.excluded) {
const exclude = new Set(body.excluded);
devices = devices.filter(device => !exclude.has(device.id));
}

return push
.sendPush(uid, devices, endpointAction, pushOptions)
.catch(catchPushError);
})
.then(() => {
// Emit a metrics event for when a user sends tabs between devices.
// In the future we will aim to get this event directly from sync telemetry,
// but we're doing it here for now as a quick way to get metrics on the feature.
if (
payload &&
payload.command === 'sync:collection_changed' &&
// Note that payload schema validation ensures that these properties exist.
payload.data.collections.length === 1 &&
payload.data.collections[0] === 'clients'
) {
let deviceId = undefined;

if (sessionToken.deviceId) {
deviceId = sessionToken.deviceId;
}

return request.emitMetricsEvent('sync.sentTabToDevice', {
device_id: deviceId,
service: 'sync',
uid: uid,
});
}
})
.then(() => {
return {};
});
let [, deviceArray] = await Promise.all([
customs.checkAuthenticated(request, uid, endpointAction),
request.app.devices,
]);

if (body.to !== 'all') {
const include = new Set(body.to);
deviceArray = deviceArray.filter(device => include.has(device.id));

if (deviceArray.length === 0) {
log.error('Account.devicesNotify', {
uid: uid,
error: 'devices empty',
});
return;
}
} else if (body.excluded) {
const exclude = new Set(body.excluded);
deviceArray = deviceArray.filter(device => !exclude.has(device.id));
}

function catchPushError(err) {
try {
await push.sendPush(uid, deviceArray, endpointAction, pushOptions);
} catch (err) {
// push may fail due to not found devices or a bad push action
// log the error but still respond with a 200.
// log the error but still respond with a 200
log.error('Account.devicesNotify', {
uid: uid,
error: err,
});
}

// Emit a metrics event for when a user sends tabs between devices.
// In the future we will aim to get this event directly from sync telemetry,
// but we're doing it here for now as a quick way to get metrics on the feature.
if (
payload &&
payload.command === 'sync:collection_changed' &&
// Note that payload schema validation ensures that these properties exist.
payload.data.collections.length === 1 &&
payload.data.collections[0] === 'clients'
) {
let deviceId;

if (sessionToken.deviceId) {
deviceId = sessionToken.deviceId;
}

await request.emitMetricsEvent('sync.sentTabToDevice', {
device_id: deviceId,
service: 'sync',
uid: uid,
});
}

return {};
},
},
{
Expand Down Expand Up @@ -556,34 +544,36 @@ module.exports = (
throw new error.featureNotEnabled();
}

return request.app.devices.then(deviceArray => {
return deviceArray.map(device => {
const formattedDevice = {
id: device.id,
isCurrentDevice: !!(
(credentials.id && credentials.id === device.sessionTokenId) ||
(credentials.refreshTokenId &&
credentials.refreshTokenId === device.refreshTokenId)
),
lastAccessTime: device.lastAccessTime,
location: device.location,
name: device.name || devices.synthesizeName(device),
// For now we assume that all oauth clients that register a device record are mobile apps.
// Ref https://github.com/mozilla/fxa/issues/449
type:
device.type ||
device.uaDeviceType ||
(device.refreshTokenId ? 'mobile' : 'desktop'),
pushCallback: device.pushCallback,
pushPublicKey: device.pushPublicKey,
pushAuthKey: device.pushAuthKey,
pushEndpointExpired: device.pushEndpointExpired,
availableCommands: device.availableCommands,
};
clientUtils.formatTimestamps(formattedDevice, request);
clientUtils.formatLocation(formattedDevice, request);
return formattedDevice;
});
const deviceArray = await request.app.devices;

return deviceArray.map(device => {
const formattedDevice = {
id: device.id,
isCurrentDevice: !!(
(credentials.id && credentials.id === device.sessionTokenId) ||
(credentials.refreshTokenId &&
credentials.refreshTokenId === device.refreshTokenId)
),
lastAccessTime: device.lastAccessTime,
location: device.location,
name: device.name || devices.synthesizeName(device),
// For now we assume that all oauth clients that register a device record are mobile apps.
// Ref https://github.com/mozilla/fxa/issues/449
type:
device.type ||
device.uaDeviceType ||
(device.refreshTokenId ? 'mobile' : 'desktop'),
pushCallback: device.pushCallback,
pushPublicKey: device.pushPublicKey,
pushAuthKey: device.pushAuthKey,
pushEndpointExpired: device.pushEndpointExpired,
availableCommands: device.availableCommands,
};

clientUtils.formatTimestamps(formattedDevice, request);
clientUtils.formatLocation(formattedDevice, request);

return formattedDevice;
});
},
},
Expand Down Expand Up @@ -676,48 +666,50 @@ module.exports = (
const sessionToken = request.auth.credentials;
const uid = sessionToken.uid;

return db.sessions(uid).then(sessions => {
return sessions.map(session => {
const deviceId = session.deviceId;
const isDevice = !!deviceId;

let deviceName = session.deviceName;
if (!deviceName) {
deviceName = devices.synthesizeName(session);
}

let userAgent;
if (!session.uaBrowser) {
userAgent = '';
} else if (!session.uaBrowserVersion) {
userAgent = session.uaBrowser;
} else {
const { uaBrowser: browser, uaBrowserVersion: version } = session;
userAgent = `${browser} ${version.split('.')[0]}`;
}

const formattedSession = {
deviceId,
deviceName,
deviceType: session.uaDeviceType || 'desktop',
deviceAvailableCommands: session.deviceAvailableCommands || null,
deviceCallbackURL: session.deviceCallbackURL,
deviceCallbackPublicKey: session.deviceCallbackPublicKey,
deviceCallbackAuthKey: session.deviceCallbackAuthKey,
deviceCallbackIsExpired: !!session.deviceCallbackIsExpired,
id: session.id,
isCurrentDevice: session.id === sessionToken.id,
isDevice,
lastAccessTime: session.lastAccessTime,
location: session.location,
createdTime: session.createdAt,
os: session.uaOS,
userAgent,
};
clientUtils.formatTimestamps(formattedSession, request);
clientUtils.formatLocation(formattedSession, request);
return formattedSession;
});
const sessions = await db.sessions(uid);

return sessions.map(session => {
const deviceId = session.deviceId;
const isDevice = !!deviceId;

let deviceName = session.deviceName;
if (!deviceName) {
deviceName = devices.synthesizeName(session);
}

let userAgent;
if (!session.uaBrowser) {
userAgent = '';
} else if (!session.uaBrowserVersion) {
userAgent = session.uaBrowser;
} else {
const { uaBrowser: browser, uaBrowserVersion: version } = session;
userAgent = `${browser} ${version.split('.')[0]}`;
}

const formattedSession = {
deviceId,
deviceName,
deviceType: session.uaDeviceType || 'desktop',
deviceAvailableCommands: session.deviceAvailableCommands || null,
deviceCallbackURL: session.deviceCallbackURL,
deviceCallbackPublicKey: session.deviceCallbackPublicKey,
deviceCallbackAuthKey: session.deviceCallbackAuthKey,
deviceCallbackIsExpired: !!session.deviceCallbackIsExpired,
id: session.id,
isCurrentDevice: session.id === sessionToken.id,
isDevice,
lastAccessTime: session.lastAccessTime,
location: session.location,
createdTime: session.createdAt,
os: session.uaOS,
userAgent,
};

clientUtils.formatTimestamps(formattedSession, request);
clientUtils.formatLocation(formattedSession, request);

return formattedSession;
});
},
},
Expand Down
2 changes: 1 addition & 1 deletion packages/fxa-auth-server/test/local/redis.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe('lib/redis:', () => {
};
fxaShared = sinon.spy(() => redis);
wrapper = proxyquire(`${LIB_DIR}/redis`, {
'../../../fxa-shared/redis': fxaShared,
'../../fxa-shared/redis': fxaShared,
Copy link
Member

Choose a reason for hiding this comment

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

oh, I was wondering why this test kept failing locally!

})(config, log);
});

Expand Down