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

Make all errors fail by sending JSON #8

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
10 changes: 8 additions & 2 deletions auth/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,21 @@ const createAuthToken = function(user) {
});
};

const localAuth = passport.authenticate('local', {session: false});
const localAuth = passport.authenticate('local', {
session: false,
failWithError: true
});
router.use(bodyParser.json());
// The user provides a username and password to login
router.post('/login', localAuth, (req, res) => {
const authToken = createAuthToken(req.user.serialize());
res.json({authToken});
});

const jwtAuth = passport.authenticate('jwt', {session: false});
const jwtAuth = passport.authenticate('jwt', {
session: false,
failWithError: true
});

// The user exchanges a valid JWT for a new one with a later expiration
router.post('/refresh', jwtAuth, (req, res) => {
Expand Down
5 changes: 5 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@
"start": "node server.js",
"test": "mocha --exit"
},
"prettier": {
"tabWidth": 2,
"singleQuote": true,
"bracketSpacing": false
},
"keywords": [],
"author": "",
"license": "ISC",
Expand Down
50 changes: 37 additions & 13 deletions server.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,20 @@ const passport = require('passport');
// const { james: jimmy, robert: bobby } = actorSurnames;
// console.log(jimmy); // Stewart - the variable name is jimmy, not james
// console.log(bobby); // De Niro - the variable name is bobby, not robert
const { router: usersRouter } = require('./users');
const { router: authRouter, localStrategy, jwtStrategy } = require('./auth');
const {router: usersRouter} = require('./users');
const {router: authRouter, localStrategy, jwtStrategy} = require('./auth');

mongoose.Promise = global.Promise;

const { PORT, DATABASE_URL } = require('./config');
const {PORT, DATABASE_URL} = require('./config');

const app = express();

// Logging
app.use(morgan('common'));

// CORS
app.use(function (req, res, next) {
app.use(function(req, res, next) {
res.header('Access-Control-Allow-Origin', '*');
res.header('Access-Control-Allow-Headers', 'Content-Type,Authorization');
res.header('Access-Control-Allow-Methods', 'GET,POST,PUT,PATCH,DELETE');
Expand All @@ -41,7 +41,10 @@ passport.use(jwtStrategy);
app.use('/api/users/', usersRouter);
app.use('/api/auth/', authRouter);

const jwtAuth = passport.authenticate('jwt', { session: false });
const jwtAuth = passport.authenticate('jwt', {
session: false,
failWithError: true
});

// A protected endpoint which needs a valid JWT to access it
app.get('/api/protected', jwtAuth, (req, res) => {
Expand All @@ -50,25 +53,46 @@ app.get('/api/protected', jwtAuth, (req, res) => {
});
});

app.use('*', (req, res) => {
return res.status(404).json({ message: 'Not Found' });
app.use('*', (req, res, next) => {
const err = new Error('Not Found');
err.status = 404;
next(err);
});

app.use((err, req, res, next) => {
if (!err.status || err.status >= 500) {
return next(err);
}
res.status(err.status).json(
Object.assign({}, err, {
message: err.message
})
);
});

app.use((err, req, res, next) => {
console.error(err);

return res.status(req.status || 500).json({
message: 'Internal Server Error'
});
});

// Referenced by both runServer and closeServer. closeServer
// assumes runServer has run and set `server` to a server object
let server;

function runServer(databaseUrl, port = PORT) {

return new Promise((resolve, reject) => {
mongoose.connect(databaseUrl, err => {
if (err) {
return reject(err);
}
server = app.listen(port, () => {
console.log(`Your app is listening on port ${port}`);
resolve();
})
server = app
.listen(port, () => {
console.log(`Your app is listening on port ${port}`);
resolve();
})
.on('error', err => {
mongoose.disconnect();
reject(err);
Expand All @@ -95,4 +119,4 @@ if (require.main === module) {
runServer(DATABASE_URL).catch(err => console.error(err));
}

module.exports = { app, runServer, closeServer };
module.exports = {app, runServer, closeServer};
83 changes: 36 additions & 47 deletions users/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,16 @@ const router = express.Router();
const jsonParser = bodyParser.json();

// Post to register a new user
router.post('/', jsonParser, (req, res) => {
router.post('/', jsonParser, (req, res, next) => {
const requiredFields = ['username', 'password'];
const missingField = requiredFields.find(field => !(field in req.body));

if (missingField) {
return res.status(422).json({
code: 422,
reason: 'ValidationError',
message: 'Missing field',
location: missingField
});
const err = new Error('Missing field');
err.status = 422;
err.reason = 'ValidationError';
err.location = missingField;
return next(err);
}

const stringFields = ['username', 'password', 'firstName', 'lastName'];
Expand All @@ -28,12 +27,11 @@ router.post('/', jsonParser, (req, res) => {
);

if (nonStringField) {
return res.status(422).json({
code: 422,
reason: 'ValidationError',
message: 'Incorrect field type: expected string',
location: nonStringField
});
const err = new Error('Incorrect field type: expected string');
err.status = 422;
err.reason = 'ValidationError';
err.location = nonStringField;
return next(err);
}

// If the username and password aren't trimmed we give an error. Users might
Expand All @@ -49,12 +47,11 @@ router.post('/', jsonParser, (req, res) => {
);

if (nonTrimmedField) {
return res.status(422).json({
code: 422,
reason: 'ValidationError',
message: 'Cannot start or end with whitespace',
location: nonTrimmedField
});
const err = new Error('Cannot start or end with whitespace');
err.status = 422;
err.reason = 'ValidationError';
err.location = nonTrimmedField;
return next(err);
}

const sizedFields = {
Expand All @@ -71,25 +68,24 @@ router.post('/', jsonParser, (req, res) => {
const tooSmallField = Object.keys(sizedFields).find(
field =>
'min' in sizedFields[field] &&
req.body[field].trim().length < sizedFields[field].min
req.body[field].trim().length < sizedFields[field].min
);
const tooLargeField = Object.keys(sizedFields).find(
field =>
'max' in sizedFields[field] &&
req.body[field].trim().length > sizedFields[field].max
req.body[field].trim().length > sizedFields[field].max
);

if (tooSmallField || tooLargeField) {
return res.status(422).json({
code: 422,
reason: 'ValidationError',
message: tooSmallField
? `Must be at least ${sizedFields[tooSmallField]
.min} characters long`
: `Must be at most ${sizedFields[tooLargeField]
.max} characters long`,
location: tooSmallField || tooLargeField
});
const err = new Error(
tooSmallField
? `Must be at least ${sizedFields[tooSmallField].min} characters long`
: `Must be at most ${sizedFields[tooLargeField].max} characters long`
);
err.status = 422;
err.reason = 'ValidationError';
err.location = tooSmallField || tooLargeField;
return next(err);
}

let {username, password, firstName = '', lastName = ''} = req.body;
Expand All @@ -103,12 +99,12 @@ router.post('/', jsonParser, (req, res) => {
.then(count => {
if (count > 0) {
// There is an existing user with the same username
return Promise.reject({
code: 422,
reason: 'ValidationError',
message: 'Username already taken',
location: 'username'
});
const err = new Error('Username already taken');
err.status = 422;
err.reason = 'ValidationError';
err.message = 'Username already taken';
err.location = 'username';
return Promise.reject(err);
}
// If there is no existing user, hash the password
return User.hashPassword(password);
Expand All @@ -124,24 +120,17 @@ router.post('/', jsonParser, (req, res) => {
.then(user => {
return res.status(201).json(user.serialize());
})
.catch(err => {
// Forward validation errors on to the client, otherwise give a 500
// error because something unexpected has happened
if (err.reason === 'ValidationError') {
return res.status(err.code).json(err);
}
res.status(500).json({code: 500, message: 'Internal server error'});
});
.catch(err => next(err));
});

// Never expose all your users like below in a prod application
// we're just doing this so we have a quick way to see
// if we're creating users. keep in mind, you can also
// verify this in the Mongo shell.
router.get('/', (req, res) => {
router.get('/', (req, res, next) => {
return User.find()
.then(users => res.json(users.map(user => user.serialize())))
.catch(err => res.status(500).json({message: 'Internal server error'}));
.catch(err => next(err));
});

module.exports = {router};