Syk/refactor config - #101
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #101 +/- ##
==========================================
+ Coverage 76.43% 77.71% +1.27%
==========================================
Files 9 13 +4
Lines 764 875 +111
Branches 136 157 +21
==========================================
+ Hits 584 680 +96
- Misses 121 123 +2
- Partials 59 72 +13
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
FrenchGithubUser
left a comment
FrenchGithubUser
left a comment
There was a problem hiding this comment.
looks good, just a few notes:
you should also change the call sites like if (oidc := getattr(self.config, "oidc", None)) is not None: now as it's written in the jira ticket
and can also use typing in __init__.py with something like:
class TokenAuthenticatorConfig:
"""
Parses and validates the provided config dictionary.
"""
jwt: JwtConfig | None
oidc: OIDCConfig | None
oauth: OAuthConfig | None
epa: EPaConfig | None
def __init__(self, other: dict):
self.jwt = JwtConfig(**jwt) if (jwt := other.get("jwt")) else None
self.oidc = OIDCConfig(**oidc) if (oidc := other.get("oidc")) else None
self.oauth = OAuthConfig(**oauth) if (oauth := other.get("oauth")) else None
self.epa = EPaConfig(**epa) if (epa := other.get("epa")) else None
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b3ec458. Configure here.
jason-famedly
left a comment
jason-famedly
left a comment
There was a problem hiding this comment.
Ok, this was quite large which can't be helped. I still have a few lingering questions about getting rid of a few more of the Anys running around. I believe they are part of the Pydantic validation and should not be too much of a concern. Only other concern I have is out-of-scope: What happens when a InvalidJWKValue is raised? Lets explore this another day as it would have existed before.
LGTM! 🚀
|
(and don't forget to squash!) |
chore: update epa config chore: update jwt config to dataclass chore: update oidc config as dataclass chore: update oauth config chore: update comments chore: update expose_metadata_resource to dict
8e4cdfd to
12545c3
Compare
3 participants
SYN-95
Note
Medium Risk
Changes how Synapse module config is parsed and validated at startup and tightens metadata-resource shape; misconfiguration may now surface as validation errors, though auth logic is largely the same with added guards.
Overview
Replaces the single
config.pywith asynapse_token_authenticator/config/package where JWT, OIDC, OAuth, and EPA settings are Pydantic dataclass models with structured validation (JWK sources, algorithms, OAuth jwt/introspection requirements, etc.).TokenAuthenticatorConfignow skips missing or explicitnullYAML sections and instantiates only the enabled blocks.OAuth/EPA
expose_metadata_resourceis now a dict that must includename; invalid metadata objects fail at config load. OIDCallowed_client_idsaccepts a list or space-separated string; numericproject_id/organization_idare coerced to strings. Unknown config keys are ignored (extra="ignore") to reduce breakage.TokenAuthenticatorreads typed config attributes directly, returns early when a flow is not configured, and_verify_jwtrejects invalid key types instead of relying on asserts. README documents the metadata object shape. Large unit test coverage added undertests/test_config/plus OIDC client allowlist and EPA JWKS-set scenarios.Reviewed by Cursor Bugbot for commit 5a91af6. Bugbot is set up for automated code reviews on this repo. Configure here.