Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🔒 fix: update refresh token handling to use plain token instead of hashed token #5088

Merged
merged 2 commits into from
Dec 23, 2024

Conversation

berry-13
Copy link
Collaborator

@berry-13 berry-13 commented Dec 23, 2024

Summary

fix login bug caused by #5077

Change Type

  • Bug fix (non-breaking change which fixes an issue)

Testing

Tested login, logout, api operations (like fetch convo and other basic interactions), refresh token logic, reset password

Checklist

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • I have commented in any complex areas of my code
  • I have made pertinent documentation changes
  • My changes do not introduce new warnings
  • I have written tests demonstrating that my changes are effective or that my feature works
  • Local unit tests pass with my changes
  • Any changes dependent on mine have been merged and published in downstream modules.
  • A pull request for updating the documentation has been submitted.

@berry-13 berry-13 marked this pull request as draft December 23, 2024 14:30
@berry-13 berry-13 marked this pull request as ready for review December 23, 2024 15:09
@berry-13 berry-13 merged commit d6f1ecf into main Dec 23, 2024
2 checks passed
@berry-13 berry-13 deleted the fix/session branch December 23, 2024 17:38
@KiGamji
Copy link
Contributor

KiGamji commented Dec 23, 2024

@berry-13 I am experiencing issue with the previous and this fix, getting unlogined each time I refresh the page after logging in.

@berry-13
Copy link
Collaborator Author

@berry-13 I am experiencing issue with the previous and this fix, getting unlogined each time I refresh the page after logging in.

what login strategy are you using?

@KiGamji
Copy link
Contributor

KiGamji commented Dec 23, 2024

@berry-13 I am experiencing issue with the previous and this fix, getting unlogined each time I refresh the page after logging in.

what login strategy are you using?

Email accounts.

@berry-13
Copy link
Collaborator Author

Email accounts.

would you mind if we move this chat to a GitHub discussion or a Discord #issue? It'll help me understand better why it's not working for you

@KiGamji
Copy link
Contributor

KiGamji commented Dec 23, 2024

Email accounts.

would you mind if we move this chat to a GitHub discussion or a Discord #issue? It'll help me understand better why it's not working for you

sure, I'll create a discord issue in a moment

@danny-avila
Copy link
Owner

@berry-13 if this is creating issues, you should revert your earlier fix. All we needed were sessions to be deleted on password reset

@KiGamji
Copy link
Contributor

KiGamji commented Dec 23, 2024

@berry-13 if this is creating issues, you should revert your earlier fix. All we needed were sessions to be deleted on password reset

This PR seems to fix the issue with sessions on SSL deployments, unsure about the local ones. We're currently investigating this further on discord issues channel.

@stanvx
Copy link

stanvx commented Dec 23, 2024

I've also found I'm unable to authenticate when using Authentik with OAuth2/OIDC, for now I'm back on the librechat:v0.7.6 image.

@danny-avila
Copy link
Owner

@stanvx Not having issues with OIDC, you are likely using an image that includes the bug

@stanvx
Copy link

stanvx commented Dec 25, 2024

@danny-avila Thanks, I'll do some testing, check the logs, find out which image it started on and make an issue if needed.

@stanvx
Copy link

stanvx commented Dec 26, 2024

Okay, I confirmed on my side of things that I was using the image with the bug, for whatever reason the librechat-dev:latest tag wasn't updating, worked fine specifying the specific sha. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants