diff --git a/Routes.js b/Routes.js index 074c0dc..0ec8d43 100644 --- a/Routes.js +++ b/Routes.js @@ -6,10 +6,12 @@ * To change this template use File | Settings | File Templates. */ function setup(app, handlers, authorisationPolicy) { - app.post('/api/profiles', handlers.account.createAccount); - app.get('/api/profiles/:username', handlers.account.getAccount); - app.put('/api/profiles/:username', handlers.account.updateAccount); - app.del('/api/profiles/:username', handlers.account.deleteAccount); + //app.post('/api/profiles', handlers.account.createAccount); + //app.get('/api/profiles/:username', handlers.account.getAccount); + app.get('/api/profiles/:userId', authorisationPolicy, handlers.account.getAccount); + //app.put('/api/profiles/:username', handlers.account.updateAccount); + //app.del('/api/profiles/:username', handlers.account.deleteAccount); + app.del('/api/profiles/:userId', authorisationPolicy, handlers.account.deleteAccount); app.post('/api/profiles/:userId/lists', authorisationPolicy, handlers.list.createShoppingList); app.post('/api/profiles/:userId/lists/:templateId', authorisationPolicy, handlers.list.createShoppingList); app.put('/api/profiles/:userId/lists/:shoppingListId', authorisationPolicy, handlers.list.updateShoppingList); diff --git a/TODO b/TODO index 9aea1ae..1e04a37 100644 --- a/TODO +++ b/TODO @@ -18,12 +18,3 @@ - Modify test/Routes.js to add a after() method to cleanup db and close connection mongodb -- Secure each API endpoint so that we check for the API ACCESS TOKEN that is valid, and that the user id related to the - token is the same user id contained in the URL. Otherwise return 403 FORBIDDEN - - Refactor following routes to accept userId: - app.get('/api/profiles/:username', handlers.account.getAccount); - app.put('/api/profiles/:username', handlers.account.updateAccount); - app.del('/api/profiles/:username', handlers.account.deleteAccount); - - - Account Search and delete: GET, DELETE -> /profiles/:username should only work - for the current user , otherwise return 401 NOT AUTHORIZED diff --git a/handlers/AccountHandler.js b/handlers/AccountHandler.js index 0dbd73b..f294b5e 100644 --- a/handlers/AccountHandler.js +++ b/handlers/AccountHandler.js @@ -6,6 +6,7 @@ var Account = require('../models/Account'); var AccountRepository = require('../repositories/accountRepository'); +var SecurityToken = require('../infrastructure/securityToken'); var logger = require('../utils/logger'); var winston = require('winston'); @@ -43,30 +44,30 @@ function handleCreateAccountRequest(req, res) { ); } -/* TODO: I should update this function to check the identity of the origin. - We don't want anyone being able to peek into the username collection */ function handleGetAccountRequest(req, res) { - var username = req.params.username || null; + var userId = req.params.userId || null; var accountRepository = new AccountRepository(); - accountRepository.findAccountByUsername(username) + accountRepository.findById(userId) .then( function(account) { - if (account && account.isActive === true) { - logger.log('info', 'Account ' + username + ' has been retrieved.' + + console.log(account); + if (account && account.isActive === true) { + logger.log('info', 'Account ' + userId + ' has been retrieved.' + 'Request from address ' + req.connection.remoteAddress + '.'); res.json(200, account); } else { - logger.log('info', 'Could not retrieve account ' + username + ', no ' + - 'such username. Request from address ' + req.connection.remoteAddress + '.'); + console.log('account not found'); + logger.log('info', 'Could not retrieve account ' + userId + ', no ' + + 'such id exists. Request from address ' + req.connection.remoteAddress + '.'); res.json(404, { - error: "No account found matching " + username + error: "No account found matching id " + userId }); } }, function(err) { logger.log('error', 'An error has occurred while processing a request to retrieve ' + - 'account ' + username + ' from ' + req.connection.remoteAddress + + 'account id ' + userId + ' from ' + req.connection.remoteAddress + '. Stack trace: ' + err.stack); res.json(500, { error: err.message @@ -109,28 +110,29 @@ function handleUpdateAccountRequest(req, res) { } function handleDeleteAccountRequest(req, res) { - var username = req.params.username || null; + var userId = req.params.userId || null; var accountRepository = new AccountRepository(); - accountRepository.disableAccount(username) + accountRepository.disableAccount(userId) .then( function (account) { if (account) { - logger.log('info', 'Account ' + username + ' has been disabled.' + + SecurityToken.removeSecurityTokensForUserId(userId); + logger.log('info', 'Account id ' + userId + ' has been disabled.' + 'Request from address ' + req.connection.remoteAddress + '.'); // No need to return anything. We just disabled the account res.json(204, null); } else { - logger.log('info', 'Could not disable account ' + username + ', no ' + - 'such username. Request from address ' + req.connection.remoteAddress + '.'); + logger.log('info', 'Could not disable account id ' + userId + ', no ' + + 'such id exists. Request from address ' + req.connection.remoteAddress + '.'); res.json(404, { - error: "No account found matching " + username + error: "No account found matching id " + userId }); } }, function (err) { logger.log('error', 'An error has occurred while processing a request to disable ' + - 'account ' + username + ' from ' + req.connection.remoteAddress + + 'account id ' + userId + ' from ' + req.connection.remoteAddress + '. Stack trace: ' + err.stack); res.json(500, { error: err.message diff --git a/handlers/ShoppingListHandler.js b/handlers/ShoppingListHandler.js index 272a39e..1d85cd1 100644 --- a/handlers/ShoppingListHandler.js +++ b/handlers/ShoppingListHandler.js @@ -199,7 +199,7 @@ function handleGetShoppingListRequest(req, res) { } } -/// Update a shopping list only if the user is authorised (is the creator of this list and the list is not deleted +/// Update a shoppfing list only if the user is authorised (is the creator of this list and the list is not deleted /// Url: /api/profiles/:userId/lists/:shoppingListId function handleUpdateShoppingListRequest(req, res) { // Retrieve the shopping list id from the request diff --git a/infrastructure/securityToken.js b/infrastructure/securityToken.js index d902a6e..9281586 100644 --- a/infrastructure/securityToken.js +++ b/infrastructure/securityToken.js @@ -87,6 +87,20 @@ securityTokenSchema.statics.removeSecurityToken = function(apiAccessToken) { return deferred.promise; }; +securityTokenSchema.statics.removeSecurityTokensForUserId = function(userId) { + var deferred = Q.defer(); + SecurityToken.remove({ userId: userId }, function (err) { + if (err) { + console.log("In removeSecurityToken: " + err); + deferred.reject(err); + } + else { + deferred.resolve(userId); + } + }); + return deferred.promise; +}; + securityTokenSchema.statics.authorise = function(apiAccessToken, userId) { var deferred = Q.defer(); SecurityToken.findSecurityToken(apiAccessToken) diff --git a/repositories/accountRepository.js b/repositories/accountRepository.js index f357d42..99411a8 100644 --- a/repositories/accountRepository.js +++ b/repositories/accountRepository.js @@ -165,10 +165,10 @@ function updateLastLoginDate(account, lastLogin) { return deferred.promise; } -function disableAccount(username) { +function disableAccount(userId) { var deferred = Q.defer(); var query = { - username: username + _id: userId }; var options = { 'new': true diff --git a/test/Routes.js b/test/Routes.js index a5cb6ff..0d2a51f 100644 --- a/test/Routes.js +++ b/test/Routes.js @@ -25,10 +25,8 @@ describe('Routing', function() { var url; - url = 'http://localhost:3000'; - var testUsername = makeid(); - var testToDeleteUsername = makeid(); - var testUserId; + url = 'http://localhost:3000'; + var fbUsername; var apiAccessToken; var loginDone = false; var fbUserId; @@ -40,7 +38,7 @@ mongoose.connect(config.db.mongodb); } var loginCredentials = { - fbToken: 'CAACEdEose0cBADpR2wZByXutZCkUvistNT52w0lKsDUzxeJBxJkW5DNl6eqAQBZC4B7nduib8Hs6hlE2Llrl1RFa0tIzbzBZCTXZCX2MB8stBhR1RrZBwZApO7HAPnFaQr5oiyAgORGVXpFoewAWX6OOLYGtD7yAzkozzCcyWPUZCWI0pLxxZBgqgdytYZBvZAuRJ4ZD', + fbToken: 'CAACEdEose0cBAEqPeiV769RPHO0duspjZAFeDFG2ShEjdfq1JcwniMLZBvfHI9NU1JL8FWyDMgOXOXzec5XEPh7HnNVnFZCzbPYmFL6b5uskagDajIgwqZCY995p4fbP4ctZAOEFZBpCDiSVTHZCmrjIYZAwrMLJ4bUbJHUFVpTZC5sRYf4GIgCd2JBZC06EmUVoYZD', appName: 'testFBMobile' }; request(url) @@ -54,12 +52,13 @@ apiAccessToken = res.body.apiAccessToken; fbUserId = res.body.userId; // I should use the fbUserId as the user id of all of my requests! loginDone = true; + fbUsername = res.body.username; done(); }); //done(); - });/* + }); describe('Account', function() { - it('should return error trying to save account without username', function(done) { + /*it('should return error trying to save account without username', function(done) { var profile = { //username: null, password: 'test', @@ -140,22 +139,24 @@ res.should.have.status(400); done(); }); - }); - it('should return a 404 status code for an unknown account', function(done){ + });*/ + it('should return a 401 status code trying to retrieve an account different than mine', function(done){ request(url) .get('/api/profiles/ciccio') + .send({apiAccessToken: apiAccessToken}) .expect('Content-Type', /json/) .end(function(err,res) { if (err) { throw err; } - res.should.have.status(404); + res.should.have.status(401); done(); }); }); it('should retrieve an existing account', function(done){ request(url) - .get('/api/profiles/' + testUsername) + .get('/api/profiles/' + fbUserId) + .send({apiAccessToken: apiAccessToken}) .expect('Content-Type', /json/) .end(function(err,res) { if (err) { @@ -164,10 +165,12 @@ res.should.have.status(200); res.body.should.have.property('_id'); res.body.creationDate.should.not.equal(null); + res.body.firstName.should.equal('Valerio'); + res.body.lastName.should.equal('Gheri'); done(); }); }); - it('should return a 404 status code trying to update an unknown account', function(done){ + /*it('should return a 404 status code trying to update an unknown account', function(done){ var body = { firstName: 'Noone', lastName: 'Unknown', @@ -218,29 +221,8 @@ res.should.have.status(404); done(); }); - }); - it('should correctly delete account ' + testToDeleteUsername, function(done){ - request(url) - .del('/api/profiles/' + testToDeleteUsername) - .end(function(err,res) { - if (err) { - throw err; - } - res.should.have.status(204); - request(url) - .get('/api/profiles/' + testToDeleteUsername) - .expect('Content-Type', /json/) - .end(function(err,res) { - if (err) { - throw err; - } - res.should.have.status(404); - - done(); - }); - }); - }); - }); */ + }); */ + }); describe('Shopping List', function() { var userId; var shoppingListId; @@ -840,6 +822,28 @@ } }); }); + /*it('should correctly delete account id ' + userId, function(done){ + request(url) + .del('/api/profiles/' + userId) + .send({apiAccessToken: apiAccessToken}) + .end(function(err,res) { + if (err) { + throw err; + } + res.should.have.status(204); + request(url) + .get('/api/profiles/' + userId) + .send({apiAccessToken: apiAccessToken}) + .expect('Content-Type', /json/) + .end(function(err,res) { + if (err) { + throw err; + } + res.should.have.status(404); + done(); + }); + }); + });*/ }); });