Jwt authentication #1042
Jwt authentication #1042
Conversation
CDimonaco
added
enhancement
CDimonaco
force-pushed
the
jwt_auth
branch
2 times, most recently
from
e7980d5 to
5ecc70c
Compare
CDimonaco
force-pushed
the
jwt_auth
branch
3 times, most recently
from
d42f8a4 to
29dfd90
Compare
|
DO NOT MERGE, BEFORE THE HELM CHART IS UPDATED |
CDimonaco
requested review from
arbulu89,
dottorblaster,
fabriziosestito and
nelsonkopliku
There was a problem hiding this comment.
Hey @CDimonaco ,
It looks great. I couldn't do a really deep review as it is huge.
Besides some cosmetic comments, 2 things:
- The routes in
trento.jsxlook duplicated - The login page component html template now is duplicated in the frontend and backend, is this needed?
PD: I have tested the happy path of the branch, and it works great. And really appreciated for the massive amount of tests you added, this give a lot of confidence to the change
| @@ -2,7 +2,7 @@ import { getValue } from '../support/common'; | |||
|
|
|||
| describe('User account page', () => { | |||
| before(() => { | |||
| cy.navigateToItem('About'); | |||
| cy.visit('/about'); | |||
| cy.url().should('include', '/about'); | |||
There was a problem hiding this comment.
Is this assertion cy.url().should('include', '/about'); needed when we do cy.visit('/about');
There was a problem hiding this comment.
I was thinking about that, but since we have some route guards that can change the route url, I just added as a defensive movement to debug the e2e tests in case of failing, but we can remove
| })), | ||
| }); | ||
| }); | ||
| it('should redirect to the / path, when the user is already logged in', async () => { |
There was a problem hiding this comment.
idk formatter goes this way 🤣
| const usernameField = screen.getByTestId('login-username'); | ||
| const passwordField = screen.getByTestId('login-password'); | ||
|
|
||
| await user.type(usernameField, 'admin'); | ||
| await user.type(passwordField, 'admin'); | ||
|
|
||
| const submitButton = screen.getByTestId('login-submit'); | ||
| await user.click(submitButton); |
There was a problem hiding this comment.
are you sure you can't get those fields by any other query?
There was a problem hiding this comment.
I think I can get them with Id certainly, do you think it's better?
…actored The new tests follows the guideline/directory structure of cypress 12. The use of the new session api allows to integration and testing of the jwt authentication across all the screens.
…hen session expires
|
excellent work, @CDimonaco ! |
Description
This pr change the authentication method of the web project from stateful cookie auth to stateless jwt auth.
Changes the web auth logic and implement on the frontend a jwt auth flow with the all the client side authentication flow handled within the single page application. Route guard on the frontend will be added.
The application will use a access/refresh token flow, with the SPA responsible for all the api interaction, including the login and the token refresh part. Here some diagrams
Login flow
Refresh token success flow
Refresh token failure flow
The frontend routes are protected with a react router
route guard. The guard will check that the user is logged in, and the token is valid. The route uses the/api/meendpoint.The backend jwts, both access and refresh token are signed with a HS256 (HMAC with SHA-256) key, on runtime the key should be provided as config variable.
The AUTHENTICATION IS ENABLED IN THE DEV ENVIRONMENT
How was this tested?
Automatic tests.
The e2e tests are refactored to use cypress version 12 with proper session support across tests, we need that in order to ensure that the authentication mechanism is present in e2e tests and well tested.