Create authentication plugin #351
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 We implement a plugin to authenticate users. This is purely intended for use by frontend routes, ie. those where a request is passed from the UI to the system.
We create a plugin that sets up the authentication strategy and creates a route to test it. We do not yet set routes to be authenticated by default; we can initially check it by uncommenting `server.auth.default('session')` in the plugin, which will enable authentication for all routes (which will break everything else as practically every request to the system is unauthenticated)
Calling `server.dependency()` in a plugin allows to specify which we will wait to be registeted, and some code to be executed once it's registered. The rest of the plugin code is executed while waiting. In this case, we use `server.dependency()` to wait until `@hapi/cookie` is registered before we set up the authentication strategy, but immediately create the test route as this isn't dependent on anything else. All of this ensures that the order of the plugins in our `registerPlugins()` function isn't important, ie. we have no issues if we register them in the "wrong" order. To demonstrate this, we deliberately register `@hapi/cookie` _after_ `AuthenticationPlugin`
To make it easier to see what's going on with authentication, we directly add our authentication to our test route
Our unit tests are failing because we don't yet have a `COOKIE_SECRET` environment variable. We therefore add it to our `ci.yml` workflow
We originally read the `COOKIE_SECRET` environment variable directly so that we could quickly get a working plugin up and running. Now that we have it working, we create `authentication.config.js` in line with our usual pattern for accessing env vars. Note that we only set the password in the env vars, not the cookie name, ttl etc. We do this because only the password is configurable in our other repos, so there's little point making other things configurable in this repo.
StuAA78
added a commit
that referenced
this pull request
Aug 25, 2023
https://eaflood.atlassian.net/browse/WATER-4085 In anticipation of developing our [authentication plugin](#351) we create migrations to set up our test db with the idm schema and required tables.
StuAA78
added a commit
that referenced
this pull request
Aug 25, 2023
https://eaflood.atlassian.net/browse/WATER-4085 In anticipation of developing our [authentication plugin](#351) we create migrations to set up our test db with the `idm` schema and required tables.
StuAA78
added a commit
that referenced
this pull request
Aug 25, 2023
https://eaflood.atlassian.net/browse/WATER-4085 In anticipation of developing our [authentication plugin](#351) we create models for the `group`, `role` and `user` tables.
StuAA78
added a commit
that referenced
this pull request
Aug 29, 2023
…ntication plugin https://eaflood.atlassian.net/browse/WATER-4085 In anticipation of developing our [authentication plugin](#351) we create models for the `group_roles`, `user_groups` and `user_roles` tables, along with helpers and unit tests.
StuAA78
added a commit
that referenced
this pull request
Aug 29, 2023
) https://eaflood.atlassian.net/browse/WATER-4085 In anticipation of developing our [authentication plugin](#351) we create models for the `group`, `role` and `user` tables, along with helpers and unit tests. The models are all straightforward ones with the exception of `UserModel`, which adds a static method `generateHashedPassword`. This takes a plaintext password and returns a salted and hashed version. The code is taken from `db/seeds/01-users.js`; the intent is that the seed file will at some point be refactored to insert users using `UserModel`, and therefore use the static method in place of the existing function.
StuAA78
added a commit
that referenced
this pull request
Aug 30, 2023
…ntication plugin (#391) https://eaflood.atlassian.net/browse/WATER-4085 In anticipation of developing our [authentication plugin](#351) we create models for the `group_roles`, `user_groups` and `user_roles` tables, along with helpers and unit tests.
We realise that our plugin needs to validate whether or not the user exists, and rather than add additional logic to the plugin to check if the user was found (for example, by checking if the response from `FetchUserRolesAndGroupsService` contains any roles in the roles array) we instead add an explicit `userFound` flag to the service.
We decided that instead of simplying returning a boolean to say if the user was found, we instead return the actual user object. This allows us to check that the user was found (as it will be `null` if not), and gives us the option of using user data later on (eg. email address)
To help us manually test the route, we add a second test path which allows us to add to the scope
Unit testing our plugin logic was proving difficult as we need to inject a valid cookie into our request in order to hit the `validate` handler. However there doesn't seem to be any straightforward way of generating a cookie so we were looking at hardcoding it. Instead of this, we move the plugin logic into a separate `AuthenticationService`, which we can easily unit test.
These test paths were used for manual poking of endpoints to ensure we were on the right track. We remove them now that they are no longer needed.
Cruikshanks
requested changes
Sep 6, 2023
Co-authored-by: Alan Cruikshanks <[email protected]>
Cruikshanks
previously approved these changes
Sep 7, 2023
We previously added a `TODO` to our authentication plugin to flag up that we want to apply our `session` authentication to all routes by default. We have now created an issue within our team repo for this work, so we remove the `TODO` from the codebase
Cruikshanks
approved these changes
Sep 7, 2023
Cruikshanks
added a commit
to DEFRA/water-abstraction-ui
that referenced
this pull request
Oct 11, 2023
https://eaflood.atlassian.net/browse/WATER-4085 https://eaflood.atlassian.net/browse/WATER-4155 In our new repo [water-abstraction-system](https://github.com/DEFRA/water-abstraction-system) we added a [new authentication plugin](DEFRA/water-abstraction-system#351). We want this UI to be able to redirect users to new pages we create in **water-abstraction-system** and for that to be seamless and secure. We could have managed authentication in our `src/internal/modules/system-proxy` module. But the idea is to move away from the legacy code base which means at some point **water-abstraction-system** needs to be doing authentication and authorisation. So, we opted to perform the same checks there; admittedly we're performing authentication twice but it will give us more flexibility in the future. Any way, that os the background so on to our issue. We're building our first page in **water-abstraction-system** where the user needs to be authenticated. This should have just worked only it didn't. After much head scratching we tracked down the problem to the cookie this project creates not getting passed through. Every cookie was, just not the one we need!! The answer was just to tweak the config for [hapi/h202](https://hapi.dev/module/h2o2/) to allow local state to be passed through.
Cruikshanks
added a commit
to DEFRA/water-abstraction-ui
that referenced
this pull request
Oct 12, 2023
https://eaflood.atlassian.net/browse/WATER-4085 https://eaflood.atlassian.net/browse/WATER-4155 In our new repo [water-abstraction-system](https://github.com/DEFRA/water-abstraction-system) we added a [new authentication plugin](DEFRA/water-abstraction-system#351). We want this UI to be able to redirect users to new pages we create in **water-abstraction-system** and for that to be seamless and secure. We could have managed authentication in our `src/internal/modules/system-proxy` module. But the idea is to move away from the legacy code base which means at some point **water-abstraction-system** needs to be doing authentication and authorisation. So, we opted to perform the same checks there; admittedly we're performing authentication twice but it will give us more flexibility in the future. Anyway, that is the background so on to our issue. We're building our first page in **water-abstraction-system** where the user needs to be authenticated. This should have just worked only it didn't. After much head scratching we tracked down the problem to the cookie this project creates not getting passed through. Every cookie was, just not the one we need!! The answer was just to tweak the config for [hapi/h202](https://hapi.dev/module/h2o2/) to allow local state to be passed through.
Cruikshanks
added a commit
that referenced
this pull request
Oct 16, 2023
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.
Cruikshanks
added a commit
that referenced
this pull request
Oct 16, 2023
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 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 in `server.js` the error went away and the 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 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 `auth` so if folks in the future are looking for authorisation-related code they don't overlook the plugin and service.
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
We implement
AuthenticationPluginto authenticate users. This is purely intended for use by frontend routes, ie. those where a request is passed from the UI to the system.The plugin creates an authentication strategy
sessionwhich implements thecookieauth scheme.The main bit of authentication is done by the strategy before our validation handler runs. It checks that the
sidcookie passed on from the UI is valid, and if not it redirects the user to/signin.If the cookie is valid then the plugin's validation handler calls on
AuthenticationServiceto do its own authentication, which in turn makes use ofFetchUserRolesAndGroupsServiceto fetch user data from the IDM.AuthenticationServiceresolves to an object populated with the data fromFetchUserRolesAndGroupsService, which theAuthenticationPluginpasses on to the underlying auth strategy:isValidboolean is settrueif the user exists in the IDM, orfalseif they don't. The auth strategy makes use of this to reject the request with a 403 error if it's set tofalse.credentialsobject is populated with theuser,groupsandroleswhich were fetched from the IDM. At present we intend to make use ofuserto read things like the user's email address; at present thegroupsandrolesare simply "nice to have", although this may change in future.credentialsis also populated with an array namedscope, which contains the names of the roles the user has (which it gets from theroleproperty of each role object).credentials.scopeis used by hapi for authentication; if a route hasoptions.auth.strategy.scopewith an array of scopes then hapi will reject requests if the user'scredentials.scopedoes not contain at least one of those strings. In other words, putting one or more role names into a route'soptions.auth.strategy.scopewill prevent requests if the user doesn't have at least one of the roles. More details on scope can be found in the hapi docs.If authentication succeeds, the auth strategy will populate
request.authwithcredentialsamongst other things; see the hapi docs for more details. This means for example that we can access the user's email viarequest.auth.credentials.user.userName.