Fix and rename Authentication plugin #464
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
https://eaflood.atlassian.net/browse/WATER-4085 In [Create authentication plugin](#351) we added the ability to authenticate and authorise requests to **water-abstraction-system** using the same data the rest of the service relies on, and the cookie [water-abstraction-ui](https://github.com/DEFRA/water-abstraction-ui) will pass through. One of the nifty things in it was the use of [server.dependency()](https://hapi.dev/api/?v=21.3.2#-serverdependencydependencies-after). Currently, we rely on plugins being registered in a particular order to prevent catastrophe. What this PR tried to demonstrate was that using `server.dependency()` we could break the dependence on the order they are registered. We're now ready to enable authentication by default but when we tried with `server.auth.default('session')` we kept getting an error. Hapi kept telling us it didn't recognise that strategy. No matter where we made the call we got the error. When we removed the call to `server.dependency()` and re-ordered the plugins in `server.js` the error went away and default auth on our routes started working. Reading posts like [Handling plugin dependencies](https://hapipal.com/best-practices/handling-plugin-dependencies) highlights there is a lot to think about when it comes to remove the dependence on plugin registration order. If we were maintainers of a plugin, we would need to nail this. But as we are just registering our own for use solely in our own project it looks like considerable overhead and complexity we don't really need. If we screw up the order it becomes obvious pretty quickly something is broken. So, in this change we're going back to being not-clever with our plugins! 😁 On the second point, a re-read of the plugin and the associated service highlighted that these enable both authentication and authorisation in Hapi for our routes. Because of this we're going to rename everything to `auth` so if folks in the future are looking for authorisation related code they don't overlook the plugin and service.
Also add a note about the order.
Cruikshanks
added a commit
that referenced
this pull request
Oct 16, 2023
https://eaflood.atlassian.net/browse/WATER-4085 Doh! Forgot to do this in [Fix and rename Authentication plugin](#464). It's not working for us and we need to remove it to make enabling the default auth strategy work.
Cruikshanks
added a commit
that referenced
this pull request
Oct 16, 2023
https://eaflood.atlassian.net/browse/WATER-4085 Doh! Forgot to do this in [Fix and rename Authentication plugin](#464). It's not working for us and we need to remove it to enable the default auth strategy to work.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://eaflood.atlassian.net/browse/WATER-4085
In Create authentication plugin we added the ability to authenticate and authorise requests to water-abstraction-system using the same data the rest of the service relies on, and the cookie water-abstraction-ui will pass through.
One of the nifty things in it was the use of server.dependency(). Currently, we rely on plugins being registered in a particular order to prevent catastrophe. What this PR tried to demonstrate was that by using
server.dependency()we could break the dependence on the order they are registered.We're now ready to enable authentication by default but when we tried with
server.auth.default('session')we kept getting an error. Hapi kept telling us it didn't recognise that strategy.No matter where we made the call we got the error. When we removed the call to
server.dependency()and re-ordered the plugins inserver.jsthe error went away and the default auth on our routes started working.Reading posts like Handling plugin dependencies highlights there is a lot to think about when it comes to removing the dependence on plugin registration order. If we were maintainers of a plugin, we would need to nail this.
But as we are just registering our own for use solely in our own project it looks like considerable overhead and complexity we don't really need. If we screw up the order it becomes obvious pretty quickly that something is broken. So, in this change, we're going back to being non-clever with our plugins! 😁
On the second point, a re-read of the plugin and the associated service highlighted that these enable both authentication and authorisation in Hapi for our routes. Because of this, we're going to rename everything to
authso if folks in the future are looking for authorisation-related code they don't overlook the plugin and service.