🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Implement "Breaking changes" detector (#912)


Since we have a pretty good OpenAPI scheme, it allows us to DO THINGS 😆

And one of the imporant things it allows us to do is to find breaking API changes.
So, here's how I plan to implement this:

1. We would need #909 first
2. Next, we can write a set of test cases that would run on the old schema, but with the new code
3. This way it can detect that clients following the old schema - won't get the expected results back
4. These tests can surely be run with schemathesis
5. We would need to document this and document the setup
6. Maybe provide some tooling, except #909

And we should also list static schema diff tools and explain what is the difference.
(Which is that some schema changes are really hard to find statically, because semantics is also important).

CC @Stranger6667


#feature #good_first_issue #help_wanted #opensource_september #django_modern_rest
sent via relator

🚀 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 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
📝 Refactor `dmr/security/jwt/auth.py` and `dmr/security/jwt/cookie.py` (#1321)


Currently we store JWT's auth as:

• dmr/security/jwt/auth.py for headers
• dmr/security/jwt/cookie.py for cookies

But, Token auth uses:

• dmr/security/token/auth/header.py
• dmr/security/token/auth/cookie.py

Which is better. We need to refactor the JWT layout to be the same.

(please, do not take this issue before the 1st of September)


#good_first_issue #help_wanted #python #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.

>>> 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 @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_variables

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

🚀 New issue to wemake-services/django-modern-rest by @kondratevdev
📝 Strict boolean `Query` component cannot parse valid OpenAPI boolean values (#1325)


What's wrong?

Boolean query parameters cannot be parsed correctly when using Pydantic strict validation, even though DMR generates a valid OpenAPI schema for them.

For example:

from pydantic import BaseModel, StrictBool

class ProjectsQuery(BaseModel):
    random: StrictBool = False

DMR generates the expected OpenAPI schema:

- name: random
  in: query
  schema:
    type: boolean
    default: false

However, a valid request:

GET /projects?random=false

is rejected with 400 Bad Request:

{
  "detail": [
    {
      "msg": "Input should be a valid boolean",
      "loc": ["parsed_query", "random"],
      "type": "value_error"
    }
  ]
}

Schemathesis detects this as a schema-compliant request being rejected:

API rejected schema-compliant request

Valid data should have been accepted
Expected: 2xx, 401, 403, 404, 409, 5xx
[400] Bad Request Reproduce with: curl -X GET --insecure \ 'http://localhost/api/projects/projects/?random=false'

Why not use a regular bool?

A regular Pydantic bool successfully parses query parameters:

class ProjectsQuery(BaseModel):
    random: bool = False

So these work as expected:

?random=true   -> True
?random=false  -> False

However, Pydantic's non-strict boolean parsing also accepts other representations:

?random=1      -> True
?random=0      -> False

This makes the actual API validation more permissive than the generated OpenAPI schema.

How it should be?

Maybe it should be possible to use strict boolean validation for query parameters while still accepting their valid HTTP/OpenAPI representation?

Used versions

0.14.0

OS information

Not important for this case

(please, do not take this issue before the 1st of September)


#bug #good_first_issue #help_wanted #openapi #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 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 app
2. 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
📝 Make sure that `SSE` validation for events work correctly, including custom types and `validate_events=False` (#1329)


django-modern-rest/dmr/streaming/sse/metadata.py

Lines 169 to 175 in a2d44b1

Currently we have two problems:

1. Even when validate_events=False, we still validate that id and event fields for SSEvent does not contain NULL char and does not contain multiline strings. Which is not really cool. Why? Beceause we slow things down in production, when users explicitly ask us not to. We need to move the validation somewhere else. I propose moving this login into the validator or renderer. But, it must respect the setting. Even if some field is not valid in production, it must not be validated if validate_events=False
2. Currently custom SSE event types are not validated the same way. It would be automatically solved, when 1. is fixed. We would just need more tests for this :)

(please, do not take this issue before the 1st of September)


#bug #good_first_issue #help_wanted #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 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:

    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
📝 `BytesIO(request.body)` in `parse_as_post` copy the `request.body` object (#1328)


django-modern-rest/dmr/internal/django.py

Lines 136 to 145 in a2d44b1

This is not really cool, we would copy the request data twice. In some cases this can be really bad, because files can be rather big.

We need to find a way not to copy the data.

(please, do not take this issue before the 1st of September)


#bug #good_first_issue #help_wanted #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 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 pyjwt

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

🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 `_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
📝 `token_secret` is assigned twice in `_BaseTokenAuth.__init__` (#1338)


django-modern-rest/dmr/security/token/auth/base.py

Lines 83 to 86 in a2d44b1

This is just a typo. No other changes are required.

(please, do not take this issue before the 1st of September)


#bug #good_first_issue #help_wanted #opensource_september #django_modern_rest
sent via relator

🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Document that it is a good idea to remove expired tokens from `BlocklistedJWToken` (#1336)


Since they are expired anyway, they won't be usable for the auth.

So, there's no point in storing them.
The same can be said about Token model from dmr/security/token/app/models.py.

Document that adding a cron-job to remove old ones is probably worth it.
Show examples, what we can remove in both cases.

(please, do not take this issue before the 1st of September)


#documentation #good_first_issue #help_wanted #opensource_september #django_modern_rest
sent via relator

🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Incorrect empty response check in `_check_empty_response_body` (#1340)


django-modern-rest/dmr/validation/endpoint_metadata.py

Lines 151 to 160 in a2d44b1

There are several things that are broken here:

1. response.status_code < HTTPStatus.CONTINUE must be response.status_code < HTTPStatus.OK, because there are no status codes less than 100. See RFC 9110
2. 205 Reset Content must be in the no-body set: https://www.rfc-editor.org/rfc/rfc9110.html#name-205-reset-content
3. The set should not be created on each call, create a frozenset per-class once
4. HEAD methods must not return a body. This needs an extra rule

(please, do not take this issue before the 1st of September)


#bug #good_first_issue #help_wanted #opensource_september #django_modern_rest
sent via relator

🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Support `WWW-Authenticate` header for auth classes (#1334)


https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/WWW-Authenticate
RFC: https://www.rfc-editor.org/rfc/rfc9110.html#section-15.5.2

Quote:

The server generating a 401 response MUST send a WWW-Authenticate header field (Section 11.6.1) containing at least one challenge applicable to the target resource.
If the request included authentication credentials, then the 401 response indicates that authorization has been refused for those credentials. The user agent MAY repeat the request with a new or replaced Authorization header field (Section 11.6.2). If the 401 response contains the same challenge as the prior response, and the user agent has already attempted authentication at least once, then the user agent SHOULD present the enclosed representation to the user, since it usually contains relevant diagnostic information.

Currently, we don't support this. But, we probably should.
Ideas on how to support this are welcome.

Criteria:

1. All auth classes should be supported, if some can't be supported, we need to figure out - why
2. It should be documented as a feature and what is supported
3. There should be a way to disable this (probably?)

#feature #good_first_issue #help_wanted #opensource_september #django_modern_rest
sent via relator

🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 `SimpleRate` lua script has missing `nil` convert (#1333)


This check:

django-modern-rest/dmr/throttling/lua.py

Lines 28 to 40 in a2d44b1

Contain missing nil cast to 0:

django-modern-rest/dmr/throttling/lua.py

Line 39 in a2d44b1

It must be tonumber(redis.call("GET", key)) or 0
We must test this case as well.


#bug #good_first_issue #help_wanted #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 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
📝 Change how `attrs` extra is defined (#1342)


django-modern-rest/pyproject.toml

Lines 69 to 70 in a2d44b1

Currently we define msgspec twice, but we should replace the second entry with django-modern-rest[msgspec]
(please, do not take this issue before the 1st of September)


#feature #good_first_issue #help_wanted #dependencies #pythonuv #opensource_september #django_modern_rest
sent via relator