GH-125 event_webhook module for webhook sign verification

adobe / aio-lib-java

Adobe I/O - Java SDK

https://opensource.adobe.com/aio-lib-java/

Apache License 2.0

6 stars 18 forks source link

GH-125 event_webhook module for webhook sign verification #118

Closed abhupadh closed 1 year ago

abhupadh commented 2 years ago

Description

adds new module events_webhook which has the I/O Events webhook signature verification method

Related Issues

GH-125

Motivation and Context

Helps I/O Events Customers to setup their webhook verification using this sdk feature.

How Has This Been Tested?

Has the test EventVerifierTest which tests the api for verifying signature.

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] I have signed the Adobe Open Source CLA.
[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[x] I have read the CONTRIBUTING document.
[x] I have added tests to cover my changes.

francoisledroff commented 2 years ago

@abhupadh thanks for the PR. Let's make this simpler, let's get rid of the retrofit and caffeine dependencies. we could implement in-memory cache with just a hash map. for http related activites, please use either the plain java sdk (https://docs.oracle.com/en/java/javase/11/docs/api/java.net.http/java/net/http/package-summary.html) or eventually openFeign as it is already a dependency of this sdk.

abhupadh commented 2 years ago

okay, will make the changes

abhupadh commented 2 years ago

Could we get rid of the caffeine dependency ? and use a Hashmap instead if you strongly feel there is a need for a cache ? could you also push your change your PR an push to a main remote https://github.com/adobe/aio-lib-java repo branch so the integration test can be ran ?

Yes, I removed the caffeine dependency and used a hashmap instead. Also, removed the retrofit with open feign. Check my latest commits. I have these tests failing which I need to fix. So, for that I think you mentioned to push this change to main branch?

francoisledroff commented 2 years ago

Could we get rid of the caffeine dependency ? and use a Hashmap instead if you strongly feel there is a need for a cache ? could you also push your change your PR an push to a main remote https://github.com/adobe/aio-lib-java repo branch so the integration test can be ran ?

Yes, I removed the caffeine dependency and used a hashmap instead. Also, removed the retrofit with open feign. Check my latest commits. I have these tests failing which I need to fix. So, for that I think you mentioned to push this change to main branch?

I guess you forgot to push, I still see caffeine Utils and dependencies ... yes to be able to run the integration test you will need to have the PR branch pushed to this origin repo

francoisledroff commented 1 year ago

I submitted a PR against your branch https://github.com/abhupadh/aio-lib-java/pull/1 showing you the way please continue, polishing it and fixing the unit test

abhupadh commented 1 year ago

@francoisledroff : the integration tests are failing for the ims module. not sure why, as no changes are made there in this PR

francoisledroff commented 1 year ago

please use the PR I did against your branch see https://github.com/abhupadh/aio-lib-java/pull/1 showing you the way, please continue, polishing it and fixing the unit test ... Let's not have any spring dependency, nor cache, IMHO it's up to the customer to decide how to implement cache and whether or not to use spring ....

abhupadh commented 1 year ago

This is not only used by customers but our own eg-auditor as well.

abhupadh commented 1 year ago

closing in favor of this enhanced PR https://github.com/adobe/aio-lib-java/pull/144

francoisledroff commented 1 year ago

closing this as it was replaced by https://github.com/adobe/aio-lib-java/pull/144