GH-125 event_webhook module for webhook sign verification #118

abhupadh · 2022-06-23T21:44:15Z

Description

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

Related Issues

events_webhook add a webhook signature verification utility #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)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

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

francoisledroff · 2022-07-01T08:41:11Z

@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 · 2022-07-19T02:57:52Z

okay, will make the changes

francoisledroff

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 ?

abhupadh · 2022-09-02T01:11:59Z

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 · 2022-09-07T12:30:06Z

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

pom.xml

francoisledroff · 2022-09-15T12:15:36Z

I submitted a PR against your branch abhupadh#1 showing you the way
please continue, polishing it and fixing the unit test

…ofit and unwanted dependecies

events_webhook/src/main/java/com/adobe/aio/event/webhook/service/EventVerifier.java

abhupadh · 2022-12-02T08:25:11Z

events_webhook/src/main/java/com/adobe/aio/event/webhook/service/EventVerifier.java


 @Service
 public class EventVerifier {



@Service / component annotation is required so that it can be auto injected in eg-auditor's -> EventServiceImpl as here in this PR

If this is the only reason that we need a @service/ @component annotation, I don't see it as a compelling reason to include the Spring framework dependency.
Why can't we simply create an instance of EventVerifier in eg-auditor?

abhupadh · 2022-12-02T08:30:32Z

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

francoisledroff · 2022-12-05T09:22:36Z

please use the PR I did against your branch see abhupadh#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 · 2022-12-05T18:08:35Z

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

francoisledroff · 2022-12-06T10:04:10Z

events_webhook/pom.xml

+
+    <dependency>
+      <groupId>com.google.code.findbugs</groupId>
+      <artifactId>jsr305</artifactId>


we need to remove this

francoisledroff · 2022-12-06T10:06:30Z

core/pom.xml

      <groupId>io.github.openfeign</groupId>
      <artifactId>feign-core</artifactId>
    </dependency>
+


let's revert this unnecessary change of core/pom.xml

francoisledroff · 2022-12-06T10:10:16Z

events_webhook/src/main/java/com/adobe/aio/event/webhook/service/EventVerifier.java

+
+  }
+
+  public EventVerifier() {


your current implementation is forcing the use of your cache .
I would rather just see in this PR just a fully reusable public static method that helps you verify the signature.
as documented here https://developer.adobe.com/events/docs/guides/#security-considerations

events_webhook/src/main/java/com/adobe/aio/event/webhook/service/EventVerifier.java

francoisledroff · 2022-12-06T12:25:06Z

events_webhook/src/main/java/com/adobe/aio/event/webhook/cache/CacheService.java

+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+public interface CacheService {


I would really like to get rid of this cache.
our customer can easily use any cache service/framework they want.
this PR should only be about showing how to validate the message headers' signature and no more

francoisledroff · 2022-12-06T12:28:36Z

pom.xml

        <artifactId>feign-form</artifactId>
        <version>${feign-form.version}</version>
      </dependency>
+      <dependency>


let's remove these unwanted spring and findbugs dependencies

abhupadh · 2023-01-31T21:15:09Z

closing in favor of this enhanced PR #144

francoisledroff · 2023-01-31T21:48:14Z

closing this as it was replaced by #144

abhupadh added 4 commits June 23, 2022 14:37

events_webhook module with signature verification feature and test

15c8edd

modified core, webhook and main pom, events_webhook module classes

e9aaf18

missing return statement

75a5678

adding EventVerifierBuilder

f0d83a1

abhupadh added 5 commits August 23, 2022 17:23

refactoring event verifier, using map cache, open fiegn for http call

914e0e7

adding api, feign service, cache, model

cd94980

updating test

1f64507

updating poms

01f11d5

minor refactoring

2af9561

francoisledroff requested changes Sep 1, 2022

View reviewed changes

francoisledroff reviewed Sep 15, 2022

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

francoisledroff reviewed Sep 15, 2022

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

francoisledroff changed the title ~~event_webhook module for webhook sign verification~~ GH-125 event_webhook module for webhook sign verification Nov 22, 2022

Merge branch 'main' into CI-5472

2b59c46

abhupadh temporarily deployed to aio_stage December 1, 2022 01:42 Inactive

abhupadh temporarily deployed to aio_prod December 1, 2022 01:42 Inactive

abhupadh added 4 commits December 2, 2022 00:19

removing caffeine, retrofit, other unwanted dependencies

8fa9f04

refactoring hashmap used cache service

6d5b8bb

refactoring event verifier and its test

a2f77f6

updating main pom with required dependencies, removing caffiene, retr…

5d1859d

…ofit and unwanted dependecies

abhupadh temporarily deployed to aio_prod December 2, 2022 08:21 Inactive

abhupadh temporarily deployed to aio_stage December 2, 2022 08:21 Inactive

abhupadh commented Dec 2, 2022

View reviewed changes

events_webhook/src/main/java/com/adobe/aio/event/webhook/service/EventVerifier.java Show resolved Hide resolved

abhupadh commented Dec 2, 2022

View reviewed changes

removing commented sentences

250cc7f

abhupadh temporarily deployed to aio_prod December 2, 2022 08:43 Inactive

abhupadh temporarily deployed to aio_stage December 2, 2022 08:43 Inactive

abhupadh added 2 commits December 5, 2022 15:00

removing spring context dependency

df90b84

removing Service annotation, minor refactoring

2bdc23e

abhupadh temporarily deployed to aio_prod December 5, 2022 23:01 Inactive

abhupadh temporarily deployed to aio_stage December 5, 2022 23:01 Inactive

francoisledroff requested changes Dec 6, 2022

View reviewed changes

francoisledroff closed this Jan 31, 2023

GH-125 event_webhook module for webhook sign verification #118

GH-125 event_webhook module for webhook sign verification #118

Uh oh!

Conversation

abhupadh commented Jun 23, 2022 • edited by francoisledroff Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

francoisledroff commented Jul 1, 2022

Uh oh!

abhupadh commented Jul 19, 2022

Uh oh!

francoisledroff left a comment

Choose a reason for hiding this comment

Uh oh!

abhupadh commented Sep 2, 2022

Uh oh!

francoisledroff commented Sep 7, 2022

Uh oh!

Uh oh!

Uh oh!

francoisledroff commented Sep 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

abhupadh Dec 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shikhartanwar Dec 5, 2022

Choose a reason for hiding this comment

Uh oh!

abhupadh commented Dec 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

francoisledroff commented Dec 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abhupadh commented Dec 5, 2022

Uh oh!

francoisledroff Dec 6, 2022

Choose a reason for hiding this comment

Uh oh!

francoisledroff Dec 6, 2022

Choose a reason for hiding this comment

Uh oh!

francoisledroff Dec 6, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

francoisledroff Dec 6, 2022

Choose a reason for hiding this comment

Uh oh!

francoisledroff Dec 6, 2022

Choose a reason for hiding this comment

Uh oh!

abhupadh commented Jan 31, 2023

Uh oh!

francoisledroff commented Jan 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

abhupadh commented Jun 23, 2022 •

edited by francoisledroff

Loading

francoisledroff commented Sep 15, 2022 •

edited

Loading

abhupadh Dec 2, 2022 •

edited

Loading

abhupadh commented Dec 2, 2022 •

edited

Loading

francoisledroff commented Dec 5, 2022 •

edited

Loading