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

Fix to vulnerability reported in #1 #3

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,24 @@ npm i oauth2orize-facebook -S

## Usage

Before using this package, you should enable the 'Require App Secret' option in the advanced settings of your Facebook app.
You also must provide the app secret by exporting it

```sh
export FB_APP_SECRET={your Facebook app secret}
```

or by adding FB_APP_SECRET={your Facebook app secret} to your .env file.

Then, you can have fun

```js
var oauth2orize = require('oauth2orize');
var oauth2orizeFacebook = require('oauth2orize-facebook');

var server = oauth2orize.createServer();

server.exchange(oauth2orizeFacebook(function (client, profile, scope, cb) {
server.exchange(oauth2orizeFacebook(['email', 'first_name', 'last_name'], function (client, profile, scope, cb) {
// Get access token from client and Facebook profile information.
var accessToken = 'access token';

Expand Down
109 changes: 61 additions & 48 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,22 @@ var objectAssign = require('object-assign');

var getFacebookProfile = require('./util').getFacebookProfile;

module.exports = function (opts, issue) {
module.exports = function (opts, fields, issue) {
if (Array.isArray(opts)) {
issue = fields;
fields = opts;
opts = null;
}

if (typeof opts === 'function') {
issue = opts;
opts = null;
fields = null;
}

if (!Array.isArray(fields)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @frenzox, I think if you pass opts as function, this condition will be met and the error thrown. I believe the idea was that fields should not be required.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're completely right! I will take a look into that.

throw new Error('You must specify the Facebook profile fields as an array.' +
'Also, make sure you have permission to access these fields');
}

if (typeof issue !== 'function') {
Expand All @@ -28,7 +40,7 @@ module.exports = function (opts, issue) {
return function facebook(req, res, next) {
if (!req.body) {
return next(new Error('Request body not parsed. ' +
'Use bodyParser middleware.'));
'Use bodyParser middleware.'));
}

// The `user` property of `req` holds the authenticated user. In the case
Expand All @@ -40,65 +52,66 @@ module.exports = function (opts, issue) {

if (!token) {
return next(new AuthorizationError(
'Missing Facebook access token!', 'invalid_request'));
'Missing Facebook access token!', 'invalid_request'));
}

getFacebookProfile(token, function (err, profile) {
if (err) {
return next(new AuthorizationError(
'Could not get Facebook profile using provided access token.',
'invalid_request'
));
}

if (scope) {
for (var i = 0, len = separators.length; i < len; i++) {
// Only separates on the first matching separator.
// This allows for a sort of separator "priority"
// (ie, favors spaces then fallback to commas).
var separated = scope.split(separators[i]);

if (separated.length > 1) {
scope = separated;
break;
}
getFacebookProfile(fields, token, function (err, profile) {
if (err) {
console.log(err);
return next(new AuthorizationError(
'Could not get Facebook profile using provided access token.',
'invalid_request'
));
}

if (!Array.isArray(scope)) {
scope = [ scope ];
}
}
if (scope) {
for (var i = 0, len = separators.length; i < len; i++) {
// Only separates on the first matching separator.
// This allows for a sort of separator "priority"
// (ie, favors spaces then fallback to commas).
var separated = scope.split(separators[i]);

if (separated.length > 1) {
scope = separated;
break;
}
}

var issued = function (err, accessToken, refreshToken, params) {
if (err) {
return next(err);
if (!Array.isArray(scope)) {
scope = [ scope ];
}
}

if (!accessToken) {
return next(new AuthorizationError(
'Permissions was not granted.', 'invalid_grant'));
}
var issued = function (err, accessToken, refreshToken, params) {
if (err) {
return next(err);
}

if (!accessToken) {
return next(new AuthorizationError(
'Permissions was not granted.', 'invalid_grant'));
}

var json = { 'access_token': accessToken };
var json = { 'access_token': accessToken };

if (refreshToken) {
json['refresh_token'] = refreshToken;
}
if (refreshToken) {
json['refresh_token'] = refreshToken;
}

if (params) {
objectAssign(json, params);
}
if (params) {
objectAssign(json, params);
}

json['token_type'] = json['token_type'] || 'bearer';
json = JSON.stringify(json);
json['token_type'] = json['token_type'] || 'bearer';
json = JSON.stringify(json);

res.setHeader('Content-Type', 'application/json');
res.setHeader('Cache-Control', 'no-store');
res.setHeader('Pragma', 'no-cache');
res.end(json);
};
res.setHeader('Content-Type', 'application/json');
res.setHeader('Cache-Control', 'no-store');
res.setHeader('Pragma', 'no-cache');
res.end(json);
};

issue(client, profile, scope, issued);
issue(client, profile, scope, issued);
});
};
};
10 changes: 7 additions & 3 deletions lib/util.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
'use strict';

require('dotenv');
var request = require('request');
var crypto = require('crypto');

exports.getFacebookProfile = function (fields, accessToken, cb) {
var appSecretProof = crypto.createHmac('sha256', process.env.FB_APP_SECRET).update(accessToken).digest('hex');
var fieldsString = fields.join();

exports.getFacebookProfile = function (accessToken, cb) {
request({
url: 'https://graph.facebook.com/me?access_token=' + accessToken,
url: 'https://graph.facebook.com/me?fields=' + fieldsString + '&access_token=' + accessToken + '&appsecret_proof=' + appSecretProof,
json: true
},
function (err, res, body) {
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"test": "eslint lib/"
},
"dependencies": {
"dotenv": "^4.0.0",
"object-assign": "^2.0.0",
"request": "^2.53.0"
},
Expand Down