Находки в опенсорсе: Python
1.06K subscribers
5 photos
332 links
Легкие задачки в опенсорсе из мира Python

Чат: @opensource_findings_chat
Download Telegram
🚀 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
🚀 New issue to wemake-services/django-modern-rest by @vyhuholl
📝 `JWToken.encode` raises `InternalServerError`, an HTTP-layer exception (#1364)



#bug #good_first_issue #help_wanted #opensource_september #django_modern_rest
sent via relator
🚀 New issue to ag2ai/faststream by @davorrunje
📝 Docs: dealing with different schema registries (#1297)


It would be good to add documentation with examples how to deal with different schema registries. Again there are many registries and coupling router with a particular registry isn't a good idea, unless there will be a some Abstract class first, so later community can add implementation


#documentation #enhancement #good_first_issue #confluent #faststream #ag2ai
sent via relator
🚀 New issue to wemake-services/wemake-python-styleguide by @Novohudonossor
📝 tstring_same_prefix params contain f-strings — the t-string coverage from #3702 is never exercised (#3793)


What's wrong

Two test constants touched by #3702 ("extend f-string rules to t-strings") do not exercise t-strings at all.

1. tests/test_visitors/test_ast/test_complexity/test_overuses/test_overused_string.py:112-131

tstring_same_prefix1 = pytest.param(
"""
x = f'Hello, {pattern}'
y = f'Hello, {pattern}'
""",
marks=pytest.mark.skipif(not PY314, reason='t-strings are only in Python 3.14+'),
)

The snippet is identical to fstring_same_prefix1 right above it. Below 3.14 the param is skipped by the mark; on 3.14+ it runs and asserts on f-strings. Either way WPS226 is never checked against a t-string, and the suite stays green.

2. tests/test_visitors/test_tokenize/test_comments/test_comment_in_formatted_string312.py:87

PREFIXES is ['f', 't'] and both tests run code.format(prefix), so every constant is exercised twice. rfstring_with_comment_triple_single_quotes hardcodes rf'''…''' instead of r{0}'''…''' like its two siblings above it, so the t round re-tests rf.

How it should be
1. t'Hello, {pattern}' and t'{pattern}-postfix' inside the two tstring_same_prefix* params.
2. r{0}'''test{{a # comment\n}}''', matching rfstring_with_comment_single_quotes and rfstring_with_comment_triple_quotes.

Happy to open a PR for both if that is useful.

Flake8 version and plugins / pip information / OS information

Not applicable — this is not a runtime report. Found by reading the tests, verified against HEAD 8ebc607; there is no flake8 --bug-report output to paste.

Provenance

Both came out of a review run with ReviewGate (https://reviewgate.dev), a local review gate I build, over 76b3ac3~1..c580fc3. The run was local, on my own model key, and nothing left my machine.


#bug #help_wanted #levelstarter #good_first_issue #wemake_python_styleguide #wps
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 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
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Cache OpenAPI views schema generation (#1398)


Currently, our OpenAPI views are really slow, because they recompute the schema on each request. Which is a bad thing if this schema is publicly available and has a lot of traffic.

Example:

django-modern-rest/dmr/openapi/views/json.py

Lines 28 to 30 in 06626f9

Proposal: we need to cache the schema generation. Not the view itself, because it can be cached by the user via a decorator. This might go to the docs as an example.

@cachedproperty seems like the best solution here.


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