chore: move resources and auth classes into dedicated files - #94
Conversation
PR SummaryMedium Risk Overview Twisted web resources ( Adds Reviewed by Cursor Bugbot for commit dc7562a. Bugbot is set up for automated code reviews on this repo. Configure here. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #94 +/- ##
==========================================
+ Coverage 72.05% 76.38% +4.33%
==========================================
Files 5 9 +4
Lines 773 792 +19
Branches 144 146 +2
==========================================
+ Hits 557 605 +48
+ Misses 157 131 -26
+ Partials 59 56 -3
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
| if auth_type == "basic": | ||
| username, password, *rest = args | ||
| if rest: | ||
| raise ValueError("BasicAuth expects username and password") | ||
| return BasicAuth(username=username, password=password) | ||
| if auth_type == "bearer": | ||
| token, *rest = args | ||
| if rest: | ||
| raise ValueError("BearerAuth expects a single token") | ||
| return BearerAuth(token=token) |
There was a problem hiding this comment.
As a curiosity, what if the config_model had the extra='forbid' and strict=True? Then wouldn't need to test for additional args, right? Can we put the nice error messages onto the base models, or does that need to stay here?
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Auth config no longer coerced
- Restored _coerce_http_auth in IntrospectionValidationConfig and NotifyOnRegistration post_init so YAML/dict auth is converted to HttpAuth before header_map() is called.
Or push these changes by commenting:
@cursor push a65e10029d
Preview (a65e10029d)
diff --git a/synapse_token_authenticator/config.py b/synapse_token_authenticator/config.py
--- a/synapse_token_authenticator/config.py
+++ b/synapse_token_authenticator/config.py
@@ -12,6 +12,7 @@
from synapse_token_authenticator.http_auth import (
HttpAuth,
NoAuth,
+ _coerce_http_auth,
)
@@ -107,6 +108,7 @@
def __post_init__(self):
if not isinstance(self.validator, Exist):
self.validator = parse_validator(self.validator)
+ self.auth = _coerce_http_auth(self.auth)
@dataclass
class NotifyOnRegistration:
@@ -114,6 +116,9 @@
auth: HttpAuth = field(default_factory=NoAuth)
interrupt_on_error: bool = True
+ def __post_init__(self):
+ self.auth = _coerce_http_auth(self.auth)
+
@dataclass
class OAuthConfig:
jwt_validation: JwtValidationConfig | None = NoneYou can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Dict auth ignores extra keys
- parse_dict_auth now constructs BasicAuth/BearerAuth with **value after popping type so extra=forbid rejects unknown dict keys, matching the previous behavior and list-form validation.
Or push these changes by commenting:
@cursor push 140bdee6bd
Preview (140bdee6bd)
diff --git a/synapse_token_authenticator/http_auth.py b/synapse_token_authenticator/http_auth.py
--- a/synapse_token_authenticator/http_auth.py
+++ b/synapse_token_authenticator/http_auth.py
@@ -47,9 +47,9 @@
if auth_type is None:
return NoAuth()
if auth_type == "basic":
- return BasicAuth(username=value["username"], password=value["password"])
+ return BasicAuth(**value)
if auth_type == "bearer":
- return BearerAuth(token=value["token"])
+ return BearerAuth(**value)
raise AuthValidationError(f"Unknown HttpAuth type {auth_type}")
diff --git a/tests/test_http_auth.py b/tests/test_http_auth.py
--- a/tests/test_http_auth.py
+++ b/tests/test_http_auth.py
@@ -44,9 +44,24 @@
assert e.value.args[0] == "HttpAuth missing type"
def test_parse_dict_auth_basic_missing_username(self):
- with pytest.raises(KeyError):
+ with pytest.raises(ValueError):
parse_dict_auth({"type": "basic", "password": "pass"})
+ def test_parse_dict_auth_basic_extra_fields(self):
+ with pytest.raises(ValueError):
+ parse_dict_auth(
+ {
+ "type": "basic",
+ "username": "user",
+ "password": "pass",
+ "extra": "field",
+ }
+ )
+
+ def test_parse_dict_auth_bearer_extra_fields(self):
+ with pytest.raises(ValueError):
+ parse_dict_auth({"type": "bearer", "token": "token", "extra": "field"})
+
def test_parse_dict_auth_unknown_http_auth_type(self):
with pytest.raises(ValueError) as e:
parse_dict_auth({"type": "unknown", "token": "token"})You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 1e33a76. Configure here.
2 participants
SYN-77