-
Notifications
You must be signed in to change notification settings - Fork 52
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
refactor/jinja-global-variable #302
base: develop
Are you sure you want to change the base?
refactor/jinja-global-variable #302
Conversation
…ssac211/calendar into refactor/jinja-global-variable
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.
The idea seem very good, yet I can't see how does it work, could you add documentation?
And if it does work the way you wanted, could you add support for showing log-out only for logged-in users, and login/sign up for not logged-in users?
Codecov Report
@@ Coverage Diff @@
## develop #302 +/- ##
========================================
Coverage 98.76% 98.77%
========================================
Files 63 64 +1
Lines 2843 2863 +20
========================================
+ Hits 2808 2828 +20
Misses 35 35
Continue to review full report at Codecov.
|
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.
I find it a bit hard to understand what is the purpose of the feature.
Can you please explain it in a nutshell?
Upload the connected user to a global variable The router I added is for illustration purposes, so it's inside a demo folder. |
#302 (review) in the tests\global_var_testing_routes.py I changed the code, and used the security module functions to make it more usable in the project. So that this function can be used in routers that still want to identify if the user is connected but do not want to be redirected to the login page if not (This is how it was defined in the security module functions) |
@@ -0,0 +1,52 @@ | |||
<!DOCTYPE html> |
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.
There shouldn't be a file used just for testing in the templates
directory.
Check the documentation for functions that allow you to load templates from strings instead.
@@ -0,0 +1,21 @@ | |||
from sqlalchemy.orm import Session |
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.
I believe this file can also be deleted
|
||
|
||
async def get_user_for_global_var(db: Session, jwt: str) -> str: | ||
jwt_payload = await get_jwt_token(db, jwt) |
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.
This will make the site query the database with every request to the site.
It's nullify the advantages of JWT
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.
So maybe I could pull the token out of the cookie, and if the token is in the right length (and maybe more tests), I wiil set a global var: 'is_token'.
To verify that there is a logged in user.
Tell me what you think
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.
Great! You can also take the user details from the JWT, not from the database.
|
||
from app.dependencies import templates | ||
from app.internal.security.ouath2 import ( | ||
Session, get_jwt_token, get_authorization_cookie |
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.
Use git precommit hooks to order that right and add a trailing comma
user global variable
Contains an example of how to use a global variable
Once it is possible to see if a user is logged in, this method can be used in the code