OpenAPIGenerator upgrade to v7.16.0 - [4] Generate new models and services #1210
Conversation
Add extensions Fix templates (wrong copy)
Fix Namespaces for System.Net.Http
Remove method DefaultOrAsync on interface-level
Rename IApi to IAdyenApiService to match /services folder
…r in parameters - Make bodyparams mandatory even if marked as non-required Update tests Updated api_doc too
Summary of ChangesHello @Kwok-he-Chu, 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 focuses on a significant upgrade of the OpenAPI Generator to v7.16.0, which necessitated a complete regeneration and restructuring of Adyen's API models and services. The primary goal is to enhance the library's architecture by organizing generated code into more logical, API-specific folders. This change impacts numerous services, requiring extensive test migrations and the introduction of new clients and webhook handlers to support the updated structure and ensure full functionality and compatibility. Highlights
Ignored Files
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 a significant upgrade to the OpenAPI generator and refactors the project structure by generating new models and services. The changes include reorganizing API folders, migrating tests to use the new models, and adding extensive new test coverage. Overall, the changes are well-structured. However, I've identified a critical security issue in the HMAC signature validation logic where the HMAC key is not being used, which needs immediate attention. Additionally, there are a couple of medium-severity issues in the new test files related to code quality and best practices that should be addressed.
| public AcsWebhooksHandler(Adyen.AcsWebhooks.Client.JsonSerializerOptionsProvider jsonSerializerOptionsProvider, ITokenProvider<HmacKeyToken> hmacKeyProvider = null) | ||
| { | ||
| JsonSerializerOptionsProvider = jsonSerializerOptionsProvider; | ||
| } |
There was a problem hiding this comment.
The hmacKeyProvider is injected into the constructor but is not used to initialize the _adyenHmacKey field. As a result, _adyenHmacKey will always be null, and the HMAC signature validation in IsValidHmacSignature will not work correctly. This is a critical security vulnerability as it bypasses webhook signature validation.
public AcsWebhooksHandler(Adyen.AcsWebhooks.Client.JsonSerializerOptionsProvider jsonSerializerOptionsProvider, ITokenProvider<HmacKeyToken> hmacKeyProvider = null)
{
JsonSerializerOptionsProvider = jsonSerializerOptionsProvider;
_adyenHmacKey = hmacKeyProvider?.Get()?.AdyenHmacKey;
}There was a problem hiding this comment.
This is by design. The developer needs to call the function to validate the webhook public bool IsValidHmacSignature(string json, string hmacSignature)
If no HmacKey is provided, developers can still test this locally without having to validate the entire payload. If this was enforced, the class instantiation would throw an error on DI
A future improvement:
- Adding a
Middleware-class that takes the incoming webhook-
- Checks username+password or use OAuth
-
- Validates the hmacSignature using
IsValidHmacSignature
- Validates the hmacSignature using
-
- Use the
Deserialize{{WebhookName}}Request(string json)to get the object
- Use the
-
The feedback above would then be applicable, but right now all generated WebhookHandler-classes are considered as a utility function that the developers need to know about. @gcatanese
Removed ApiConstants.cs
…ersion for the LibName constant, but it should refer to AdyenLibraryName.
Remove unused templates ApiTests, DependencyInjectionTests #1208 (comment)
…Platform Configure methods
…//github.com/Adyen/adyen-dotnet-api-library into v7-generichost-branch-with-generated-classes
Kwok-he-Chu
deleted the branch
v7-generator/4-removed-old-Model-and-Service-folders
PR-4 of #1204 - Do not merge this branch - this is intended for review-only.
Description
This PR uses the new mustache to generate the new models and services.
This PR prepares the structure to be more in-line with the new library structure, grouping the APIs in one folder. Rather than having them separated across different folders.
(After)
/Adyen/+ {{ApiName}} +/Models/Adyen/+ {{ApiName}} +/ServicesExample:
/Adyen/Checkout/Modelsor/Adyen/BalancePlatform/Services(Before)
/Adyen/Model+ {{ApiName}}/Adyen/Service+ {{ApiName}}Example:
Adyen/Model/CheckoutMigrated all tests to use the new models, added tests accordingly
Added
/docsin the/docs-folder for each {{ApiName}}Fix #1211