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

Mask fields when request is string #102

Merged
merged 1 commit into from
Jul 26, 2022
Merged

Conversation

rabinshr
Copy link
Contributor

Description
Mask sensitive fields when request is a string. The only times I have managed to see request come in as a string is when Adyen web service user has a missing permission.
image

Due to the fact that we parse request string into a hash inside of the mask_fields method, the current code doesn't modify the original request and it preserves the original request data without masking any of the fields.

Additionally, while parsing it wasn't symbolising the name which meant, further down we were comparing string with symbols here

Tested scenarios

  • screenshot showing request is actually masked even when it is a string
    image
  • added unit tests to cover request is a string scenario

Fixed issue:

@crrood
Copy link
Contributor

crrood commented Jul 19, 2022

Hi Rabin,

Makes sense to me! Thanks for the PR - approved.

Best,
Colin

@rabinshr
Copy link
Contributor Author

@wboereboom , any chance you could give this a look?

@rabinshr
Copy link
Contributor Author

@wboereboom any chance you could have a look at this?

@wboereboom
Copy link
Contributor

Hi @rabinshr,

Thanks for contributing!
I was Out Of Office for a bit, hence my lack of response 😉 .

Looks good to me, so I'm approving and merging!

Kind Regards,
Wouter
Adyen

@wboereboom wboereboom merged commit cef2497 into Adyen:develop Jul 26, 2022
@acampos1916 acampos1916 mentioned this pull request Jul 27, 2022
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.

3 participants