-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat: Rewrite of session-based securitykeys #764
feat: Rewrite of session-based securitykeys #764
Conversation
This is a draft for solving the problem of one session-based securitykey, making multiple requests in parallel impossible. - a new securitykey is created every time it is requested - securitykeys are bound to a session, if `session`-flag is True (default) - unused securitykeys from a session are either being dropped by session or when they became invalid - Entire code refactoring for both securitykey and session module - This pull request partly replaces viur-framework#704
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.
Hey @phorward thank you for this PR. I have a littel suggetion. Can you please take a look.
Co-authored-by: agudermann <47318461+ArneGudermann@users.noreply.github.com>
core/securitykey.py
Outdated
return True | ||
|
||
elif session.validateSecurityKey(key): | ||
if session and key == "staticSessionKey": |
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.
Can we please introduce a constant for this?
e.g.
STATIC_KEY = "staticSessionKey"
(without "session", bc if you do import session from viur.core
and do session.STATIC_KEY
, "session" would be redundant otherwise)
if session and key == "staticSessionKey": | |
if session and key == STATIC_KEY: |
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.
0bfc59e cleans up the use of the static session key and its naming.
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.
But things got mixed up here.
SECURITYKEY_STATIC
now contains the header name. The static key for the session skey, which was previously "staticSessionKey"
, is now also the header name "Sec-X-ViUR-StaticSessionKey"
. This is btw. a breaking change...
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.
yeah, and who uses it?
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've created #796
Co-authored-by: Sven Eberth <mail@sveneberth.de>
…ore into feat-securitykey.rewrite
- removed duration flag - added amount flag - batch-mode only possible for authenticated users
+ updated the security key documentation
Resolve viur-framework#796 as follow-up of viur-framework#764
This is a draft for solving the problem of one session-based securitykey, making multiple requests in parallel impossible. This pull request partly replaces #704.
Features:
session
-flag is True (default)/json/skey' accepts
amount`-flag for batch-mode skey requesting (decreases amount of requests)