Update for hapi 17.x.x #61
Conversation
README.md
Outdated
|
|
||
| const main = async () => { | ||
|
|
||
| const server = new Hapi.Server({ port: 4000 }); |
| @@ -31,22 +33,47 @@ const users = { | |||
| } | |||
| }; | |||
|
|
|||
| const validate = function (request, username, password, callback) { | |||
| const validate = async (request, username, password, h) => { | |||
There was a problem hiding this comment.
What's the point of h if you're not using it ? Should there be an example with it being used ?
There was a problem hiding this comment.
I'll remove it from here as it's not used. Don't need to show all functionality in the example.
There was a problem hiding this comment.
Is there any case where you would use it ?
There was a problem hiding this comment.
Yes, you can set the return object response property to a custom response. Previously this was passed in the error position but it doesn't seem appropriate for it to be thrown.
You would use h to do something like:
return { response: h.redirect('http://hapijs.com') };There was a problem hiding this comment.
May be worth of an example then ?
There was a problem hiding this comment.
Sure, added to the README: d6396b9
package.json
Outdated
| "boom": "3.x.x", | ||
| "hoek": "4.x.x" | ||
| "boom": "7.x.x", | ||
| "bounce": "^1.2.0", |
lib/index.js
Outdated
| if (!authorization) { | ||
| return reply(Boom.unauthorized(null, 'Basic', settings.unauthorizedAttributes)); | ||
| return h.unauthenticated(Boom.unauthorized(null, 'Basic', settings.unauthorizedAttributes)); |
There was a problem hiding this comment.
If you are not also passing credentials, you can just throw the error.
lib/index.js
Outdated
|
|
||
| credentials = credentials || null; | ||
| try { |
There was a problem hiding this comment.
This try scope is far too big. Declare the try variables with var and limit it to the one line.
There was a problem hiding this comment.
Correct me if I'm wrong but I don't think we need the try/catch at all then, if it's ok to just have the error rethrown in this scope.
lib/index.js
Outdated
|
|
||
| credentials = credentials || null; | ||
| try { | ||
| const { isValid, credentials, takeover } = await settings.validate(request, username, password, h); |
There was a problem hiding this comment.
Maybe response is a better variable name than takeover given the overlap with a hapi api.
|
Please set milestones... |
|
This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions. |
Would appreciate reviews from people more familiar with async/await etc to give feedback on the approach taken.
Main changes:
validateFuncoption tovalidateerr, isValid, credentialsto validateFunc callback it now returns an object or throws an errorerrposition (e/g/ to support redirects) has been supported here by instead returning atakeoverresponseproperty of the return object. I did consider just having it return a response object but not sure there's a way to check if it is a response object in the scheme?Closes #60