Security: explicit string requirement for request parameters (client id, tokens etc.)

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

@@ -11,6 +11,9 @@ const InvalidRequestError = require('../errors/invalid-request-error');
 const ServerError = require('../errors/server-error');
 const isFormat = require('@node-oauth/formats');
 const pkce = require('../pkce/pkce');
+const { isDefined, toString, isInTypes } = require('../utils/param-util');
+const isStringOrNumber = isInTypes('string', 'number');
+const isString = isInTypes('string');
 
 /**
  * @class
@@ -73,14 +76,22 @@ class AuthorizationCodeGrantType extends AbstractGrantType {
      */
 
   async getAuthorizationCode(request, client) {
-    if (!request.body.code) {
+    if (!isDefined(request.body.code)) {
       throw new InvalidRequestError('Missing parameter: `code`');
     }
 
-    if (!isFormat.vschar(request.body.code)) {
+    if (!isStringOrNumber(request.body.code)) {
       throw new InvalidRequestError('Invalid parameter: `code`');
     }
 
+    // normalize string|number to string
+    const requestCode = toString(request.body.code);
+
+    if (!isFormat.vschar(requestCode)) {
+      throw new InvalidRequestError('Invalid parameter: `code`');
+    }
+
+    // XXX: still passing the original value from request to model
     const code = await this.model.getAuthorizationCode(request.body.code);
 
     if (!code) {
@@ -163,7 +174,7 @@ class AuthorizationCodeGrantType extends AbstractGrantType {
 
     const redirectUri = request.body.redirect_uri || request.query.redirect_uri;
 
-    if (!isFormat.uri(redirectUri)) {
+    if (!redirectUri || !isString(redirectUri) || !isFormat.uri(redirectUri)) {
       throw new InvalidRequestError('Invalid request: `redirect_uri` is not a valid URI');
     }
 

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

@@ -9,6 +9,8 @@ const InvalidArgumentError = require('../errors/invalid-argument-error');
 const InvalidGrantError = require('../errors/invalid-grant-error');
 const InvalidRequestError = require('../errors/invalid-request-error');
 const isFormat = require('@node-oauth/formats');
+const { isDefined, isInTypes } = require('../utils/param-util');
+const isString = isInTypes('string');
 
 /**
  * Constructor.
@@ -58,19 +60,19 @@ class PasswordGrantType extends AbstractGrantType {
 	 */
 
   async getUser(request, client) {
-    if (!request.body.username) {
+    if (!isDefined(request.body.username)) {
       throw new InvalidRequestError('Missing parameter: `username`');
     }
 
-    if (!request.body.password) {
+    if (!isDefined(request.body.password)) {
       throw new InvalidRequestError('Missing parameter: `password`');
     }
 
-    if (!isFormat.uchar(request.body.username)) {
+    if (!isString(request.body.username) || !isFormat.uchar(request.body.username)) {
       throw new InvalidRequestError('Invalid parameter: `username`');
     }
 
-    if (!isFormat.uchar(request.body.password)) {
+    if (!isString(request.body.password) || !isFormat.uchar(request.body.password)) {
       throw new InvalidRequestError('Invalid parameter: `password`');
     }
 

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

@@ -11,6 +11,8 @@ const InvalidRequestError = require('../errors/invalid-request-error');
 const ServerError = require('../errors/server-error');
 const isFormat = require('@node-oauth/formats');
 const InvalidScopeError = require('../errors/invalid-scope-error');
+const { isInTypes, toString, isDefined } = require('../utils/param-util');
+const isStringOrNumber = isInTypes('string', 'number');
 
 /**
  * Constructor.
@@ -64,16 +66,23 @@ class RefreshTokenGrantType extends AbstractGrantType {
   /**
 	 * Get refresh token.
 	 */
-
   async getRefreshToken(request, client) {
-    if (!request.body.refresh_token) {
+    if (!isDefined(request.body.refresh_token)) {
       throw new InvalidRequestError('Missing parameter: `refresh_token`');
     }
 
-    if (!isFormat.vschar(request.body.refresh_token)) {
+    if (!isStringOrNumber(request.body.refresh_token)) {
+      throw new InvalidRequestError('Invalid parameter: `refresh_token`');
+    }
+
+    // normalize string|number to string
+    const refreshToken = toString(request.body.refresh_token);
+
+    if (!isFormat.vschar(refreshToken)) {
       throw new InvalidRequestError('Invalid parameter: `refresh_token`');
     }
 
+    // XXX: still passing the original value from request to model
     const token = await this.model.getRefreshToken(request.body.refresh_token);
 
     if (!token) {

lib/handlers/authorize-handler.js

@@ -21,6 +21,9 @@ const tokenUtil = require('../utils/token-util');
 const url = require('url');
 const pkce = require('../pkce/pkce');
 const { parseScope } = require('../utils/scope-util');
+const { isDefined, isInTypes } = require('../utils/param-util');
+const isString = isInTypes('string');
+const isStringOrNumber = isInTypes('string', 'number');
 
 /**
  * Response types.
@@ -160,17 +163,18 @@ class AuthorizeHandler {
     const self = this;
     const clientId = request.body.client_id || request.query.client_id;
 
-    if (!clientId) {
+    if (!isDefined(clientId)) {
       throw new InvalidRequestError('Missing parameter: `client_id`');
     }
 
-    if (!isFormat.vschar(clientId)) {
+    if (!isStringOrNumber(clientId) || !isFormat.vschar(clientId)) {
       throw new InvalidRequestError('Invalid parameter: `client_id`');
     }
 
     const redirectUri = request.body.redirect_uri || request.query.redirect_uri;
 
-    if (redirectUri && !isFormat.uri(redirectUri)) {
+    // XXX: why is there no 'Missing parameter: `redirect_uri`' error?
+    if (isDefined(redirectUri) && (!isString(redirectUri) || !isFormat.uri(redirectUri))) {
       throw new InvalidRequestError('Invalid request: `redirect_uri` is not a valid URI');
     }
 
@@ -236,7 +240,7 @@ class AuthorizeHandler {
 
   getState (request) {
     const state = request.body.state || request.query.state;
-    const stateExists = state && state.length > 0;
+    const stateExists = isString(state) && state.length > 0;
     const stateIsValid = stateExists
       ? isFormat.vschar(state)
       : this.allowEmptyState;

lib/handlers/token-handler.js

@@ -18,6 +18,8 @@ const UnsupportedGrantTypeError = require('../errors/unsupported-grant-type-erro
 const auth = require('basic-auth');
 const pkce = require('../pkce/pkce');
 const isFormat = require('@node-oauth/formats');
+const { isInTypes, toString } = require('../utils/param-util');
+const isStringOrNumber = isInTypes('string', 'number');
 
 /**
  * Grant types.
@@ -123,11 +125,11 @@ class TokenHandler {
       throw new InvalidRequestError('Missing parameter: `client_secret`');
     }
 
-    if (!isFormat.vschar(credentials.clientId)) {
+    if (!isStringOrNumber(credentials.clientId) || !isFormat.vschar(toString(credentials.clientId))) {
       throw new InvalidRequestError('Invalid parameter: `client_id`');
     }
 
-    if (credentials.clientSecret && !isFormat.vschar(credentials.clientSecret)) {
+    if (credentials.clientSecret && (!isStringOrNumber(credentials.clientSecret) || !isFormat.vschar(toString(credentials.clientSecret)))) {
       throw new InvalidRequestError('Invalid parameter: `client_secret`');
     }
 

lib/utils/param-util.js

@@ -0,0 +1,60 @@
+'use strict';
+
+/**
+ * @module ParamUtil
+ */
+
+/**
+ * Create a function to check, whether a value is one of the given types.
+ * @param types {...string[]}
+ * @return {function(*): boolean}
+ */
+function isInTypes (...types) {
+  return function (value) {
+    return types.includes(typeof value);
+  };
+}
+
+/**
+ * Check if a value is defined (not missing)
+ * @param value
+ * @return {boolean}
+ */
+function isDefined (value) {
+  // TODO: in future versions, consider changing this to `value !== undefined && value !== null && value !== ''`,
+  //   which might be a breaking changes for some users
+  return !!value;
+}
+
+/**
+ * Safely converts a value to a string in the following order:
+ * - If the value is already a string, return it.
+ * - If the value has a `toString` method, call it and return the result.
+ * - If the value is `null` or `undefined`, return an empty string.
+ * - Otherwise, use the global `String` function to convert the value.
+ * @param value {*}
+ * @return {string}
+ */
+function toString(value) {
+  const type = typeof value;
+  if (type === 'string') {
+    return value;
+  }
+
+  if (type === 'undefined' || value === null) {
+    throw new TypeError(`Cannot convert ${value} to a string`);
+  }
+
+  if (type === 'number' || type === 'bigint') {
+    const val = String(value);
+    if (val === 'NaN' || val === 'Infinity' || val === '-Infinity') {
+      throw new TypeError(`Invalid numeric value ${value}, cannot be converted to a string (${val})`);
+    }
+    return val;
+  }
+
+
+  throw new TypeError(`Cannot convert value ${value} of type ${type} to a string`);
+}
+
+module.exports = { isInTypes, isDefined, toString };

lib/utils/scope-util.js

@@ -1,6 +1,8 @@
 const isFormat = require('@node-oauth/formats');
 const InvalidScopeError = require('../errors/invalid-scope-error');
+const { isInTypes } = require('../utils/param-util');
 const whiteSpace = /\s+/g;
+const isString = isInTypes('string');
 
 /**
  * @module ScopeUtil
@@ -22,7 +24,7 @@ function parseScope (requestedScope) {
     return undefined;
   }
 
-  if (typeof requestedScope !== 'string') {
+  if (!isString(requestedScope)) {
     throw new InvalidScopeError('Invalid parameter: `scope`');
   }