Skip to content

Commit 8e11cdd

Browse files
author
Matt Gaunt
committed
Last set of patches from review
1 parent 826d6d8 commit 8e11cdd

File tree

6 files changed

+77
-24
lines changed

6 files changed

+77
-24
lines changed

src/vapid-helper.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,11 @@ function getVapidHeaders(audience, subject, publicKey, privateKey, expiration) {
104104
throw new Error('Vapid private key should be 32 bytes long');
105105
}
106106

107+
if (expiration) {
108+
// TODO: Check if expiration is valid and use it in place of the hard coded
109+
// expiration of 24hours.
110+
}
111+
107112
const header = {
108113
typ: 'JWT',
109114
alg: 'ES256'
@@ -115,10 +120,6 @@ function getVapidHeaders(audience, subject, publicKey, privateKey, expiration) {
115120
sub: subject
116121
};
117122

118-
if (expiration) {
119-
console.log('expiration', expiration);
120-
}
121-
122123
const jwt = jws.sign({
123124
header: header,
124125
payload: jwtPayload,

src/web-push-lib.js

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,21 @@ WebPushLib.prototype.sendNotification =
103103
let timeToLive = DEFAULT_TTL;
104104

105105
if (options) {
106+
const validOptionKeys = [
107+
'gcmAPIKey',
108+
'vapidDetails',
109+
'TTL'
110+
];
111+
const optionKeys = Object.keys(options);
112+
for (let i = 0; i < optionKeys.length; i++) {
113+
const optionKey = optionKeys[i];
114+
if (validOptionKeys.indexOf(optionKey) === -1) {
115+
return Promise.reject('\'' + optionKey + '\' is an invalid option. ' +
116+
'The valid options are [\'' + validOptionKeys.join('\', \'') +
117+
'\'].');
118+
}
119+
}
120+
106121
if (options.gcmAPIKey) {
107122
currentGCMAPIKey = options.gcmAPIKey;
108123
}
@@ -160,6 +175,7 @@ WebPushLib.prototype.sendNotification =
160175

161176
const isGCM = subscription.endpoint.indexOf(
162177
'https://android.googleapis.com/gcm/send') === 0;
178+
// VAPID isn't supported by GCM hence the if, else if.
163179
if (isGCM) {
164180
if (!currentGCMAPIKey) {
165181
console.warn('Attempt to send push notification to GCM endpoint, ' +
@@ -168,7 +184,6 @@ WebPushLib.prototype.sendNotification =
168184
requestOptions.headers.Authorization = 'key=' + currentGCMAPIKey;
169185
}
170186
} else if (currentVapidDetails) {
171-
// VAPID isn't supported by GCM.
172187
const parsedUrl = url.parse(subscription.endpoint);
173188
const audience = parsedUrl.protocol + '//' +
174189
parsedUrl.hostname;
@@ -217,7 +232,6 @@ WebPushLib.prototype.sendNotification =
217232
});
218233

219234
pushRequest.on('error', function(e) {
220-
console.error(e);
221235
reject(e);
222236
});
223237

test/test-vapid-helper.js

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@ const assert = require('assert');
44
const webPush = require('../src/index');
55
const vapidHelper = require('../src/vapid-helper');
66

7+
const VALID_AUDIENCE = 'https://example.com';
8+
const VALID_SUBJECT_MAILTO = 'mailto: [email protected]';
9+
const VALID_SUBJECT_URL = 'https://exampe.com/contact';
10+
const VALID_PUBLIC_KEY = new Buffer(65);
11+
const VALID_PRIVATE_KEY = new Buffer(32);
12+
const VALID_EXPIRATION = Math.floor(Date.now() / 1000) + (60 * 60 * 12);
13+
714
suite('Test Vapid Helpers', function() {
815
test('is defined', function() {
916
assert(webPush.generateVAPIDKeys);
@@ -32,12 +39,6 @@ suite('Test Vapid Helpers', function() {
3239
});
3340

3441
test('should throw errors on bad input', function() {
35-
const VALID_AUDIENCE = 'https://example.com';
36-
const VALID_SUBJECT_MAILTO = 'mailto: [email protected]';
37-
const VALID_SUBJECT_URL = 'https://exampe.com/contact';
38-
const VALID_PUBLIC_KEY = new Buffer(65);
39-
const VALID_PRIVATE_KEY = new Buffer(5);
40-
4142
const badInputs = [
4243
function() {
4344
// No args
@@ -93,6 +94,32 @@ suite('Test Vapid Helpers', function() {
9394
});
9495
});
9596

96-
// TODO:2 good tests for the vapid headers
97-
// TODO: Make sure you can v
97+
test('should get valid VAPID headers', function() {
98+
const validInputs = [
99+
function() {
100+
return vapidHelper.getVapidHeaders(VALID_AUDIENCE, VALID_SUBJECT_URL,
101+
VALID_PUBLIC_KEY, VALID_PRIVATE_KEY);
102+
},
103+
function() {
104+
return vapidHelper.getVapidHeaders(VALID_AUDIENCE, VALID_SUBJECT_MAILTO,
105+
VALID_PUBLIC_KEY, VALID_PRIVATE_KEY);
106+
},
107+
function() {
108+
return vapidHelper.getVapidHeaders(VALID_AUDIENCE, VALID_SUBJECT_URL,
109+
VALID_PUBLIC_KEY, VALID_PRIVATE_KEY, VALID_EXPIRATION);
110+
}
111+
];
112+
113+
validInputs.forEach(function(validInput, index) {
114+
try {
115+
const headers = validInput();
116+
assert(headers.Authorization);
117+
assert(headers['Crypto-Key']);
118+
} catch (err) {
119+
console.warn('Valid input call for getVapidHeaders() threw an ' +
120+
'error. [' + index + ']');
121+
throw err;
122+
}
123+
});
124+
});
98125
});

test/testSelenium.js

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,6 @@
3535
const testDirectory = './test/output/';
3636

3737
webPush.setGCMAPIKey('AIzaSyAwmdX6KKd4hPfIcGU2SOfj9vuRDW6u-wo');
38-
webPush.setVapidDetails(
39-
VAPID_PARAM.subject,
40-
VAPID_PARAM.publicKey,
41-
VAPID_PARAM.privateKey
42-
);
4338

4439
let globalServer;
4540
let globalDriver;
@@ -179,15 +174,15 @@
179174

180175
if (!pushPayload) {
181176
promise = webPush.sendNotification(subscription, null, {
182-
vapid: vapid
177+
vapidDetails: vapid
183178
});
184179
} else {
185180
if (!subscription.keys) {
186181
throw new Error('Require subscription.keys not found.');
187182
}
188183

189184
promise = webPush.sendNotification(subscription, pushPayload, {
190-
vapid: vapid
185+
vapidDetails: vapid
191186
});
192187
}
193188

test/testSendNotification.js

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ suite('sendNotification', function() {
267267
},
268268
message: 'hello',
269269
extraOptions: {
270-
vapid: {
270+
vapidDetails: {
271271
subject: 'mailto:[email protected]',
272272
privateKey: vapidKeys.privateKey,
273273
publicKey: vapidKeys.publicKey
@@ -350,7 +350,7 @@ suite('sendNotification', function() {
350350
subscription: {
351351
},
352352
extraOptions: {
353-
vapid: {
353+
vapidDetails: {
354354
subject: 'mailto:[email protected]',
355355
privateKey: vapidKeys.privateKey,
356356
publicKey: vapidKeys.publicKey
@@ -553,6 +553,22 @@ suite('sendNotification', function() {
553553
},
554554
addEndpoint: true,
555555
serverFlags: ['statusCode=404']
556+
}, {
557+
testTitle: 'send notification with invalid vapid option',
558+
requestOptions: {
559+
subscription: {
560+
keys: VALID_KEYS
561+
},
562+
message: 'hello',
563+
addEndpoint: true,
564+
extraOptions: {
565+
vapid: {
566+
subject: 'mailto:[email protected]',
567+
privateKey: vapidKeys.privateKey,
568+
publicKey: vapidKeys.publicKey
569+
}
570+
}
571+
}
556572
}
557573
];
558574

test/testSetGCMAPIKey.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ suite('setGCMAPIKey', function() {
3232
}, Error);
3333
});
3434

35-
test('undefined valud', function() {
35+
test('undefined value', function() {
3636
assert.throws(function() {
3737
webPush.setGCMAPIKey();
3838
}, Error);

0 commit comments

Comments
 (0)