Closed djdduty closed 6 years ago
Merging #2 into develop will decrease coverage by
-41.2%
. The diff coverage is0%
.
@@ Coverage Diff @@
## develop #2 +/- ##
==========================================
- Coverage 100% 58.8% -41.2%
==========================================
Files 4 7 +3
Lines 167 284 +117
==========================================
Hits 167 167
- Misses 0 117 +117
Impacted Files | Coverage Δ | |
---|---|---|
web/security/csrf.py | 0% <0%> (ø) |
|
web/security/permission.py | 0% <0%> (ø) |
|
web/security/release.py | 0% <0%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 4119bab...846b559. Read the comment docs.
So, after an initial look-over I see the user_*
stuff as being methods on the CSRFHandler, and that handler should be lazily bound to the context at start
, not on each prepare
. The method can then be moved from the extension to the handler.
There is a repeating pattern, here, of signers with secrets that sign new signers. To illustrate:
A form's token is a combination of "{32 byte random value}{SHA256 HMAC}", the HMAC is signed using the user's secret (from cookie or session). If a cookie, it's a signed combination of "{32 byte random value}{SHA256 HMAC}", the HMAC is signed using the application's CSRF secret. The token / token factory thing seems to warrant its own reusable type.
The now included csrf ext appears better documented, and likely better designed, going to close this but keep the code around for future reference in case modification of the included extension is needed.
bytes(context.csrf)
(Python 3)str(context.csrf)
(same)__str__
and__bytes__
to allow for flexible session configuration on that side of things.)context.csrf(token)
— returnsTrue
orFalse
. No token, shouldn't trigger the generation of.@when(when.csrf)
(or direct use of class in__acl__
assignment)${context.csrf}
→context.csrf.__html__()
→<input type="hidden" name="_csrf" value="${str(token)}">
Still needs improvements to the actual CSRF check (referer checking and such).