feat: add apsig's rfc9421 support #47
Conversation
for more information, see https://pre-commit.ci
Summary of ChangesHello @AmaseCocoa, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the system's HTTP signature capabilities by adding full support for RFC9421 (Structured Fields) signatures. This enhancement improves the security and standardization of HTTP request and response handling, particularly for ActivityPub interactions. It introduces new signing and verification components, refines header management, and updates the overall signature verification workflow to incorporate the new standard while maintaining compatibility with existing draft specifications. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive support for RFC9421 HTTP signatures, adding new RFC9421Signer and RFC9421Verifier classes to handle signing and verification of requests. The .gitignore file was updated to broadly ignore private key files and a new devel/ directory. The project's version was incremented to 0.3.3. The _common.py module saw significant changes, including renaming ensure_user_agent_and_reconstruct to reconstruct_headers and enhancing it to automatically set Content-Type based on the JSON body. The sign_request function was updated to integrate RFC9421 signing, with warnings added to manage exclusivity between RFC9421 and draft-cavage signing. The inbox.py module was extended with a new __verify_rfc9421 method for RFC9421 verification, which includes public key caching, and the main verify method was updated to prioritize RFC9421 verification. Throughout the client modules (asyncio/actor.py, asyncio/client.py, sync/actor.py, sync/client.py), calls to the renamed header reconstruction function were updated. Review comments highlight issues in rfc9421.py, specifically an incomplete list of supported algorithms in the verifier's alg check, duplicated logic for p256-pub and p384-pub key handling, and a redundant type annotation. Additionally, a commented-out code block in _common.py was noted for removal, and a broad except Exception in inbox.py was flagged for being too general, suggesting more specific exception handling.
| return cast(Tuple[ItemType | InnerListType, ParamsType], (value, params)) | ||
|
|
||
| def __generate_sig_input( |
There was a problem hiding this comment.
The type annotation here is redundant and also incomplete, as it's missing ec.EllipticCurvePublicKey. The type for self.public_key is already declared on lines 135-139. You can simplify this by just assigning the value.
self.public_key = public_key
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
apsig 0.6.0 is not released yet, this pr merged after release