Skip to content

Commit c76e4c0

Browse files
jankapunktjankapunkt
committed
refactor: use param-util for parameter type checks and conversions
1 parent a4a14f4 commit c76e4c0

6 files changed

Lines changed: 138 additions & 31 deletions

File tree

‎lib/grant-types/authorization-code-grant-type.js‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ const InvalidRequestError = require('../errors/invalid-request-error');
1111
const ServerError = require('../errors/server-error');
1212
const isFormat = require('@node-oauth/formats');
1313
const pkce = require('../pkce/pkce');
14+
const { isDefined, toString, isInTypes } = require('../utils/param-util');
15+
const isStringOrNumber = isInTypes('string', 'number');
16+
const isString = isInTypes('string');
1417

1518
/**
1619
* @class
@@ -73,15 +76,21 @@ class AuthorizationCodeGrantType extends AbstractGrantType {
7376
*/
7477

7578
async getAuthorizationCode(request, client) {
76-
if (!request.body.code) {
79+
if (!isDefined(request.body.code)) {
7780
throw new InvalidRequestError('Missing parameter: `code`');
7881
}
7982

80-
if (!isFormat.vschar(request.body.code)) {
83+
if (!isStringOrNumber(request.body.code)) {
8184
throw new InvalidRequestError('Invalid parameter: `code`');
8285
}
8386

84-
const code = await this.model.getAuthorizationCode(request.body.code);
87+
const requestCode = toString(request.body.code);
88+
89+
if (!isFormat.vschar(requestCode)) {
90+
throw new InvalidRequestError('Invalid parameter: `code`');
91+
}
92+
93+
const code = await this.model.getAuthorizationCode(requestCode);
8594

8695
if (!code) {
8796
throw new InvalidGrantError('Invalid grant: authorization code is invalid');
@@ -163,7 +172,7 @@ class AuthorizationCodeGrantType extends AbstractGrantType {
163172

164173
const redirectUri = request.body.redirect_uri || request.query.redirect_uri;
165174

166-
if (!redirectUri || !isFormat.uri(redirectUri)) {
175+
if (!redirectUri || !isString(redirectUri) || !isFormat.uri(redirectUri)) {
167176
throw new InvalidRequestError('Invalid request: `redirect_uri` is not a valid URI');
168177
}
169178

‎lib/grant-types/password-grant-type.js‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ const InvalidArgumentError = require('../errors/invalid-argument-error');
99
const InvalidGrantError = require('../errors/invalid-grant-error');
1010
const InvalidRequestError = require('../errors/invalid-request-error');
1111
const isFormat = require('@node-oauth/formats');
12+
const { isDefined, isInTypes } = require('../utils/param-util');
13+
const isString = isInTypes('string');
1214

1315
/**
1416
* Constructor.
@@ -58,19 +60,19 @@ class PasswordGrantType extends AbstractGrantType {
5860
*/
5961

6062
async getUser(request, client) {
61-
if (!request.body.username) {
63+
if (!isDefined(request.body.username)) {
6264
throw new InvalidRequestError('Missing parameter: `username`');
6365
}
6466

65-
if (!request.body.password) {
67+
if (!isDefined(request.body.password)) {
6668
throw new InvalidRequestError('Missing parameter: `password`');
6769
}
6870

69-
if (!isFormat.uchar(request.body.username)) {
71+
if (!isString(request.body.username) || !isFormat.uchar(request.body.username)) {
7072
throw new InvalidRequestError('Invalid parameter: `username`');
7173
}
7274

73-
if (!isFormat.uchar(request.body.password)) {
75+
if (!isString(request.body.password) || !isFormat.uchar(request.body.password)) {
7476
throw new InvalidRequestError('Invalid parameter: `password`');
7577
}
7678

‎lib/grant-types/refresh-token-grant-type.js‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ const InvalidRequestError = require('../errors/invalid-request-error');
1111
const ServerError = require('../errors/server-error');
1212
const isFormat = require('@node-oauth/formats');
1313
const InvalidScopeError = require('../errors/invalid-scope-error');
14+
const { isInTypes, toString, isDefined } = require('../utils/param-util');
15+
const isStringOrNumber = isInTypes('string', 'number');
1416

1517
/**
1618
* Constructor.
@@ -64,17 +66,22 @@ class RefreshTokenGrantType extends AbstractGrantType {
6466
/**
6567
* Get refresh token.
6668
*/
67-
6869
async getRefreshToken(request, client) {
69-
if (!request.body.refresh_token) {
70+
if (!isDefined(request.body.refresh_token)) {
7071
throw new InvalidRequestError('Missing parameter: `refresh_token`');
7172
}
7273

73-
if (!isFormat.vschar(request.body.refresh_token)) {
74+
if (!isStringOrNumber(request.body.refresh_token)) {
75+
throw new InvalidRequestError('Invalid parameter: `refresh_token`');
76+
}
77+
78+
const refreshToken = toString(request.body.refresh_token);
79+
80+
if (!isFormat.vschar(refreshToken)) {
7481
throw new InvalidRequestError('Invalid parameter: `refresh_token`');
7582
}
7683

77-
const token = await this.model.getRefreshToken(request.body.refresh_token);
84+
const token = await this.model.getRefreshToken(refreshToken);
7885

7986
if (!token) {
8087
throw new InvalidGrantError('Invalid grant: refresh token is invalid');

‎lib/handlers/authorize-handler.js‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ const tokenUtil = require('../utils/token-util');
2121
const url = require('url');
2222
const pkce = require('../pkce/pkce');
2323
const { parseScope } = require('../utils/scope-util');
24+
const { isDefined, isInTypes } = require('../utils/param-util');
25+
const isString = isInTypes('string');
26+
const isStringOrNumber = isInTypes('string', 'number');
2427

2528
/**
2629
* Response types.
@@ -160,17 +163,17 @@ class AuthorizeHandler {
160163
const self = this;
161164
const clientId = request.body.client_id || request.query.client_id;
162165

163-
if (!clientId) {
166+
if (!isDefined(clientId)) {
164167
throw new InvalidRequestError('Missing parameter: `client_id`');
165168
}
166169

167-
if (!isFormat.vschar(clientId)) {
170+
if (!isStringOrNumber(clientId) || !isFormat.vschar(clientId)) {
168171
throw new InvalidRequestError('Invalid parameter: `client_id`');
169172
}
170173

171174
const redirectUri = request.body.redirect_uri || request.query.redirect_uri;
172175

173-
if (redirectUri && !isFormat.uri(redirectUri)) {
176+
if (isDefined(redirectUri) && (!isString(redirectUri) || !isFormat.uri(redirectUri))) {
174177
throw new InvalidRequestError('Invalid request: `redirect_uri` is not a valid URI');
175178
}
176179

@@ -236,7 +239,7 @@ class AuthorizeHandler {
236239

237240
getState (request) {
238241
const state = request.body.state || request.query.state;
239-
const stateExists = state && state.length > 0;
242+
const stateExists = isString(state) && state.length > 0;
240243
const stateIsValid = stateExists
241244
? isFormat.vschar(state)
242245
: this.allowEmptyState;

‎lib/utils/param-util.js‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
'use strict';
2+
3+
/**
4+
* @module StringUtil
5+
*/
6+
7+
/**
8+
* Create a function to check, whether a value is one of the given types.
9+
* @param types {...string[]}
10+
* @return {function(*): boolean}
11+
*/
12+
function isInTypes (...types) {
13+
return function (value) {
14+
return types.includes(typeof value);
15+
};
16+
}
17+
18+
/**
19+
* Check if a value is defined (not missing)
20+
* @param value
21+
* @return {boolean}
22+
*/
23+
function isDefined (value) {
24+
// TODO: in future versions, consider changing this to `value !== undefined && value !== null && value !== ''`,
25+
// which might be a breaking changes for some users
26+
return !!value;
27+
}
28+
29+
/**
30+
* Safely converts a value to a string in the following order:
31+
* - If the value is already a string, return it.
32+
* - If the value has a `toString` method, call it and return the result.
33+
* - If the value is `null` or `undefined`, return an empty string.
34+
* - Otherwise, use the global `String` function to convert the value.
35+
* @param value {*}
36+
* @return {string}
37+
*/
38+
function toString(value) {
39+
if (typeof value === 'string') {
40+
return value;
41+
}
42+
43+
if (Object.prototype.hasOwnProperty.call(value, 'toString')) {
44+
return value.toString();
45+
}
46+
47+
if (value === null || value === undefined) {
48+
return '';
49+
}
50+
51+
return String(value);
52+
}
53+
54+
module.exports = { isInTypes, isDefined, toString };

‎test/integration/grant-types/refresh-token-grant-type_test.js‎

Lines changed: 47 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -187,22 +187,49 @@ describe('RefreshTokenGrantType integration', function() {
187187

188188
describe('getRefreshToken()', function() {
189189
it('should throw an error if the `refreshToken` parameter is missing from the request body', async function() {
190+
const client = {};
191+
const model = Model.from({
192+
getRefreshToken: () => should.fail(),
193+
revokeToken: () => should.fail(),
194+
saveToken: () => should.fail()
195+
});
196+
const values = [null, undefined];
197+
198+
for (const value of values) {
199+
const grantType = new RefreshTokenGrantType({ accessTokenLifetime: 120, model });
200+
const request = new Request({ body: { refresh_token: value }, headers: {}, method: {}, query: {} });
201+
202+
try {
203+
await grantType.getRefreshToken(request, client);
204+
205+
should.fail();
206+
} catch (e) {
207+
e.should.be.an.instanceOf(InvalidRequestError);
208+
e.message.should.equal('Missing parameter: `refresh_token`');
209+
}
210+
}
211+
});
212+
213+
it('should throw an error if `refreshToken` is not a valid format', async () => {
190214
const client = {};
191215
const model = Model.from({
192216
getRefreshToken: () => should.fail(),
193217
revokeToken: () => should.fail(),
194218
saveToken: () => should.fail()
195219
});
196220
const grantType = new RefreshTokenGrantType({ accessTokenLifetime: 120, model });
197-
const request = new Request({ body: {}, headers: {}, method: {}, query: {} });
221+
const values = ['toke😇n', () => {}, [], Symbol('test')];
198222

199-
try {
200-
await grantType.getRefreshToken(request, client);
223+
for (const value of values) {
224+
const request = new Request({ body: { refresh_token: value }, headers: {}, method: {}, query: {} });
225+
try {
226+
await grantType.getRefreshToken(request, client);
201227

202-
should.fail();
203-
} catch (e) {
204-
e.should.be.an.instanceOf(InvalidRequestError);
205-
e.message.should.equal('Missing parameter: `refresh_token`');
228+
should.fail();
229+
} catch (e) {
230+
e.should.be.an.instanceOf(InvalidRequestError);
231+
e.message.should.equal('Invalid parameter: `refresh_token`');
232+
}
206233
}
207234
});
208235

@@ -233,6 +260,7 @@ describe('RefreshTokenGrantType integration', function() {
233260
revokeToken: () => should.fail(),
234261
saveToken: () => should.fail()
235262
});
263+
236264
const grantType = new RefreshTokenGrantType({ accessTokenLifetime: 120, model });
237265
const request = new Request({ body: { refresh_token: '12345' }, headers: {}, method: {}, query: {} });
238266

@@ -255,16 +283,20 @@ describe('RefreshTokenGrantType integration', function() {
255283
revokeToken: () => should.fail(),
256284
saveToken: () => should.fail()
257285
});
258-
const grantType = new RefreshTokenGrantType({ accessTokenLifetime: 120, model });
259-
const request = new Request({ body: { refresh_token: '12345' }, headers: {}, method: {}, query: {} });
260286

261-
try {
262-
await grantType.getRefreshToken(request, client);
287+
const values = [12345, '12345'];
288+
for (const value of values) {
289+
const grantType = new RefreshTokenGrantType({ accessTokenLifetime: 120, model });
290+
const request = new Request({ body: { refresh_token: value }, headers: {}, method: {}, query: {} });
263291

264-
should.fail();
265-
} catch (e) {
266-
e.should.be.an.instanceOf(ServerError);
267-
e.message.should.equal('Server error: `getRefreshToken()` did not return a `user` object');
292+
try {
293+
await grantType.getRefreshToken(request, client);
294+
295+
should.fail();
296+
} catch (e) {
297+
e.should.be.an.instanceOf(ServerError);
298+
e.message.should.equal('Server error: `getRefreshToken()` did not return a `user` object');
299+
}
268300
}
269301
});
270302

0 commit comments

Comments
 (0)