Implement refresh endpoint #11
Conversation
There was a problem hiding this comment.
One minor thing and same query about tests/linting.
Also note for @joshuadkitenge, it appears operations gateway-api/this actually currently uses the icat provider instead of the jwtAuthProvider (https://github.com/ral-facilities/scigateway/blob/8c501b09aeec1fdf6c04474c080b1b30577c86be/src/authentication/icatAuthProvider.tsx vs https://github.com/ral-facilities/scigateway/blob/8c501b09aeec1fdf6c04474c080b1b30577c86be/src/authentication/jwtAuthProvider.tsx). The main difference is the inclusion of the maintenance states by the looks of it. Also the jwt provider instead uses Never mind - it was updated in the React 18 branch./api/jwt/authenticate, /api/jwt/checkToken, /api/jwt/refresh instead which will also not currently work unless we change this to use that instead.
ldap_jwt_auth/auth/jwt_hanlder.py
Outdated
| payload = self._get_jwt_payload(access_token, {"verify_exp": False}) | ||
| payload["exp"] = datetime.now(timezone.utc) + timedelta( | ||
| minutes=config.authentication.access_token_validity_minutes | ||
| ) |
There was a problem hiding this comment.
Just some info on token rotation https://auth0.com/docs/secure/tokens/refresh-tokens/refresh-token-rotation I mentioned. No need here though.
ldap_jwt_auth/routers/refresh.py
Outdated
| ) | ||
| def refresh_access_token( | ||
| jwt_handler: Annotated[JWTHandler, Depends(JWTHandler)], | ||
| access_token: Annotated[str, Query(description="The JWT access token to refresh")], |
There was a problem hiding this comment.
This needs to be named token in the body for SciGateway (See https://github.com/ral-facilities/operationsgateway-api/blob/fb655d976e794e23a96828f6d2ad8cdef46d0141/operationsgateway_api/src/routes/auth.py#L18-L24)
There was a problem hiding this comment.
Code LGTM - On the refresh issue, it appears SciGateway is missing { withCredentials: true } in the refresh axios request to send the cookie - Adding it made it work for me otherwise it couldn't find the cookie like you mentioned. Although I don't know why this hasn't already been addressed unless it hasn't been tried yet or there is some other way around it.
Although even with that change - it fails as the token is returned in the body but needs
{
"token": token
}
instead of
{
token
}
I don't know why it is different to the login though - it doesn't look like operations gateway does this either, so I suspect SciGateway needs updating. If this looks right to you, I think I can just request the changes on the react-18 pr.
|
@joelvdavies I am not sure about the cookie, sorry. Regarding the response for the |
|
@VKTB SciGateway auth also has a maintenance endpoint so it looks like it uses SciGateway's |
Description
Implements a
/refreshPOSTendpoint and relevant application logic that takes a JWT access token along with a JWT refresh token, generates an updated JWT access token, and returns it along with the JWT refresh token as an HTTP-only cookie.Testing instructions
Add a set of instructions describing how the reviewer should test the code
Agile board tracking
closes #10