From 0acac9456efb163688dcd23717dbe6888f3ee5c8 Mon Sep 17 00:00:00 2001 From: Matt Date: Sun, 13 Apr 2014 21:50:05 -0500 Subject: [PATCH 1/2] adding a clientType option for ROPC flows to allow for public client types --- README.md | 5 ++- lib/cc/grantToken.js | 2 +- lib/common/makeSetup.js | 24 +++++++++++---- lib/common/validateGrantTokenRequest.js | 4 +-- lib/ropc/grantToken.js | 41 +++++++++++++++---------- lib/ropc/index.js | 1 + 6 files changed, 50 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index eefc0e1..1a17731 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,7 @@ application. Checks that the API client is authorized to use your API, and has the correct secret. It should call back with `true` or `false` depending on the result of the check. It can also call back with an error if there was some internal server -error while doing the check. +error while doing the check. This hook is not required if `clientType: "public"` is passed as an option. #### `grantUserToken(username, password, cb)` @@ -111,6 +111,9 @@ The `hooks` hash is the only required option, but the following are also availab * `tokenExpirationTime`: the value returned for the `expires_in` component of the response from the token endpoint. Note that this is *only* the value reported; you are responsible for keeping track of token expiration yourself and calling back with `false` from `authenticateToken` when the token expires. Defaults to `Infinity`. +* `clientType`: valid values are `"confidential"` (default) or `"public"`. Client credentials are not authenticated + for public clients. This option only applies to the [Resource Owner Password Credentials][ropc] flow. OAuth2 only + allows for confidential clients for the [Client Credentials][cc] flow. ## What Does That Look Like? diff --git a/lib/cc/grantToken.js b/lib/cc/grantToken.js index 7c9a7fd..2a672e0 100644 --- a/lib/cc/grantToken.js +++ b/lib/cc/grantToken.js @@ -6,7 +6,7 @@ var validateGrantTokenRequest = require("../common/validateGrantTokenRequest"); var makeOAuthError = require("../common/makeOAuthError"); module.exports = function grantToken(req, res, next, options) { - if (!validateGrantTokenRequest("client_credentials", req, next)) { + if (!validateGrantTokenRequest("client_credentials", "confidential", req, next)) { return; } diff --git a/lib/common/makeSetup.js b/lib/common/makeSetup.js index 156aef1..3ad1ade 100644 --- a/lib/common/makeSetup.js +++ b/lib/common/makeSetup.js @@ -10,18 +10,30 @@ module.exports = function makeSetup(grantTypes, reqPropertyName, requiredHooks, if (typeof options.hooks !== "object" || options.hooks === null) { throw new Error("Must supply hooks."); } + + // if the client type is public, then + if (options.clientType && options.clientType === "public") { + requiredHooks = requiredHooks.filter(function (hookName) { + return hookName !== "validateClient"; + }); + } + + options = _.defaults(options, { + tokenEndpoint: "/token", + wwwAuthenticateRealm: "Who goes there?", + tokenExpirationTime: Infinity, + clientType: "confidential" + }); + requiredHooks.forEach(function (hookName) { + if (options.clientType === "public" && hookName === "validateClient") { + return; + } if (typeof options.hooks[hookName] !== "function") { throw new Error("Must supply " + hookName + " hook."); } }); - options = _.defaults(options, { - tokenEndpoint: "/token", - wwwAuthenticateRealm: "Who goes there?", - tokenExpirationTime: Infinity - }); - // Allow `tokenExpirationTime: Infinity` (like above), but translate it into `undefined` so that // `JSON.stringify` omits it entirely when we write out the response as // `JSON.stringify({ expires_in: tokenExpirationTime, ... })`. diff --git a/lib/common/validateGrantTokenRequest.js b/lib/common/validateGrantTokenRequest.js index 339e6e3..ddf3a13 100644 --- a/lib/common/validateGrantTokenRequest.js +++ b/lib/common/validateGrantTokenRequest.js @@ -3,7 +3,7 @@ var _ = require("underscore"); var makeOAuthError = require("./makeOAuthError"); -module.exports = function validateGrantTokenRequest(grantType, req, next) { +module.exports = function validateGrantTokenRequest(grantType, clientType, req, next) { function sendBadRequestError(type, description) { next(makeOAuthError("BadRequest", type, description)); } @@ -24,7 +24,7 @@ module.exports = function validateGrantTokenRequest(grantType, req, next) { return false; } - if (!req.authorization || !req.authorization.basic) { + if (clientType === "confidential" && (!req.authorization || !req.authorization.basic)) { sendBadRequestError("invalid_request", "Must include a basic access authentication header."); return false; } diff --git a/lib/ropc/grantToken.js b/lib/ropc/grantToken.js index 531837b..ad3ac3b 100644 --- a/lib/ropc/grantToken.js +++ b/lib/ropc/grantToken.js @@ -12,7 +12,7 @@ module.exports = function grantToken(req, res, next, options) { } - if (!validateGrantTokenRequest("password", req, next)) { + if (!validateGrantTokenRequest("password", options.clientType, req, next)) { return; } @@ -27,33 +27,40 @@ module.exports = function grantToken(req, res, next, options) { return next(makeOAuthError("BadRequest", "invalid_request", "Must specify password field.")); } - var clientId = req.authorization.basic.username; - var clientSecret = req.authorization.basic.password; - - options.hooks.validateClient(clientId, clientSecret, function (error, result) { + function grantUserTokenCallback(error, token) { if (error) { return next(error); } - if (!result) { - return sendUnauthorizedError("invalid_client", "Client ID and secret did not validate."); + if (!token) { + return sendUnauthorizedError("invalid_grant", "Username and password did not authenticate."); } - options.hooks.grantUserToken(username, password, function (error, token) { + res.send({ + access_token: token, + token_type: "Bearer", + expires_in: options.tokenExpirationTime + }); + next(); + } + + if (options.clientType === "confidential") { + var clientId = req.authorization.basic.username; + var clientSecret = req.authorization.basic.password; + + options.hooks.validateClient(clientId, clientSecret, function (error, result) { if (error) { return next(error); } - if (!token) { - return sendUnauthorizedError("invalid_grant", "Username and password did not authenticate."); + if (!result) { + return sendUnauthorizedError("invalid_client", "Client ID and secret did not validate."); } - res.send({ - access_token: token, - token_type: "Bearer", - expires_in: options.tokenExpirationTime - }); - next(); + options.hooks.grantUserToken(username, password, grantUserTokenCallback); }); - }); + } + else { + options.hooks.grantUserToken(username, password, grantUserTokenCallback); + } }; diff --git a/lib/ropc/index.js b/lib/ropc/index.js index 0fd8bd0..b7e3149 100644 --- a/lib/ropc/index.js +++ b/lib/ropc/index.js @@ -5,6 +5,7 @@ var grantToken = require("./grantToken"); var grantTypes = "password"; var reqPropertyName = "username"; +// validateClient hook is not required if {clientType: "public"} is passed as an option var requiredHooks = ["validateClient", "grantUserToken", "authenticateToken"]; module.exports = makeSetup(grantTypes, reqPropertyName, requiredHooks, grantToken); From 2df00c24f0901c991e43018545001401a57b4f30 Mon Sep 17 00:00:00 2001 From: Matt Date: Sun, 13 Apr 2014 22:34:21 -0500 Subject: [PATCH 2/2] more elegant handling of requiredHooks --- lib/cc/index.js | 4 +++- lib/common/makeSetup.js | 9 +-------- lib/ropc/index.js | 8 ++++++-- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/cc/index.js b/lib/cc/index.js index b1ff8a3..39ba356 100644 --- a/lib/cc/index.js +++ b/lib/cc/index.js @@ -5,6 +5,8 @@ var grantToken = require("./grantToken"); var grantTypes = "client_credentials"; var reqPropertyName = "clientId"; -var requiredHooks = ["grantClientToken", "authenticateToken"]; +var requiredHooks = function (options) { + return ["grantClientToken", "authenticateToken"]; +} module.exports = makeSetup(grantTypes, reqPropertyName, requiredHooks, grantToken); diff --git a/lib/common/makeSetup.js b/lib/common/makeSetup.js index 3ad1ade..29eed33 100644 --- a/lib/common/makeSetup.js +++ b/lib/common/makeSetup.js @@ -10,13 +10,6 @@ module.exports = function makeSetup(grantTypes, reqPropertyName, requiredHooks, if (typeof options.hooks !== "object" || options.hooks === null) { throw new Error("Must supply hooks."); } - - // if the client type is public, then - if (options.clientType && options.clientType === "public") { - requiredHooks = requiredHooks.filter(function (hookName) { - return hookName !== "validateClient"; - }); - } options = _.defaults(options, { tokenEndpoint: "/token", @@ -25,7 +18,7 @@ module.exports = function makeSetup(grantTypes, reqPropertyName, requiredHooks, clientType: "confidential" }); - requiredHooks.forEach(function (hookName) { + requiredHooks(options).forEach(function (hookName) { if (options.clientType === "public" && hookName === "validateClient") { return; } diff --git a/lib/ropc/index.js b/lib/ropc/index.js index b7e3149..78030e0 100644 --- a/lib/ropc/index.js +++ b/lib/ropc/index.js @@ -5,7 +5,11 @@ var grantToken = require("./grantToken"); var grantTypes = "password"; var reqPropertyName = "username"; -// validateClient hook is not required if {clientType: "public"} is passed as an option -var requiredHooks = ["validateClient", "grantUserToken", "authenticateToken"]; +var requiredHooks = function (options) { + if (options.clientType === "public") { + return ["grantUserToken", "authenticateToken"]; + } + return ["validateClient", "grantUserToken", "authenticateToken"]; +} module.exports = makeSetup(grantTypes, reqPropertyName, requiredHooks, grantToken);