Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move all signature checks into SignatureValidator... #7

Closed
wants to merge 3 commits into from

Conversation

leeovery
Copy link

This enables user to skip signature validation completely via custom validator. Simply return true from isValid method and it's skipped.

Moved signature exist check into protected method to give option of extending default validator and reusing functionality.

I didnt really see it necessary to add any new tests this time around. All tests are still passing and if Im honest this was my assumption on behaviour anyway. So I think we're good :)

@@ -50,7 +50,7 @@ public function it_validates_the_webhook_profile()
}

/** @test */
public function it_validates_the_process_webhook_ojb()
public function it_validates_the_process_webhook_jpb()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost... 😀

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha!! Its a complex word!

@leeovery
Copy link
Author

Is that issue an issue for merging? lol

Seriously though...

@freekmurze
Copy link
Member

I'm a little bit short on time, but I'll be sure to review this PR in coming days or weeks.

@freekmurze
Copy link
Member

I've decided against merging this in for now, because it could be breaking change for people that are using a custom validator.

I'll revisit this when creating a new major version.

@freekmurze freekmurze closed this Jul 1, 2019
@freekmurze freekmurze reopened this Jul 8, 2019
@freekmurze
Copy link
Member

freekmurze commented Jul 8, 2019

I've implemented this myself in a clean way. It will be available in v2 which will be released soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants