From f1ac2b4f3494bcd27d1a3f9ff298e29a0bb6ef2b Mon Sep 17 00:00:00 2001 From: elixic Date: Wed, 11 Mar 2015 08:48:51 -0700 Subject: [PATCH 01/11] adding passport libs. --- package.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 95ae046..0b6d1e7 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,10 @@ "bluebird": "^2.9.13", "express": "^4.12.0", "github": "^0.2.3", - "json-mask": "0.3.4" + "json-mask": "0.3.4", + "method-override": "^2.3.1", + "passport": "^0.2.1", + "passport-github": "^0.1.5" }, "devDependencies": { "babelify": "^5.0.4", From a39d47bcca83e9dc06a04bcb9b0a753b9eb7feeb Mon Sep 17 00:00:00 2001 From: elixic Date: Wed, 11 Mar 2015 19:38:59 -0700 Subject: [PATCH 02/11] adding the auth routes to the main app and a mixin for the sub apps. --- app/server/apps/repos/index.js | 7 ++++- app/server/apps/users/index.js | 7 ++++- app/server/authicator.js | 35 +++++++++++++++++++++++++ app/server/main.js | 37 ++++++++++++++++++++++++--- app/server/middleware/authenticate.js | 11 ++++++++ 5 files changed, 91 insertions(+), 6 deletions(-) create mode 100644 app/server/authicator.js create mode 100644 app/server/middleware/authenticate.js diff --git a/app/server/apps/repos/index.js b/app/server/apps/repos/index.js index f17ba1c..0294ddb 100644 --- a/app/server/apps/repos/index.js +++ b/app/server/apps/repos/index.js @@ -4,13 +4,15 @@ */ var repos = require('../../middleware/repos'), permissions = require('../../middleware/permissions'), - send = require('../../middleware/send'); + send = require('../../middleware/send'), + auth = require('../../middleware/authenticate'); module.exports = { list: { method: 'GET', path: '/repos', middleware: [ + auth.isAuthenticated, repos.listRepos, repos.listReposPermission, repos.listReposLinks, @@ -21,6 +23,7 @@ module.exports = { method: 'GET', path: '/repos/:id', middleware: [ + auth.isAuthenticated, repos.readRepo, send.json ] @@ -29,6 +32,7 @@ module.exports = { method: 'PUT', path: '/repos/:id/users/:username/permissions/:permission', middleware: [ + auth.isAuthenticated, permissions.editRepoPermissionForUser, send.noContent ] @@ -37,6 +41,7 @@ module.exports = { method: 'DELETE', path: '/repos/:id/users/:username', middleware: [ + auth.isAuthenticated, permissions.removeRepoPermissionForUser, send.noContent ] diff --git a/app/server/apps/users/index.js b/app/server/apps/users/index.js index 26beed5..736f4f1 100644 --- a/app/server/apps/users/index.js +++ b/app/server/apps/users/index.js @@ -4,13 +4,15 @@ */ var users = require('../../middleware/users'), permissions = require('../../middleware/permissions'), - send = require('../../middleware/send'); + send = require('../../middleware/send'), + auth = require('../../authenticate'); module.exports = { list: { method: 'GET', path: '/users', middleware: [ + auth.isAuthenticated, users.listUsers, users.listUsersPermission, users.listUsersLinks, @@ -21,6 +23,7 @@ module.exports = { method: 'GET', path: '/users/:username', middleware: [ + auth.isAuthenticated, users.readUser, send.json ] @@ -29,6 +32,7 @@ module.exports = { method: 'PUT', path: '/users/:username/repos/:id/permissions/:permission', middleware: [ + auth.isAuthenticated, permissions.editRepoPermissionForUser, send.noContent ] @@ -37,6 +41,7 @@ module.exports = { method: 'DELETE', path: '/users/:username/repos/:id', middleware: [ + auth.isAuthenticated, permissions.removeRepoPermissionForUser, send.noContent ] diff --git a/app/server/authicator.js b/app/server/authicator.js new file mode 100644 index 0000000..cdff5fe --- /dev/null +++ b/app/server/authicator.js @@ -0,0 +1,35 @@ +'use strict'; +var passport = require('passport'), + GitHubStrategy = require('passport-github').Strategy; + +module.exports = { + setup: function(app, config) { + passport.use(new GitHubStrategy({ + clientID: config.github.clientID, + clientSecret: config.github.clientSecret, + callbackURL: config.server.hostName + ":" + config.app.port + config.github.authCallbackRoute + }, + function(accessToken, refreshToken, profile, done) { + done(null, { displayName: profile.displayName, id: profile.id, token: accessToken } ); + })); + + passport.serializeUser(function(user, done) { + done(null, user); + }); + + passport.deserializeUser(function(user, done) { + done(null, user); + }); + + app.use(passport.initialize()); + app.use(passport.session()); + + app.get(config.github.authRoute, passport.authenticate('github')); + app.get(config.github.authCallbackRoute, passport.authenticate('github', { failureRedirect: config.github.failureRedirect, session: true }), + function(req, res) { + // TODO: redirect to initial request location... + res.redirect('/'); + } + ); + } +} diff --git a/app/server/main.js b/app/server/main.js index 6779233..8bbb487 100644 --- a/app/server/main.js +++ b/app/server/main.js @@ -4,24 +4,53 @@ require('babel/polyfill'); exports.start = () => { + // HACKING IN CONFIG OBJECT HERE + var config = { + server: { + port: '3000', + api_prefix: '/api/v1', + hostname: process.env.HOSTNAME + }, + github: { + clientID: process.env.GITHUB_CLIENTID, + clientSecret: process.env.GITHUB_CLIENT_KEY, + authRoute: '/auth/github', + authCallbackRoute: '/auth/github/callback', + failureCallback: '/signup' + }, + session: { + secret: 'keyboard cat', + resave: false, + saveUninitialized: true, + cookie: { + secure: false + } + } + } + var express = require('express'), http = require('http'), path = require('path'), + session = require('express-session'), + authenticator = require('./authenticator'), app = express(), discovery = require('./discovery'); - app.set('port', 3000); + app.use(session(config.session)); + + authenticator.setup(app, config); + app.set('port', config.server.port); app.use(express.static(path.resolve(__dirname, '../../app/client/build'))); discovery.find(path.join(__dirname, './apps')) .then((apps) => { apps.forEach((subapp) => { - app.use('/api/v1', subapp); + app.use(config.server.api_prefix, subapp); }); - http.createServer(app).listen(app.get('port'), () => { + http.createServer(app).listen(config.server.port, () => { console.log('-----------------------------------------------------------------------'); - console.log('Express server listening on port ' + app.get('port')); + console.log('Express server listening on port ' + config.server.port); }); }) .catch((err) => { diff --git a/app/server/middleware/authenticate.js b/app/server/middleware/authenticate.js new file mode 100644 index 0000000..4d00673 --- /dev/null +++ b/app/server/middleware/authenticate.js @@ -0,0 +1,11 @@ +'use strict'; + +module.exports = { + isAuthenticated: function(req, res, next) { + if (req.isAuthenticated()) { + next(); + } + + res.redirect('/auth/github'); + } +} \ No newline at end of file From b691e94d9c71c23e9b271e342c94faece1323fb5 Mon Sep 17 00:00:00 2001 From: elixic Date: Wed, 11 Mar 2015 20:34:17 -0700 Subject: [PATCH 03/11] a few issues with file names and left session out of packager.json accidentally. --- .../{authicator.js => authenticator.js} | 23 +++++++++++++------ app/server/main.js | 12 ++++------ app/server/middleware/authenticate.js | 11 ++++++--- package.json | 1 + 4 files changed, 30 insertions(+), 17 deletions(-) rename app/server/{authicator.js => authenticator.js} (63%) diff --git a/app/server/authicator.js b/app/server/authenticator.js similarity index 63% rename from app/server/authicator.js rename to app/server/authenticator.js index cdff5fe..2ad39e9 100644 --- a/app/server/authicator.js +++ b/app/server/authenticator.js @@ -1,23 +1,32 @@ 'use strict'; + + var passport = require('passport'), GitHubStrategy = require('passport-github').Strategy; module.exports = { - setup: function(app, config) { + /** + * Sets up passport-github for authentication by creating routes on the app for retrieving oauth tokens from + * github. + * + * @param app the app that will use the githug authentication + * @param config configuration from the app using the authenticator + */ + setup: function (app, config) { passport.use(new GitHubStrategy({ clientID: config.github.clientID, clientSecret: config.github.clientSecret, callbackURL: config.server.hostName + ":" + config.app.port + config.github.authCallbackRoute }, - function(accessToken, refreshToken, profile, done) { - done(null, { displayName: profile.displayName, id: profile.id, token: accessToken } ); + function (accessToken, refreshToken, profile, done) { + done(null, { displayName: profile.displayName, id: profile.id, token: accessToken }); })); - passport.serializeUser(function(user, done) { + passport.serializeUser(function (user, done) { done(null, user); }); - passport.deserializeUser(function(user, done) { + passport.deserializeUser(function (user, done) { done(null, user); }); @@ -26,10 +35,10 @@ module.exports = { app.get(config.github.authRoute, passport.authenticate('github')); app.get(config.github.authCallbackRoute, passport.authenticate('github', { failureRedirect: config.github.failureRedirect, session: true }), - function(req, res) { + function (req, res) { // TODO: redirect to initial request location... res.redirect('/'); } ); } -} +}; diff --git a/app/server/main.js b/app/server/main.js index 8bbb487..4a820de 100644 --- a/app/server/main.js +++ b/app/server/main.js @@ -25,16 +25,14 @@ exports.start = () => { cookie: { secure: false } - } - } - - var express = require('express'), + }}, + express = require('express'), http = require('http'), path = require('path'), session = require('express-session'), - authenticator = require('./authenticator'), - app = express(), - discovery = require('./discovery'); + authenticator = require('./authenticator.js'), + discovery = require('./discovery'), + app = express(); app.use(session(config.session)); diff --git a/app/server/middleware/authenticate.js b/app/server/middleware/authenticate.js index 4d00673..3015121 100644 --- a/app/server/middleware/authenticate.js +++ b/app/server/middleware/authenticate.js @@ -1,11 +1,16 @@ 'use strict'; module.exports = { - isAuthenticated: function(req, res, next) { + isAuthenticated: function (req, res, next) { + console.log("isAuthenticated"); if (req.isAuthenticated()) { + console.log("Authenticated"); next(); } - res.redirect('/auth/github'); + console.log("redirecting to authentication"); + + // todo : use config object to create this url... + res.redirect('/api/v1/auth/github'); } -} \ No newline at end of file +}; diff --git a/package.json b/package.json index 0b6d1e7..b3ee3c6 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,7 @@ "babel": "4.7.1", "bluebird": "^2.9.13", "express": "^4.12.0", + "express-session": "^1.10.3", "github": "^0.2.3", "json-mask": "0.3.4", "method-override": "^2.3.1", From 0507815eadd6d611a07d61b33ec8750072554e25 Mon Sep 17 00:00:00 2001 From: elixic Date: Wed, 11 Mar 2015 20:39:02 -0700 Subject: [PATCH 04/11] renamed authenticator to passport and modified main to use require that file. --- app/server/main.js | 4 ++-- app/server/{authenticator.js => passport.js} | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename app/server/{authenticator.js => passport.js} (100%) diff --git a/app/server/main.js b/app/server/main.js index 4a820de..54525db 100644 --- a/app/server/main.js +++ b/app/server/main.js @@ -30,13 +30,13 @@ exports.start = () => { http = require('http'), path = require('path'), session = require('express-session'), - authenticator = require('./authenticator.js'), discovery = require('./discovery'), + passport = require('./passport'), app = express(); app.use(session(config.session)); - authenticator.setup(app, config); + passport.setup(app, config); app.set('port', config.server.port); app.use(express.static(path.resolve(__dirname, '../../app/client/build'))); diff --git a/app/server/authenticator.js b/app/server/passport.js similarity index 100% rename from app/server/authenticator.js rename to app/server/passport.js From e89791880ddcba63d5f87c9a702d9ebb7ee0535a Mon Sep 17 00:00:00 2001 From: elixic Date: Thu, 12 Mar 2015 10:04:36 -0700 Subject: [PATCH 05/11] changing response for bad authentication to be a 401 and fixing a require path issue. --- app/server/apps/users/index.js | 2 +- app/server/middleware/authenticate.js | 10 +++------- app/server/passport.js | 6 +++--- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/app/server/apps/users/index.js b/app/server/apps/users/index.js index 736f4f1..943a665 100644 --- a/app/server/apps/users/index.js +++ b/app/server/apps/users/index.js @@ -5,7 +5,7 @@ var users = require('../../middleware/users'), permissions = require('../../middleware/permissions'), send = require('../../middleware/send'), - auth = require('../../authenticate'); + auth = require('../../middleware/authenticate'); module.exports = { list: { diff --git a/app/server/middleware/authenticate.js b/app/server/middleware/authenticate.js index 3015121..ac06ef5 100644 --- a/app/server/middleware/authenticate.js +++ b/app/server/middleware/authenticate.js @@ -1,16 +1,12 @@ 'use strict'; module.exports = { - isAuthenticated: function (req, res, next) { - console.log("isAuthenticated"); + isAuthenticated (req, res, next) { if (req.isAuthenticated()) { - console.log("Authenticated"); next(); } - console.log("redirecting to authentication"); - - // todo : use config object to create this url... - res.redirect('/api/v1/auth/github'); + console.log("Not authenticated, sending 401"); + res.send(401); } }; diff --git a/app/server/passport.js b/app/server/passport.js index 2ad39e9..ac856cc 100644 --- a/app/server/passport.js +++ b/app/server/passport.js @@ -16,7 +16,7 @@ module.exports = { passport.use(new GitHubStrategy({ clientID: config.github.clientID, clientSecret: config.github.clientSecret, - callbackURL: config.server.hostName + ":" + config.app.port + config.github.authCallbackRoute + callbackURL: config.server.hostName + ":" + config.server.port + config.server.api_prefix + config.github.authCallbackRoute }, function (accessToken, refreshToken, profile, done) { done(null, { displayName: profile.displayName, id: profile.id, token: accessToken }); @@ -33,8 +33,8 @@ module.exports = { app.use(passport.initialize()); app.use(passport.session()); - app.get(config.github.authRoute, passport.authenticate('github')); - app.get(config.github.authCallbackRoute, passport.authenticate('github', { failureRedirect: config.github.failureRedirect, session: true }), + app.get(config.server.api_prefix + config.github.authRoute, passport.authenticate('github')); + app.get(config.server.api_prefix + config.github.authCallbackRoute, passport.authenticate('github', { failureRedirect: config.github.failureRedirect, session: true }), function (req, res) { // TODO: redirect to initial request location... res.redirect('/'); From 6f19f21974c2b3d4c96765660bf0376fca09ecfc Mon Sep 17 00:00:00 2001 From: elixic Date: Thu, 12 Mar 2015 16:14:31 -0700 Subject: [PATCH 06/11] adding login page and making sure redirects and config values are proper. fixed issues with an error occuring even if the user was authenticated. --- app/client/src/modules/app/index.js | 15 ++++++++++++++- app/client/src/modules/app/main/login.html | 1 + app/client/src/modules/app/main/main.js | 4 ++++ app/server/main.js | 4 ++-- app/server/middleware/authenticate.js | 11 +++++++---- app/server/passport.js | 19 ++++++++++++++++--- 6 files changed, 44 insertions(+), 10 deletions(-) create mode 100644 app/client/src/modules/app/main/login.html diff --git a/app/client/src/modules/app/index.js b/app/client/src/modules/app/index.js index d6c33dc..0d7c110 100755 --- a/app/client/src/modules/app/index.js +++ b/app/client/src/modules/app/index.js @@ -20,6 +20,19 @@ module.exports = //load other app modules here, e.g.: //require('./account').name ]) - .config(function ($urlRouterProvider) { + .config(function ($urlRouterProvider, $httpProvider) { $urlRouterProvider.otherwise('/'); + $httpProvider.interceptors.push('authInterceptor'); + }).factory('authInterceptor', function ($rootScope, $q, $location) { + return { + responseError(response) { + if (response.status === 401) { + console.log('redirecting to login'); + $location.path('/login'); + //$cookieStore.remove('token'); + } + + return $q.reject(response); + } + }; }); diff --git a/app/client/src/modules/app/main/login.html b/app/client/src/modules/app/main/login.html new file mode 100644 index 0000000..e1d8a19 --- /dev/null +++ b/app/client/src/modules/app/main/login.html @@ -0,0 +1 @@ +Login with GitHub \ No newline at end of file diff --git a/app/client/src/modules/app/main/main.js b/app/client/src/modules/app/main/main.js index 27281b6..459f635 100644 --- a/app/client/src/modules/app/main/main.js +++ b/app/client/src/modules/app/main/main.js @@ -8,6 +8,10 @@ module.exports = url: '/', templateUrl: 'app/main/main.html', controller: 'mainController as ctrl' + }) + .state('login', { + url: '/login', + templateUrl: 'app/main/login.html' }); }) .controller('mainController', function () { diff --git a/app/server/main.js b/app/server/main.js index 54525db..f578bc4 100644 --- a/app/server/main.js +++ b/app/server/main.js @@ -9,14 +9,14 @@ exports.start = () => { server: { port: '3000', api_prefix: '/api/v1', - hostname: process.env.HOSTNAME + hostname: 'localhost' }, github: { clientID: process.env.GITHUB_CLIENTID, clientSecret: process.env.GITHUB_CLIENT_KEY, authRoute: '/auth/github', authCallbackRoute: '/auth/github/callback', - failureCallback: '/signup' + failureCallback: '/auth/failure' }, session: { secret: 'keyboard cat', diff --git a/app/server/middleware/authenticate.js b/app/server/middleware/authenticate.js index ac06ef5..6d22638 100644 --- a/app/server/middleware/authenticate.js +++ b/app/server/middleware/authenticate.js @@ -2,11 +2,14 @@ module.exports = { isAuthenticated (req, res, next) { - if (req.isAuthenticated()) { + var authenticated = req.isAuthenticated(); + console.log("authenticated? " + authenticated); + + if (authenticated) { + console.log("next...."); next(); + } else { + res.send(401); } - - console.log("Not authenticated, sending 401"); - res.send(401); } }; diff --git a/app/server/passport.js b/app/server/passport.js index ac856cc..cf67f3a 100644 --- a/app/server/passport.js +++ b/app/server/passport.js @@ -16,29 +16,42 @@ module.exports = { passport.use(new GitHubStrategy({ clientID: config.github.clientID, clientSecret: config.github.clientSecret, - callbackURL: config.server.hostName + ":" + config.server.port + config.server.api_prefix + config.github.authCallbackRoute + callbackURL: "http://" + config.server.hostname + ":" + config.server.port + config.github.authCallbackRoute }, function (accessToken, refreshToken, profile, done) { + console.log("Authentication success!"); done(null, { displayName: profile.displayName, id: profile.id, token: accessToken }); })); passport.serializeUser(function (user, done) { + console.log("serialize user"); + console.log(user); done(null, user); }); passport.deserializeUser(function (user, done) { + console.log("deserialize user"); + console.log(user); done(null, user); }); app.use(passport.initialize()); app.use(passport.session()); - app.get(config.server.api_prefix + config.github.authRoute, passport.authenticate('github')); - app.get(config.server.api_prefix + config.github.authCallbackRoute, passport.authenticate('github', { failureRedirect: config.github.failureRedirect, session: true }), + app.get(config.github.authRoute, passport.authenticate('github')); + app.get(config.github.authCallbackRoute, passport.authenticate('github', + { failureRedirect: config.github.failureRedirect, session: true }), function (req, res) { + console.log("authenticated, redirect to /"); + var authenticated = req.isAuthenticated(); + console.log("authenticated? " + authenticated); // TODO: redirect to initial request location... res.redirect('/'); } ); + app.get(config.github.failureCallback, function (req, res, next) { + console.log("Error authenticating with github..."); + res.send(401); + }); } }; From d653ac9b0b951a7c1428fac1721cd9426ac40b82 Mon Sep 17 00:00:00 2001 From: elixic Date: Fri, 13 Mar 2015 16:02:42 -0700 Subject: [PATCH 07/11] adding membershipt check --- app/server/components/repositories/users.js | 8 ++++++++ app/server/components/repositories/util/users.js | 7 +++++++ app/server/components/services/github.js | 1 + app/server/middleware/authenticate.js | 4 +++- app/server/passport.js | 11 ++++++++++- 5 files changed, 29 insertions(+), 2 deletions(-) diff --git a/app/server/components/repositories/users.js b/app/server/components/repositories/users.js index b051afd..8158c07 100644 --- a/app/server/components/repositories/users.js +++ b/app/server/components/repositories/users.js @@ -25,5 +25,13 @@ module.exports = { }); return users; }); + }, + + isMember (username) { + console.log("isMember(" + username + ")"); + return userUtil.isMember(username).then(function (data){ + console.log(data); + return "204 No Content" === data.meta.success; + }); } }; diff --git a/app/server/components/repositories/util/users.js b/app/server/components/repositories/util/users.js index bcc084d..8a4441d 100644 --- a/app/server/components/repositories/util/users.js +++ b/app/server/components/repositories/util/users.js @@ -28,5 +28,12 @@ module.exports = { org: github.config.org, per_page: 100 }); + }, + + isMember(username) { + return github.isMember({ + org: githug.config.org, + username: username + }); } }; diff --git a/app/server/components/services/github.js b/app/server/components/services/github.js index 2d9031e..5927bed 100644 --- a/app/server/components/services/github.js +++ b/app/server/components/services/github.js @@ -40,6 +40,7 @@ module.exports = { config: { org: org }, + isMember: Bluebird.promisify(github.orgs.getMember), getUsers: Bluebird.promisify(github.orgs.getMembers), getUser: Bluebird.promisify(github.user.getFrom), getRepos: Bluebird.promisify(github.repos.getFromOrg), diff --git a/app/server/middleware/authenticate.js b/app/server/middleware/authenticate.js index 6d22638..ba24359 100644 --- a/app/server/middleware/authenticate.js +++ b/app/server/middleware/authenticate.js @@ -1,11 +1,13 @@ 'use strict'; +var users = require('../components/repositories/users') + module.exports = { isAuthenticated (req, res, next) { var authenticated = req.isAuthenticated(); console.log("authenticated? " + authenticated); - if (authenticated) { + if (authenticated && users.isMember(req.session.passport.user.username)) { console.log("next...."); next(); } else { diff --git a/app/server/passport.js b/app/server/passport.js index cf67f3a..222e07d 100644 --- a/app/server/passport.js +++ b/app/server/passport.js @@ -20,7 +20,7 @@ module.exports = { }, function (accessToken, refreshToken, profile, done) { console.log("Authentication success!"); - done(null, { displayName: profile.displayName, id: profile.id, token: accessToken }); + done(null, { username: profile.username, displayName: profile.displayName, id: profile.id, token: accessToken }); })); passport.serializeUser(function (user, done) { @@ -38,6 +38,15 @@ module.exports = { app.use(passport.initialize()); app.use(passport.session()); + + app.get('/auth/authenticated', function (req, res) { + var authenticated = req.isAuthenicated(); + if (authenticated) { + res.send(200); + } else { + res.send(401); + } + }); app.get(config.github.authRoute, passport.authenticate('github')); app.get(config.github.authCallbackRoute, passport.authenticate('github', { failureRedirect: config.github.failureRedirect, session: true }), From 27b00914bae27bd48e658256c10ac618569e24fd Mon Sep 17 00:00:00 2001 From: elixic Date: Fri, 13 Mar 2015 16:16:43 -0700 Subject: [PATCH 08/11] fix a few typos and took out some logging the authentication now ensures users are part of the ats org. --- app/server/components/repositories/users.js | 3 +-- app/server/components/repositories/util/users.js | 2 +- app/server/middleware/authenticate.js | 2 -- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/app/server/components/repositories/users.js b/app/server/components/repositories/users.js index 8158c07..a48c7db 100644 --- a/app/server/components/repositories/users.js +++ b/app/server/components/repositories/users.js @@ -29,8 +29,7 @@ module.exports = { isMember (username) { console.log("isMember(" + username + ")"); - return userUtil.isMember(username).then(function (data){ - console.log(data); + return userUtil.isMember(username).then(function (data) { return "204 No Content" === data.meta.success; }); } diff --git a/app/server/components/repositories/util/users.js b/app/server/components/repositories/util/users.js index 8a4441d..8f0b55b 100644 --- a/app/server/components/repositories/util/users.js +++ b/app/server/components/repositories/util/users.js @@ -32,7 +32,7 @@ module.exports = { isMember(username) { return github.isMember({ - org: githug.config.org, + org: github.config.org, username: username }); } diff --git a/app/server/middleware/authenticate.js b/app/server/middleware/authenticate.js index ba24359..2873504 100644 --- a/app/server/middleware/authenticate.js +++ b/app/server/middleware/authenticate.js @@ -5,10 +5,8 @@ var users = require('../components/repositories/users') module.exports = { isAuthenticated (req, res, next) { var authenticated = req.isAuthenticated(); - console.log("authenticated? " + authenticated); if (authenticated && users.isMember(req.session.passport.user.username)) { - console.log("next...."); next(); } else { res.send(401); From d4d5c1eb3beebc4d705b61dfb2a815307daf254a Mon Sep 17 00:00:00 2001 From: elixic Date: Mon, 16 Mar 2015 10:56:25 -0700 Subject: [PATCH 09/11] removing some debug logging. --- app/server/components/repositories/users.js | 1 - app/server/components/repositories/util/users.js | 1 - app/server/passport.js | 16 +++++++--------- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/app/server/components/repositories/users.js b/app/server/components/repositories/users.js index 9a8f785..09c8455 100644 --- a/app/server/components/repositories/users.js +++ b/app/server/components/repositories/users.js @@ -28,7 +28,6 @@ module.exports = { }, isMember (username) { - console.log("isMember(" + username + ")"); return userUtil.isMember(username).then(function (data) { return "204 No Content" === data.meta.success; }); diff --git a/app/server/components/repositories/util/users.js b/app/server/components/repositories/util/users.js index ffc036c..20496a4 100644 --- a/app/server/components/repositories/util/users.js +++ b/app/server/components/repositories/util/users.js @@ -5,7 +5,6 @@ var provider = require('./provider'), convertGithubUser; convertGithubUser = (user) => { - console.log('user', user); return { username: user.login, name: user.name, diff --git a/app/server/passport.js b/app/server/passport.js index 222e07d..6ab6ebe 100644 --- a/app/server/passport.js +++ b/app/server/passport.js @@ -2,7 +2,8 @@ var passport = require('passport'), - GitHubStrategy = require('passport-github').Strategy; + GitHubStrategy = require('passport-github').Strategy, + debug = require('debug')('app:passport'); module.exports = { /** @@ -19,19 +20,18 @@ module.exports = { callbackURL: "http://" + config.server.hostname + ":" + config.server.port + config.github.authCallbackRoute }, function (accessToken, refreshToken, profile, done) { - console.log("Authentication success!"); done(null, { username: profile.username, displayName: profile.displayName, id: profile.id, token: accessToken }); })); passport.serializeUser(function (user, done) { - console.log("serialize user"); - console.log(user); + debug("serialize user"); + debug(user); done(null, user); }); passport.deserializeUser(function (user, done) { - console.log("deserialize user"); - console.log(user); + debug("deserialize user"); + debug(user); done(null, user); }); @@ -51,15 +51,13 @@ module.exports = { app.get(config.github.authCallbackRoute, passport.authenticate('github', { failureRedirect: config.github.failureRedirect, session: true }), function (req, res) { - console.log("authenticated, redirect to /"); var authenticated = req.isAuthenticated(); - console.log("authenticated? " + authenticated); + debug("authenticated? " + authenticated); // TODO: redirect to initial request location... res.redirect('/'); } ); app.get(config.github.failureCallback, function (req, res, next) { - console.log("Error authenticating with github..."); res.send(401); }); } From b6f8bac7b95051937d9ebf22b1cb8e0db5babd02 Mon Sep 17 00:00:00 2001 From: elixic Date: Mon, 16 Mar 2015 11:00:03 -0700 Subject: [PATCH 10/11] removing unused dependency method-override. --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index b9d43c6..fc16f99 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,6 @@ "express-mountie": "^3.0.0", "github": "^0.2.3", "json-mask": "0.3.4", - "method-override": "^2.3.1", "passport": "^0.2.1", "passport-github": "^0.1.5" }, From 3b7eb2342ebbc42f99f830ecf01f2834cabbc976 Mon Sep 17 00:00:00 2001 From: elixic Date: Mon, 16 Mar 2015 16:58:21 -0700 Subject: [PATCH 11/11] addressing some concerns in the pull request comment and adding mock passport init for testing when using mocks. --- app/client/src/modules/app/index.js | 2 +- app/server/components/repositories/users.js | 4 +-- .../components/repositories/util/users.js | 4 +-- app/server/components/services/github.js | 2 +- app/server/components/services/github.mock.js | 8 ++++++ app/server/main.js | 2 +- app/server/middleware/authenticate.js | 2 +- app/server/mock-passport-middleware.js | 27 +++++++++++++++++++ app/server/passport.js | 10 +++++-- 9 files changed, 51 insertions(+), 10 deletions(-) create mode 100644 app/server/mock-passport-middleware.js diff --git a/app/client/src/modules/app/index.js b/app/client/src/modules/app/index.js index 0d7c110..73c9f0c 100755 --- a/app/client/src/modules/app/index.js +++ b/app/client/src/modules/app/index.js @@ -23,7 +23,7 @@ module.exports = .config(function ($urlRouterProvider, $httpProvider) { $urlRouterProvider.otherwise('/'); $httpProvider.interceptors.push('authInterceptor'); - }).factory('authInterceptor', function ($rootScope, $q, $location) { + }).factory('authInterceptor', function ($q, $location) { return { responseError(response) { if (response.status === 401) { diff --git a/app/server/components/repositories/users.js b/app/server/components/repositories/users.js index 09c8455..80bbfba 100644 --- a/app/server/components/repositories/users.js +++ b/app/server/components/repositories/users.js @@ -27,8 +27,8 @@ module.exports = { }); }, - isMember (username) { - return userUtil.isMember(username).then(function (data) { + isOrgMember (username) { + return userUtil.isOrgMember(username).then(function (data) { return "204 No Content" === data.meta.success; }); } diff --git a/app/server/components/repositories/util/users.js b/app/server/components/repositories/util/users.js index 20496a4..9d6e9da 100644 --- a/app/server/components/repositories/util/users.js +++ b/app/server/components/repositories/util/users.js @@ -25,8 +25,8 @@ module.exports = { return provider.github.getUsers(args).then(users => users.map(user => convertGithubUser(user))); }, - isMember(username) { - return provider.github.isMember({ + isOrgMember(username) { + return provider.github.isOrgMember({ org: provider.github.config.org, username: username }); diff --git a/app/server/components/services/github.js b/app/server/components/services/github.js index 75a89ba..0868746 100644 --- a/app/server/components/services/github.js +++ b/app/server/components/services/github.js @@ -34,7 +34,7 @@ module.exports = { config: { org: org }, - isMember: Bluebird.promisify(github.orgs.getMember), + isOrgMember: Bluebird.promisify(github.orgs.getMember), getUsers: Bluebird.promisify(github.orgs.getMembers), getUser: Bluebird.promisify(github.user.getFrom), getRepos: Bluebird.promisify(github.repos.getFromOrg), diff --git a/app/server/components/services/github.mock.js b/app/server/components/services/github.mock.js index feb5cd0..d64ea91 100644 --- a/app/server/components/services/github.mock.js +++ b/app/server/components/services/github.mock.js @@ -201,5 +201,13 @@ module.exports = { reject(new Error('No mock team: ' + id)); } }); + }, + + isOrgMember (msg) { + return new Promise((resolve, reject) => { + resolve({ meta: { + success: "204 No Content" + }}); + }); } }; diff --git a/app/server/main.js b/app/server/main.js index 5a92943..6d4ce9a 100644 --- a/app/server/main.js +++ b/app/server/main.js @@ -17,7 +17,7 @@ exports.start = () => { failureCallback: '/auth/failure' }, session: { - secret: 'keyboard cat', + secret: process.env.SESSION_SECRET || 'keyboard cat', resave: false, saveUninitialized: true, cookie: { diff --git a/app/server/middleware/authenticate.js b/app/server/middleware/authenticate.js index 541fcd2..96e9207 100644 --- a/app/server/middleware/authenticate.js +++ b/app/server/middleware/authenticate.js @@ -6,7 +6,7 @@ module.exports = { isAuthenticated (req, res, next) { var authenticated = req.isAuthenticated(); - if (authenticated && users.isMember(req.session.passport.user.username)) { + if (authenticated && users.isOrgMember(req.session.passport.user.username)) { next(); } else { res.send(401); diff --git a/app/server/mock-passport-middleware.js b/app/server/mock-passport-middleware.js new file mode 100644 index 0000000..ef4b9c6 --- /dev/null +++ b/app/server/mock-passport-middleware.js @@ -0,0 +1,27 @@ +'use strict'; + +//jscs:disable disallowDanglingUnderscores +module.exports = { + initialize(mockUser) { + return function (req, res, next) { + var passport = {}; + passport._key = 'passport'; + passport._userProperty = 'user'; + passport.serializeUser = (user, req, done) => { + done(null, user); + }; + passport.deserializeUser = (user, req, done) => { + done(null, user); + }; + + req._passport = { instance: passport }; + req._passport.session = { user: mockUser }; + req.session.passport = { user: mockUser }; + + next(); + }; + }, + + // TODO ... PUT Mock users in a seperate file so we can tests users that are + mockUser: { username: "TestUser", displayName: "Test User", id: 1 } +}; diff --git a/app/server/passport.js b/app/server/passport.js index 6ab6ebe..ccb8526 100644 --- a/app/server/passport.js +++ b/app/server/passport.js @@ -35,10 +35,16 @@ module.exports = { done(null, user); }); - app.use(passport.initialize()); + if (process.env.SERVICE === 'mock') { + // TODO ... is there a real di way to do this?? + console.log('using the mock passport middlware'); + var mock = require('./mock-passport-middleware'); + app.use(mock.initialize(mock.mockUser)); + } else { + app.use(passport.initialize()); + } app.use(passport.session()); - app.get('/auth/authenticated', function (req, res) { var authenticated = req.isAuthenicated(); if (authenticated) {