Implement authorisation logic for usage statuses #550

[MatteoGuarnaccia5]

Description

This PR implements authorisation logic in the JWTBearer class, which is then utilised in the usage statuses DELETE and POST endpoints to verify if the user is authorised to perform said operation. How the configurable roles are implemented (currently in .env) file is subject to review, and potentially further discussion if there are changing/expanding of user requirements.

This PR also changes the token's used in mock data, to include the changes made in the token payload in ldap-jwt-auth.

Note
This PR is dependent on this pull request in ldap-jwt-auth, and should be merged in conjunction with it, to avoid breaking changes.

Testing instructions

Add a set up instructions describing how the reviewer should test the code

• Review code
• Check Actions build
• Review changes to test coverage

Agile board tracking

connect to #550

[codecov]

Codecov Report

❌ Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 97.26%. Comparing base (f4313f2) to head (5e28387).

Files with missing lines	Patch %	Lines
...entory_management_system_api/auth/authorisation.py	90.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #602      +/-   ##
===========================================
- Coverage    97.29%   97.26%   -0.03%     
===========================================
  Files           57       58       +1     
  Lines         1998     2014      +16     
===========================================
+ Hits          1944     1959      +15     
- Misses          54       55       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[VKTB]

Posting this before I forget. The app fails to start if AUTHENTICATION__ENABLED in the ENV VAR is set to false.

inventory-management-system-api  | │                                                                              │
inventory-management-system-api  | │ /app/inventory_management_system_api/main.py:15 in <module>                  │
inventory-management-system-api  | │                                                                              │
inventory-management-system-api  | │    12                                                                        │
inventory-management-system-api  | │    13 from inventory_management_system_api.core.config import config         │
inventory-management-system-api  | │    14 from inventory_management_system_api.core.logger_setup import setup_lo │
inventory-management-system-api  | │ ❱  15 from inventory_management_system_api.routers.v1 import (               │
inventory-management-system-api  | │    16 │   catalogue_category,                                                │
inventory-management-system-api  | │    17 │   catalogue_item,                                                    │
inventory-management-system-api  | │    18 │   item,                                                              │
inventory-management-system-api  | │                                                                              │
inventory-management-system-api  | │ /app/inventory_management_system_api/routers/v1/usage_status.py:16 in        │
inventory-management-system-api  | │ <module>                                                                     │
inventory-management-system-api  | │                                                                              │
inventory-management-system-api  | │    13                                                                        │
inventory-management-system-api  | │    14 from fastapi import APIRouter, Depends, HTTPException, Path, Request,  │
inventory-management-system-api  | │    15                                                                        │
inventory-management-system-api  | │ ❱  16 from inventory_management_system_api.auth.jwt_bearer import JWTBearer  │
inventory-management-system-api  | │    17 from inventory_management_system_api.core.exceptions import (          │
inventory-management-system-api  | │    18 │   DuplicateRecordError,                                              │
inventory-management-system-api  | │    19 │   InvalidObjectIdError,                                              
inventory-management-system-api  | │                                                                              │
inventory-management-system-api  | │ /app/inventory_management_system_api/auth/jwt_bearer.py:13 in <module>       │
inventory-management-system-api  | │                                                                              │
inventory-management-system-api  | │    10 from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer  │
inventory-management-system-api  | │    11                                                                        │
inventory-management-system-api  | │    12 from inventory_management_system_api.core.config import config         │
inventory-management-system-api  | │ ❱  13 from inventory_management_system_api.core.consts import PUBLIC_KEY     │
inventory-management-system-api  | │    14                                                                        │
inventory-management-system-api  | │    15 logger = logging.getLogger()                                           │
inventory-management-system-api  | ╰──────────────────────────────────────────────────────────────────────────────╯
inventory-management-system-api  | ImportError: cannot import name 'PUBLIC_KEY' from 
inventory-management-system-api  | 'inventory_management_system_api.core.consts' 
inventory-management-system-api  | (/app/inventory_management_system_api/core/consts.py)



inventory-management-system-api exited with code 1

[VKTB]

@MatteoGuarnaccia5 Could you please also make sure that you reference the issue number in your commit messages otherwise they are not showing up in the issue themselves?

[VKTB]

I remembered after submitting my review but could you please update the docstring of is_jwt_access_token_authorised to say that the method should only be called after the token has been verified by the JWTBearer dependency? Also a note to say why we are not checking the signature and expiry time.

[VKTB]

Something that came to my mind is how we can be sure that this dependency is going to run after the JWTBearer dependency which is included in every router in main.py?

[joelvdavies]

I think technically we can't guarentee the order. But would that be a problem? Its still doing the both checks eitherway and if it effects this, then it probably means the same with service dependencies. Putting the authentication in the middleware is the other way to ensure it happens before as had to be done for the object storage api to prevent it with the file uploads as it would allow the whole file to upload before authentication as it is also a dependency.

[MatteoGuarnaccia5]

Isn't it guaranteed to run as it is hard coded into the specific endpoints? Or do dependencies not work like that?

Also, I can add some logging statements so that we can be sure it is checking for authorisation?

[VKTB]

From some resources I came across online, the order in which the dependencies are run may not be guaranteed.

But would that be a problem?

I don't know. Walking through a case may help.

1. is_jwt_access_token_authorised dependency runs first and it returns True. This means the user is authorised but the token's signature and expiry were not verified.
2. JWTBearer dependency runs after the is_jwt_access_token_authorised dependency returns.
   • is_jwt_access_token_authorised returns True (token verification passes) so the endpoint will proceed with executing the request. (This is what we want to happen so it would not be a problem)
   • is_jwt_access_token_authorised returns False (token verification fails) so the endpoint will not proceed with executing the request and return 403 to the client. (This is what we want to happen so it would not be a problem)

The above assumes that any of the logic in the endpoint body methods will execute ONLY after the dependencies fully complete/return.

However, I don't know what the behaviour will be if things are asynchronous i.e. will the logic in the endpoint body methods begin to execute if dependencies have not completed/returned. In the case of this API, we don't have async endpoints (so that is one less thing to worry about) but the callable method of the JWTBearer class is async so I don't know how that is going to behave.

---

Isn't it guaranteed to run as it is hard coded into the specific endpoints? Or do dependencies not work like that?

@MatteoGuarnaccia5 They are guaranteed to run but the order in which they are run may not be guaranteed. I also don't know if they will all run before any logic in the endpoint body methods gets executed.

[MatteoGuarnaccia5]

Aren't these issues still a thing to think about even before this PR, what if endpoint logic runs before a token is authenticated by the JWTBearer class. On any endpoint with or without the extra _authorised_dep dependency it is still a concern no?

[VKTB]

Just to add - since the JWTBearer was introduced in IMS API, I never noticed a case where the logic in the endpoint body methods begin to execute before the JWTBearer dependency finished running so it might be fine after all but I am not sure about the behaviour when endpoints are async.