search

bunq / sdk_csharp

C# SDK for bunq API
MIT License
35 stars 22 forks source link

Add header verification test  #60

Open OGKevin opened 6 years ago

OGKevin commented 6 years ago

Steps to reproduce:

  1. Response verification should be tested,

What should happen:

  1. Response verification is not tested.

What happens:

  1. Response verification is tested.

Logs

Extra info:

References

kid-cavaquinho commented 6 years ago

@OGKevin what type of tests do you have in mind for it? Unit tests on the level of SecurityUtils.cs, for example? Contract/collaboration tests on ApiClient?

OGKevin commented 6 years ago

@antao My initial thought was to mock the returned value of https://github.com/bunq/sdk_csharp/blob/c71114b58d95293be99416f13e920c45f28d69c8/BunqSdk/Http/ApiClient.cs#L154

and then change the x-bunq-header to different caps and ensure that signature verification still works by setting the casing correctly. And another test that changes the value of x-bunq-header and ensure that signature validation fails.

Eventually, we can save save an actual TResult(The type that is returned by the line mentioned above) to a file and create the mock object from the contents saved to that file. And then do the header manipulation from there before it touches the validation code.

kid-cavaquinho commented 6 years ago

@OGKevin perfect. I can pick it. Incoming PR.

kid-cavaquinho commented 6 years ago

@OGKevin I cannot run the current setup of tests. The endpoint and ApiContext tests throw null reference exceptions. Unfortunately I do not have an API key, is it required? Am I missing some configuration file to be able to run these? Thanks in advance.

OGKevin commented 6 years ago

@antao yes, it would be ideally that we mock all the requests instead of actually making them! This way we can also use something like Travis to run tests 👍. But that would be a future refactor.

You indeed need an API key for the SANDBOX environment. You can use this key pair, because its sandbox, there is no harm in sharing it publicly, but _don't do this with your production credentials!

"apiKey": "9807ed7aaded6e7026ff46440b09a9d42db81f1eb107e7db93586df3fbef2122"

"docKey": "39ae97a6-c704-4095-bee2-32d9d3c936ee"

You can place the doc key here: https://doc.bunq.com/settings and you will be presented with 3 users you can use to login into the sandbox bunq app which is located here: https://doc.bunq.com/api/1/page/android-emulator-setup

Afterwards, fill in the configuration file with data: https://github.com/bunq/sdk_csharp/blob/develop/BunqSdk.Tests/README.md#configuration

kid-cavaquinho commented 6 years ago

Aww cool @OGKevin thanks for the input. Won't be using it on PROD for the time being. I am a bunq user because I believe in the same ideas behind it and even tough I some ideas for apps, for the time being I just want to contribute 😺

Yes, automation execution of the tests is desirable, with Travis or insert favourite ci/cd tool here. I've been using appveyor for my simple pet projects at home, it is quite simple and does the job effectively.

I will try it tomorrow as I am a bit busy today. I will give you feedback. Bedankt.

kid-cavaquinho commented 6 years ago

@OGKevin thanks for the update. The keys you provided helped me running the tests locally! 👍

Regarding the issue itself, do you pretend to change the design of ApiClient, by let's say... inject the HttpClient via the ctor? Or having a WrapperClass with the HttpClient dependency so that it can be mocked then in the tests? Also which mocking framework do you favour?

Thanks!

OGKevin commented 6 years ago

Hey @antao I think I missed this comment 😓.

I haven't done any research yet regarding how I want to implemented this. I also want to mock all the requests to not make actual requests to our API, but instead read the response from a JSON file. Something like https://github.com/bunq/sdk_csharp/blob/9b673328783515d47a6ece507fd7adafe086e0bb/BunqSdk.Tests/Model/Generated/Object/NotificationUrlTest.cs#L13 does but then higher in the code. Somewhere around https://github.com/bunq/sdk_csharp/blob/9b673328783515d47a6ece507fd7adafe086e0bb/BunqSdk/Http/ApiClient.cs#L162 or lower but ill create a separate issue for this.

For this particular issue, you might list suggestions on how you feel like implementing this and we can discuss what would be the best solution 👍 .