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

refactor: make security validations easier to reason about #830

Merged
merged 14 commits into from Nov 21, 2019

Conversation

XVincentX
Copy link
Contributor

@XVincentX XVincentX commented Nov 19, 2019

The following PR brings some significant simplification to the security checks that were made in Prism.

It literally took me ages to understand what was going on and why some choices were made so far — I guess this part definitely went out of control.

Hopefully this PR, although not 100% complete (but honestly I'm so tired of working on this section) will bring some clarity and order so that in the next flight refactor it one more time will be easier.

@XVincentX XVincentX force-pushed the feat/security-refactor branch 2 times, most recently from ebeb4a8 to c404d6c Compare November 19, 2019 19:25
@XVincentX XVincentX marked this pull request as ready for review November 20, 2019 17:08
Copy link
Contributor

@karol-maciaszek karol-maciaszek left a comment

Choose a reason for hiding this comment

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

I've put a question and naming change request

packages/http/src/validator/validators/security/index.ts Outdated Show resolved Hide resolved
packages/http/src/validator/validators/security/index.ts Outdated Show resolved Hide resolved
Co-Authored-By: Karol Maciaszek <karol@maciaszek.pl>
Copy link
Contributor

@karol-maciaszek karol-maciaszek left a comment

Choose a reason for hiding this comment

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

Perfetto!

Good job @XVincentX 👍

@XVincentX XVincentX merged commit a9eca6e into master Nov 21, 2019
@XVincentX XVincentX deleted the feat/security-refactor branch November 21, 2019 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants