fix: enforce auth on addClientLog call - #1284
Conversation
TCMeldrum
left a comment
TCMeldrum
left a comment
There was a problem hiding this comment.
Looks good only minor question
| errorMessage = 'Exception while preparing error message'; | ||
| } finally { | ||
| getUnauthorizedApi().addClientLog({ error: errorMessage }); | ||
| void sendClientLog(errorMessage, this.state.errorToken); |
There was a problem hiding this comment.
Do we need to void this function call?
There was a problem hiding this comment.
It means the async call is intentionally not awaited. However using void or not does not change anything at runtime.
There was a problem hiding this comment.
Cool thanks for clarifying
jekabs-karklins
left a comment
jekabs-karklins
left a comment
There was a problem hiding this comment.
question: I wonder if you have considered what happens when the call fails during the login, for example in apps/frontend/src/components/user/ExternalAuth.tsx when we call
const handleAuthorizationCode = (authorizationCode: string)
If this fails then we would not get a log.
I think we might weigh on the value of protecing this endpoint vs not getting 100% of the logs, and this might be justified to require the auth.
Do you mean the externalTokenLogin call inside the handleAuthorizationCode? If it fails, we receive an error from the backend, don’t we? |
3 participants
This PR enforces authentication for the addClientLog mutation on the backend and updates the frontend to handle authenticated logging in contexts where React hooks are unavailable.