Chore(feature)/add request and session logs #532 #727
Conversation
leoseg
force-pushed
the
chore(feature)/addRequestAndSessionLogs-#532
branch
from
83d4e5d to
e380a9f
Compare
|
Hi i did E2E testing with cypress all test run trough except the reset password tests as the mail doesn't arrive at Mailslurp, manual password reset with my Protonmail works without a problem (also done over the password reset button) , do I have to set a specific setting for the Mailslurp? There was another test that failed which was in the inital-exploration test because it expected the "Healing from sexual trauma" to be displayed in the "welcome/bumble" page which was not there (but still is available for the user under courses manually checked it with a bumble user). I think this could be more of a Frontend issue. What do you think @kyleecodes ? |
|
@leoseg yes these are frontend issues, not related to the logging upgrades in this PR. You may disregard these. As for MailSlurp, there is no additional option to set for MailSlurp, only need to follow the instructions in the frontend docs. Thank you for letting us know! |
Fix syntax, missing comma
There was a problem hiding this comment.
@leoseg This is great! Almost done, just for sessionId, it's best to ensure when users logout that the sessionId is cleared from the CLS to avoid retaining an old session. This enables the sessionId to be functional in a testing environment where many users are often linked to one auth token.
This can be done by calling something like this.cls.set('sessionId', null); or this.cls.delete('sessionId'); when the user logs out.
Thanks for your patience. Will merge this ASAP. 👍
|
@kyleecodes Hi thanks for the Review! :) As far as I understand there is no logout API in the backend as the whole session management is managed by the firebase auth guard over token validation and if a user has logged out the token gets deleted in the frontend without a corresponding logout call to the backend (but maybe I am overlooking something here?). Also the CLS context and its session Id is only available in the same request and gets new created for each request. The only problem I see here if are test users using the exactly the same token with same user id AND also the same authTime, if the AuthTime is different the sessionId should be too. If the auth token is exactly the same we need a different solution. I think in cypress the login with password API of firebase is used so there it shouldn't be a problem. What do you think? |
|
Hi @leoseg, great observations. I see your point about firebase managing authentication client-side, and authTime for differentiating tokens. It may be because test data users are linked to the same firebase auth tokens, however, they do have unique user IDs. This makes me wonder if there is an issue with the sessionIDs persisting across user logins, even if logouts are managed on the frontend? Can you verify that the CLS session context is explicitly tied to the authenticated user's token, and resets when user changes? Meanwhile, I'll also be looking into this myself. Once this is verified, it's ready to merge! requestID is all good. Thanks for your help! |
Resolves #532
What changes did you make and why did you make them?
Implemented Cls for request-id logging (request id generated by uuid4) and session logging. For session logging a hash is generated from the firebase user id and the login time which should be unique and also coupling the session id to firebase.
Did you run tests? Share screenshot of results:
Run tests with npm run tests -> All passed .
How did you find us? (GitHub, Google search, social media, etc.):
GitHub