-
Notifications
You must be signed in to change notification settings - Fork 11
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
Update/ux reloading web pages #111
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, remove the auth autologin and auth session cache decorators and implement the needed logic inside the methods.
Handle cookie encrypt secret validation failed in setup, don't throw an error, render a friendly Docq failed to start type message.
Okay |
I left a few comments but feel like the structure is still off. Some of it is just confusion caused by naming conventions. But also cache setting and getting seems a bit spaghetti which could accidently cause a critical security regression in the future. That's my concern. Maybe it's easier for me to look dive into the code a bit deeper first before you make any changes based on my comments. So please hold off making any adjustments until I say so. 🙏🏾 |
* refactor: auth token based logic * fix cookie setting * refactor: remove AUTO_LOGIN as feature type adding ability to toggle token based auth per org is making it more brittle and risky * refactor: rename env vars to prefix "DOCQ_" * refactor: rename session cookie var to be more specific * tests: fix auth utils tests after refactor * fix: remove _auto_login_feature_enabled * chore: tweak debug logs in auth utils
refactor: cache var names
chore: adjust logging not to leak hmac session id into logs.
Description
Please include a summary of the change, e.g. what the new feature # is and/or what bug # it fixes. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes/Implements #(issue/feature)
Fixes page goes to login page on page reload even if you have already logged in.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration by modifying the list below.
Test Configuration:
Please describe the test setup. List them below as bullet points.
Checklist: