Skip to content

Commit

Permalink
Secured account get/delete endpoints. On delete, all api tokens linke…
Browse files Browse the repository at this point in the history 
…d with the account are also deleted
  • Loading branch information
@vgheri
vgheri committed Nov 17, 2013
1 parent 7935671 commit 4c56b05
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 68 deletions.
10 changes: 6 additions & 4 deletions Routes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
9 changes: 0 additions & 9 deletions TODO
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
36 changes: 19 additions & 17 deletions handlers/AccountHandler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion handlers/ShoppingListHandler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions infrastructure/securityToken.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions repositories/accountRepository.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
74 changes: 39 additions & 35 deletions test/Routes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -40,7 +38,7 @@
mongoose.connect(config.db.mongodb);
}
var loginCredentials = {
fbToken: 'CAACEdEose0cBADpR2wZByXutZCkUvistNT52w0lKsDUzxeJBxJkW5DNl6eqAQBZC4B7nduib8Hs6hlE2Llrl1RFa0tIzbzBZCTXZCX2MB8stBhR1RrZBwZApO7HAPnFaQr5oiyAgORGVXpFoewAWX6OOLYGtD7yAzkozzCcyWPUZCWI0pLxxZBgqgdytYZBvZAuRJ4ZD',
fbToken: 'CAACEdEose0cBAEqPeiV769RPHO0duspjZAFeDFG2ShEjdfq1JcwniMLZBvfHI9NU1JL8FWyDMgOXOXzec5XEPh7HnNVnFZCzbPYmFL6b5uskagDajIgwqZCY995p4fbP4ctZAOEFZBpCDiSVTHZCmrjIYZAwrMLJ4bUbJHUFVpTZC5sRYf4GIgCd2JBZC06EmUVoYZD',
appName: 'testFBMobile'
};
request(url)
Expand All @@ -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',
Expand Down Expand Up @@ -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) {
Expand All @@ -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',
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
});
});
});*/
});
});

Expand Down

0 comments on commit 4c56b05

Please sign in to comment.