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.

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

Introduce device id v2 #362

Merged
merged 5 commits into from Dec 23, 2019
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

fix legacy device id reset issue

fix test

add comments
  • Loading branch information
darkdh committed Dec 2, 2019
commit 0387b4ec20788c34905e69162028183674f122ba
@@ -34,12 +34,17 @@ const messages = {
* saved. 'config' contains apiVersion, serverUrl, debug;
* see server/config/default.json
*/
GOT_INIT_DATA: _, /* @param {Array} seed, @param {Array} deviceId, @param {Object} config */
GOT_INIT_DATA: _, /* @param {Array} seed, @param {Array} deviceId,
* @param {Object} config, @param {string} deviceIdV2
* */
/**
* webview -> browser
* browser must save values in persistent storage if non-empty
* It will be called during initial setup or deviceIdV2 migration
*/
SAVE_INIT_DATA: _, /* @param {Uint8Array} seed, @param {Uint8Array} deviceId */
SAVE_INIT_DATA: _, /* @param {Uint8Array} seed, @param {Uint8Array} deviceId,
* @param {string} deviceIdV2
*/
/**
* webview -> browser
* sent when sync has finished initialization
@@ -251,18 +251,21 @@ RequestUtil.prototype.list = function (category, startAt, maxRecords, nextContin
WaitTimeSeconds: CONFIG.SQS_MESSAGES_LONGPOLL_TIMEOUT
}

// We will fetch from both old and new SQS queues until old one gets retired
if (this.oldSQSUrlByCat[category]) {
let oldNotificationParams = Object.assign({}, notificationParams)
oldNotificationParams.QueueUrl = `${this.oldSQSUrlByCat[category]}`

return s3Helper.listNotifications(this.sqs, notificationParams, category, prefix).then((values) => {
return s3Helper.listNotifications(
this.sqs, notificationParams, category, prefix).then((values) => {
if (this.shouldRetireOldSQSQueue(parseInt(values.createdTimeStamp))) {
return this.deleteSQSQueue(this.oldSQSUrlByCat[category]).then(() => {

This comment has been minimized.

Copy link
@darkdh

darkdh Dec 12, 2019

Author Member

@AlexeyBarabash reminded me that when device id is duplicated, if there is an old Brave which doesn't contain the fix, its SQS queue will be unavailable until next relaunch because upgraded Brave deletes it after 24 hours

This comment has been minimized.

Copy link
@AlexeyBarabash

AlexeyBarabash Dec 12, 2019

Contributor

I was confused and I thought this.deleteSQSQueue can delete queues belonging to other devices. And like @darkdh mentioned that only can happen if device id was duplicated. Which is a separate case.

This comment has been minimized.

Copy link
@darkdh

darkdh Dec 13, 2019

Author Member

Non upgraded device will get reset anyway, so duplicate case won’t be an issue

delete this.oldSQSUrlByCat[category]
return values
})
}
return s3Helper.listNotifications(this.sqs, oldNotificationParams, category, prefix).then((oldValues) => {
return s3Helper.listNotifications(
this.sqs, oldNotificationParams, category, prefix).then((oldValues) => {
if (deepEqual(values.contents, oldValues.contents, {strict: true})) {
return values
}
@@ -296,6 +299,13 @@ RequestUtil.prototype.shouldListObject = function (startAt, category) {
this.listInProgress === true
}

/**
* The retention time of our SQS queue is 24 hours so we will retire old SQS
* queue created by device id after 24 hours
* @param {number=} createdTimestamp is the when new device id v2 queue was
* created
* @returns {boolean}
*/
RequestUtil.prototype.shouldRetireOldSQSQueue = function (createdTimestamp) {
let currentTime = new Date().getTime()
let newQueueCreatedTime =
@@ -423,7 +433,6 @@ RequestUtil.prototype.createAndSubscribeSQS = function (deviceId, deviceIdV2) {
// Doesn't have to create about to deprecated queues
createSQSPromises.push(subscribeOldSQSforCategory(deviceId, CATEGORIES_FOR_SQS[i], this))
createSQSPromises.push(createAndSubscribeSQSforCategory(deviceIdV2, CATEGORIES_FOR_SQS[i], this))
// TODO(darkdh): encode into base64
}

return Promise.all(createSQSPromises)
@@ -604,6 +613,10 @@ RequestUtil.prototype.purgeUserCategoryQueue = function (category) {
})
}

/**
* Delete SQS queue by url
* @param {string} url - SQS queue url
*/
RequestUtil.prototype.deleteSQSQueue = function (url) {
return new Promise((resolve, reject) => {
let params = {
@@ -52,7 +52,11 @@ const logSync = (message, logLevel = DEBUG) => {
* @returns {Promise}
*/
const maybeSetDeviceId = (requester) => {
if (clientDeviceId !== null && clientDeviceIdV2.length !== 0 ) {
if (clientDeviceId !== null) {
if (clientDeviceIdV2.length === 0) {
clientDeviceIdV2 = generateDeviceIdV2()
ipc.send(messages.SAVE_INIT_DATA, seed, clientDeviceId, clientDeviceIdV2)
}
return Promise.resolve(requester)
}
if (!requester || !requester.s3) {
@@ -113,6 +113,10 @@ module.exports.decrypt = function (ciphertext/* : Uint8Array */,
return nacl.secretbox.open(ciphertext, nonce, secretboxKey)
}

module.exports.generateDeviceIdV2 = function() {
/**
* Generate device id v2 which is a 3 random bytes hex encoded string
* @returns {string} device id v2
*/
module.exports.generateDeviceIdV2 = function () {
return Buffer.from(nacl.randomBytes(3)).toString('hex')
}
@@ -2,6 +2,7 @@ const test = require('tape')
const testHelper = require('../testHelper')
const timekeeper = require('timekeeper')
const proto = require('../../client/constants/proto')
const {generateDeviceIdV2} = require('../../lib/crypto')
const recordUtil = require('../../client/recordUtil')
const Serializer = require('../../lib/serializer')

@@ -548,8 +549,10 @@ test('recordUtil.syncRecordAsJS()', (t) => {
})
conversionEquals({ objectData: 'siteSetting', siteSetting })

const deviceIdV2 = generateDeviceIdV2()
const device = serializer.api.SyncRecord.Device.create({
name: 'mobile pyramid'
name: 'mobile pyramid',
deviceIdV2
})
conversionEquals({ objectData: 'device', device })
})
@@ -2,6 +2,7 @@ const test = require('tape')
const testHelper = require('../testHelper')
const timekeeper = require('timekeeper')
const clientTestHelper = require('./testHelper')
const {generateDeviceIdV2} = require('../../lib/crypto')
const Serializer = require('../../lib/serializer')
const RequestUtil = require('../../client/requestUtil')
const proto = require('../../client/constants/proto')
@@ -30,7 +31,7 @@ test('client RequestUtil', (t) => {
}

const requestUtil = new RequestUtil(args)
requestUtil.createAndSubscribeSQS('0')
requestUtil.createAndSubscribeSQS('0', generateDeviceIdV2())
.then(()=> {
t.pass('can instantiate requestUtil')
t.test('prototype', (t) => {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.