-
Notifications
You must be signed in to change notification settings - Fork 192
Assignment 4: add basic auth #121
Assignment 4: add basic auth #121
Conversation
added users.json: defines some dummy username/password couples modified index.js: registers new private plugin
} | ||
}, | ||
authStrategyName: 'private-basic', | ||
authValidate: function(username, password, callback) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
personally I would declare the function outside of the object declaration as in internals.authValidate = funtion()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hum, that's what I did in the tests of the file, mainly because I copied their overall style from hapi-auth-basic plugin tests.
Naturally, I tend to define all of an object properties on the spot when their existence is unconditional. Could you please elaborate on why you would declare it outside? I don't see the benefits but I may miss something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the only benefit is readability. If the function gets large or you have a lot of them it can get messy but of course this is more of a personal note than an official style from Hapi
adds return statement for consistency and removes unnecessary blank line in lib/private.js as pointed by @AdriVanHoudt
…tion. updates single user retrieval in lib/private.js to reflect the change and update tests to inject test users before each test and restore users module after each test
I wasn't satisfied by directly using 'production' users in the tests, so I modified Users structure and tests to inject users before each test and switch back to original list after. |
The users are just a placeholder right now, see the assignment, this will later probably be stored in a db which allows you to make a test db. |
Yes, I know that's only a placeholder and the code will be further refactored later, if I was just prototyping and chaining tasks I think I would not care as much, and wait for the next round. However I have some time, so I use it to do some experiments ;) |
You can consider the users files config files ;) |
…n as parameter by Private module. refactor tests accordingly.
Here a version that feels better (to me), where users module is injected during application bootstrapping phase instead of being required in private module, so that fake users can be injected in tests. |
this only has a point if you at least pass in 'test users' instead of the same users you would normally use :D |
Indeed. The previous commit also took care of that ;) |
|
||
// Declare internals | ||
|
||
var internals = {}; | ||
|
||
|
||
exports.init = function (port, next) { | ||
exports.init = function (config, next) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
passing the user file via config wasn't in the assignment.
hueniversity assignment 4 (solves #118 )