🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Refresh JWT tokens authenticate as access JWT tokens (#1320)
Currently it is possible to auth with JWT access token and JWT refresh token when
We need to change how
And define
(please, do not take this issue before the 1st of September)
#bug #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
📝 Refresh JWT tokens authenticate as access JWT tokens (#1320)
Currently it is possible to auth with JWT access token and JWT refresh token when
HeaderJWTSyncAuth / HeaderJWTAsyncAuth / CookieJWT*Auth are used.We need to change how
decode_token method works. It must check:if token.extras.get('type') != self.expected_token_type:
raise NotAuthenticatedError
And define
expected_token_type attribute on base JWT auth with 'access' as the default value.(please, do not take this issue before the 1st of September)
#bug #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 `RedirectTo` accepts protocol-relative URLs (#1326)
django-modern-rest/dmr/response.py
Lines 164 to 168 in a2d44b1
This might be a bug in Django as well.
So, the
Django has https://github.com/django/django/blob/73cc09f14f13fedddc14d6ba5b287cb33c24e4a4/django/utils/http.py#L274 for this case.
And this is how it is used: https://github.com/django/django/blob/73cc09f14f13fedddc14d6ba5b287cb33c24e4a4/django/contrib/auth/views.py#L43-L59
We need to add docs about
(please, do not take this issue before the 1st of September)
#documentation #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
📝 `RedirectTo` accepts protocol-relative URLs (#1326)
django-modern-rest/dmr/response.py
Lines 164 to 168 in a2d44b1
This might be a bug in Django as well.
>>> from urllib.parse import urlsplit
>>> urlsplit('//evil.example/x').scheme
''
So, the
scheme check is skipped.Django has https://github.com/django/django/blob/73cc09f14f13fedddc14d6ba5b287cb33c24e4a4/django/utils/http.py#L274 for this case.
And this is how it is used: https://github.com/django/django/blob/73cc09f14f13fedddc14d6ba5b287cb33c24e4a4/django/contrib/auth/views.py#L43-L59
We need to add docs about
RedirectTo usage. So, developers will know that redirects to users' paths are not always safe.(please, do not take this issue before the 1st of September)
#documentation #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 `@sensitive_variables` decorator is missing on auth views (#1323)
We already have
We need to add vars that must not leak into the logs / error reporting middlewares / etc.
(please, do not take this issue before the 1st of September)
#bug #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
📝 `@sensitive_variables` decorator is missing on auth views (#1323)
We already have
@endpoint_decorator(sensitive_post_parameters()) on all API views that work with auth, but I forgot about sensitive_variables: https://docs.djangoproject.com/en/6.1/howto/error-reporting/#django.views.decorators.debug.sensitive_variablesWe need to add vars that must not leak into the logs / error reporting middlewares / etc.
(please, do not take this issue before the 1st of September)
#bug #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 The JWT blocklist is silently bypassed by tokens without a `jti` (#1322)
There are two problem with
1. We ignore cases where
2. We don't check that token is created with a valid
django-modern-rest/dmr/security/jwt/blocklist/auth.py
Lines 59 to 70 in a2d44b1
So, the logout path raises
We need to add
(please, do not take this issue before the 1st of September)
#bug #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
📝 The JWT blocklist is silently bypassed by tokens without a `jti` (#1322)
There are two problem with
jti and blocklist app:1. We ignore cases where
jti is None in the payload. None can't be found here: self.blocklist_model().objects.filter(jti=token.jti).exists(), so this check always passes. Moreover, it does not make sence to use tokens without jti and blocklist app2. We don't check that token is created with a valid
jti when blocklist app is used. We must do that, so this won't potentially fail on tokens with jti=None:django-modern-rest/dmr/security/jwt/blocklist/auth.py
Lines 59 to 70 in a2d44b1
So, the logout path raises
IntegrityError (HTTP 500) rather than a clean error.We need to add
['jti'] to self.require_claims with JWTokenBlocklistSyncMixin and JWTokenBlocklistAsyncMixin.(please, do not take this issue before the 1st of September)
#bug #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 `HttpBasicSyncAuth` and `HttpBasicAsyncAuth` must strictly configure `auth_scheme` prefix (#1330)
Currently HTTP-Basic auth is not strict about its
This would be a breaking change, I allow it.
Currently, we allow both header values:
django-modern-rest/dmr/security/http.py
Lines 56 to 76 in a2d44b1
We must make this configurable, as all other auth classes do. This must require
Also, we must update the OpenAPI description as well:
django-modern-rest/dmr/security/http.py
Lines 82 to 89 in a2d44b1
(please, do not take this issue before the 1st of September)
#feature #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
📝 `HttpBasicSyncAuth` and `HttpBasicAsyncAuth` must strictly configure `auth_scheme` prefix (#1330)
Currently HTTP-Basic auth is not strict about its
auth_scheme prefix.This would be a breaking change, I allow it.
Currently, we allow both header values:
some-login:password and Basic some-login:password. Which is not really cool:django-modern-rest/dmr/security/http.py
Lines 56 to 76 in a2d44b1
We must make this configurable, as all other auth classes do. This must require
auth_scheme: str = 'Basic' prefix by default.Also, we must update the OpenAPI description as well:
django-modern-rest/dmr/security/http.py
Lines 82 to 89 in a2d44b1
(please, do not take this issue before the 1st of September)
#feature #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Do not expose `reason` in `ensure_csrf` check (#1332)
Currently our responses show why CSRF check fails:
1.
django-modern-rest/dmr/internal/csrf.py
Line 12 in a2d44b1
2.
django-modern-rest/dmr/internal/csrf.py
Lines 35 to 44 in a2d44b1
It is even tested in some places as:
Which is not correct at all! We must only show this during the debug builds, not in production. Just like Django does:
1. https://github.com/django/django/blob/73cc09f14f13fedddc14d6ba5b287cb33c24e4a4/django/views/templates/csrf_403.html#L38-L46
2. https://github.com/django/django/blob/73cc09f14f13fedddc14d6ba5b287cb33c24e4a4/django/views/csrf.py#L22-L81
(please, do not take this issue before the 1st of September)
#bug #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
📝 Do not expose `reason` in `ensure_csrf` check (#1332)
Currently our responses show why CSRF check fails:
1.
django-modern-rest/dmr/internal/csrf.py
Line 12 in a2d44b1
2.
django-modern-rest/dmr/internal/csrf.py
Lines 35 to 44 in a2d44b1
It is even tested in some places as:
assert json.loads(response.content) == snapshot({
'detail': [
{
'msg': 'CSRF Failed: CSRF cookie not set.',
},
],
})
Which is not correct at all! We must only show this during the debug builds, not in production. Just like Django does:
1. https://github.com/django/django/blob/73cc09f14f13fedddc14d6ba5b287cb33c24e4a4/django/views/templates/csrf_403.html#L38-L46
2. https://github.com/django/django/blob/73cc09f14f13fedddc14d6ba5b287cb33c24e4a4/django/views/csrf.py#L22-L81
(please, do not take this issue before the 1st of September)
#bug #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 `leeway` / `iat` checks are not consistent in JWT token (#1324)
Currently we do double validation of
1.
django-modern-rest/dmr/security/jwt/token.py
Lines 97 to 104 in a2d44b1
Our way
2.
django-modern-rest/dmr/security/jwt/token.py
Lines 154 to 162 in a2d44b1
pyjwt way
The question is: do we really need to do this second validation here:
django-modern-rest/dmr/security/jwt/token.py
Line 257 in a2d44b1
Because it was just checked here:
django-modern-rest/dmr/security/jwt/token.py
Lines 236 to 242 in a2d44b1
Context:
•
This probably needs a rework to have a single source of truth. API breakage is allowed.
(please, do not take this issue before the 1st of September)
#bug #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
📝 `leeway` / `iat` checks are not consistent in JWT token (#1324)
Currently we do double validation of
leeway and iat for tokens:1.
django-modern-rest/dmr/security/jwt/token.py
Lines 97 to 104 in a2d44b1
Our way
2.
django-modern-rest/dmr/security/jwt/token.py
Lines 154 to 162 in a2d44b1
pyjwt way
The question is: do we really need to do this second validation here:
django-modern-rest/dmr/security/jwt/token.py
Line 257 in a2d44b1
Because it was just checked here:
django-modern-rest/dmr/security/jwt/token.py
Lines 236 to 242 in a2d44b1
Context:
•
_validate_iat in jwt/api_jwt.py in pyjwtThis probably needs a rework to have a single source of truth. API breakage is allowed.
(please, do not take this issue before the 1st of September)
#bug #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 `_HttpBasicAuth._get_username_and_password` must not unquote `%` chars (#1331)
Currently, we incorrectly use
django-modern-rest/dmr/security/http.py
Lines 60 to 76 in a2d44b1
This is a problem, because now these two passwords are the same:
But, the thing is that https://www.rfc-editor.org/info/rfc7617/ does not say that this unquoting is needed.
I just copied this errors from somewhere else. This needs to be fixed.
(please, do not take this issue before the 1st of September)
#bug #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
📝 `_HttpBasicAuth._get_username_and_password` must not unquote `%` chars (#1331)
Currently, we incorrectly use
unquote in the auth:django-modern-rest/dmr/security/http.py
Lines 60 to 76 in a2d44b1
This is a problem, because now these two passwords are the same:
>>> from base64 import b64decode, b64encode
>>> from urllib.parse import unquote
>>> unquote(b64decode(b64encode(b'test_%40')).decode())
'test_@'
>>> unquote(b64decode(b64encode(b'test_@')).decode())
'test_@'
But, the thing is that https://www.rfc-editor.org/info/rfc7617/ does not say that this unquoting is needed.
I just copied this errors from somewhere else. This needs to be fixed.
(please, do not take this issue before the 1st of September)
#bug #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 All pre-existing auth views must provide `Cache-Control: no-store` header (#1335)
Refs #1323
It should better be explicit. By default
(please, do not take this issue before the 1st of September)
#feature #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
📝 All pre-existing auth views must provide `Cache-Control: no-store` header (#1335)
Refs #1323
It should better be explicit. By default
POST methods are never cached anyway, but we should be better safe here.(please, do not take this issue before the 1st of September)
#feature #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Provide faster JWT decode and encode options with `msgspec` (#1390)
When
By default it uses the default
1. https://github.com/jpadilla/pyjwt/blob/7144e4534c34810f4525dc4578a32addd8212cff/jwt/api_jwt.py#L156-L172
2. https://github.com/jpadilla/pyjwt/blob/7144e4534c34810f4525dc4578a32addd8212cff/jwt/api_jwt.py#L287-L301
We can provide our own
This will require a benchmark test :)
#feature #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator
📝 Provide faster JWT decode and encode options with `msgspec` (#1390)
When
msgspec is installed, we can speed up the pyjwt encode and decode.By default it uses the default
json module for both convertions:1. https://github.com/jpadilla/pyjwt/blob/7144e4534c34810f4525dc4578a32addd8212cff/jwt/api_jwt.py#L156-L172
2. https://github.com/jpadilla/pyjwt/blob/7144e4534c34810f4525dc4578a32addd8212cff/jwt/api_jwt.py#L287-L301
We can provide our own
PyJWT subclass and make the process faster. Since JWT is a hot path, even a smaller boost will be a massive win overall.This will require a benchmark test :)
#feature #good_first_issue #help_wanted #security #opensource_september #django_modern_rest
sent via relator