Skip to content
This repository was archived by the owner on Dec 14, 2023. It is now read-only.

Commit 19f7ed0

Browse files
authored
Enforce usage of specific callback uri instead of allowing any (#193)
1 parent 76387a2 commit 19f7ed0

File tree

3 files changed

+67
-28
lines changed

3 files changed

+67
-28
lines changed

config/config.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,10 @@ module.exports = function () {
6565
},
6666
oauth2: {
6767
clients: {
68-
coderdojoadultforums: process.env.CODERDOJO_ADULT_FORUMS_SECRET || 'ilikecode',
69-
coderdojoyouthforums: process.env.CODERDOJO_YOUTH_FORUMS_SECRET || 'ilikecode'
68+
coderdojoadultforums: { code: process.env.CODERDOJO_ADULT_FORUMS_SECRET || 'ilikecode',
69+
baseUrl: process.env.ADULT_FORUM + '/auth/CoderDojo/callback'},
70+
coderdojoyouthforums: { code: process.env.CODERDOJO_YOUTH_FORUMS_SECRET || 'ilikecode',
71+
baseUrl: process.env.YOUTH_FORUM + '/auth/CoderDojo/callback'}
7072
}
7173
},
7274
nodebb: {

oauth2.js

Lines changed: 62 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,24 @@ module.exports = function (options) {
1313
seneca.add({role: plugin, cmd: 'token'}, cmd_token);
1414
seneca.add({role: plugin, cmd: 'profile'}, cmd_profile);
1515

16-
// Note: currently got from the config, this may be moved to the database in the near future..
16+
// NOTE: currently got from the config, this may be moved to the database in the near future..
1717
function _verifyClientId (clientId, cb) {
18-
setImmediate(function () {
19-
if (!options.clients[clientId]) return cb('Invalid client_id: ' + clientId);
20-
return cb();
18+
if (!options.clients[clientId]) return cb('Invalid client_id: ' + clientId);
19+
return cb();
20+
}
21+
22+
function _verifyCallbackUrl (callback, cb) {
23+
var allowed = _.map(options.clients, 'baseUrl');
24+
var valid = _.some(allowed, function (url) {
25+
// We can't use _.includes as it match for partials which is insecure
26+
return _.isEqual(url, callback);
2127
});
28+
if (!valid) return cb('Invalid callback_url: ' + callback);
29+
return cb();
2230
}
2331

2432
function _getAccessCodeForUser (user, cb) {
33+
// NOTE : maybe use load for security sake ??????!!
2534
seneca.make$(OAUTH2_ENTITY).list$({userid: user.id}, function (err, auths) {
2635
if (err) return cb(err);
2736
if (auths.length > 0) return cb(null, auths[0].code);
@@ -63,18 +72,19 @@ module.exports = function (options) {
6372
}
6473

6574
function getUser (auths, done) {
75+
// Why.
6676
var userEntity = seneca.make('sys/user');
6777
userEntity.load$(auths[0].userid, done);
6878
}
6979

7080
function checkPermissions (user, done) {
71-
seneca.act({role: 'cd-profiles', cmd: 'list', query: {userId: user.id}}, function (err, profiles) {
81+
seneca.act({role: 'cd-profiles', cmd: 'load_user_profile', userId: user.id}, function (err, userProfile) {
7282
if (err) return done(err);
73-
var userProfile = profiles[0];
7483
user.profileId = userProfile.id;
7584
if (userProfile.userType === 'champion') user.isChampion = true;
7685
if (userProfile.userType === 'attendee-o13') user.isYouthOver13 = true;
7786
if (userProfile.userType === 'mentor') user.isMentor = true;
87+
if (userProfile.userType === 'parent-guardian') user.isParent = true;
7888

7989
seneca.act({role: 'cd-dojos', cmd: 'load_usersdojos', query: {userId: user.id}}, function (err, usersDojos) {
8090
if (err) return done(err);
@@ -87,9 +97,11 @@ module.exports = function (options) {
8797
var mentorTypeFound = _.find(usersDojos, function (userDojo) {
8898
return _.contains(userDojo.userTypes, 'mentor');
8999
});
100+
var verifyFound = _.any(usersDojos, 'backgroundChecked');
90101
if (championTypeFound) user.isChampion = true;
91102
if (youthOver13TypeFound) user.isYouthOver13 = true;
92103
if (mentorTypeFound) user.isMentor = true;
104+
if (verifyFound) user.isVerified = true;
93105
return done(null, user);
94106
});
95107
});
@@ -100,31 +112,54 @@ module.exports = function (options) {
100112
if (args.response_type !== 'code') {
101113
return done(null, {error: 'Only authorization code auth supported!'});
102114
}
103-
104-
_verifyClientId(args.client_id, function (err) {
105-
if (err) return done(null, {error: err});
106-
107-
if (!args.user) {
108-
return done(null, {
109-
http$: {
110-
redirect: '/login?redirect=' + args['redirect_uri']
115+
async.waterfall([
116+
function (waterfallCb) {
117+
if (args.redirect_uri) {
118+
_verifyCallbackUrl(args.redirect_uri, function (err) {
119+
if (err) {
120+
return done(null, {
121+
error: err,
122+
http$: {
123+
status: 403
124+
}
125+
});
126+
} else {
127+
waterfallCb();
128+
}
129+
});
130+
} else {
131+
return done(null, {error: 'Missing callback_url', http$: {status: 422}});
132+
}
133+
},
134+
function (waterfallCb) {
135+
_verifyClientId(args.client_id, function (err) {
136+
if (err) return done(null, {error: err});
137+
if (!args.user) {
138+
return done(null, {
139+
http$: {
140+
redirect: '/login?redirect=' + args.redirect_uri
141+
}
142+
});
143+
} else {
144+
waterfallCb();
111145
}
112146
});
113-
}
114-
115-
_getAccessCodeForUser(args.user, function (err, code) {
116-
if (err) return done(null, {error: err, http$: {status: 500}});
117-
118-
done(null, {
119-
http$: {
120-
redirect: args['redirect_uri'] + '?code=' + code
121-
}
147+
},
148+
function (waterfallCb) {
149+
_getAccessCodeForUser(args.user, function (err, code) {
150+
if (err) return done(null, {error: err, http$: {status: 500}});
151+
done(null, {
152+
http$: {
153+
redirect: args.redirect_uri + '?code=' + code
154+
}
155+
});
122156
});
123-
});
124-
});
157+
}
158+
]);
125159
}
126160

127161
function cmd_token (args, done) {
162+
// TODO : check if code exists maybe ?
128163
_getAccessTokenForAccessCode(args.code, function (err, access_token) {
129164
if (err) return done(null, {error: err, http$: {status: 500}});
130165

@@ -146,6 +181,8 @@ module.exports = function (options) {
146181
isChampion: user.isChampion,
147182
isYouthOver13: user.isYouthOver13,
148183
isMentor: user.isMentor,
184+
isParent: user.isParent,
185+
isVerified: user.isVerified,
149186
profileId: user.profileId
150187
};
151188
return done(null, profile);

service.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ require('./migrate-psql-db.js')(function (err) {
2727
seneca.use(require('./email-notifications.js'));
2828
seneca.use(require('./agreements.js'));
2929
seneca.use(require('./profiles.js'), { postgresql: config['postgresql-store'] });
30-
seneca.use(require('./oauth2.js'), config.oauth2);
30+
seneca.use(require('./oauth2.js'), {clients: config.oauth2.clients});
3131
seneca.use('user');
3232
seneca.use('auth');
3333
seneca.use(require('./users.js'),

0 commit comments

Comments
 (0)