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

Fixed hide_response body (#370) #375

Merged
merged 1 commit into from May 27, 2021
Merged

Conversation

hebertjulio
Copy link
Member

@hebertjulio hebertjulio commented May 25, 2021

Closes #370
Closes #371

@hebertjulio hebertjulio requested review from a team as code owners May 25, 2021 17:59
Copy link
Member

@camilamaia camilamaia left a comment

Choose a reason for hiding this comment

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

Hey, @hebertjulio, would you mind creating new tests for the changes you've made? I've just created a detailed doc of how to do it:

Please, let me know if you need any help!

@hebertjulio
Copy link
Member Author

Of course @camilamaia I can do it, but I don't know which test can be created, at the moment I am learning to create tests, and I have difficulties in defining the tests that need to be created.

@camilamaia
Copy link
Member

Of course @camilamaia I can do it, but I don't know which test can be created, at the moment I am learning to create tests, and I have difficulties in defining the tests that need to be created.

No problem, I can help you :) I would create:

  • one test to check that it uses .body when there is a body. And it overrides the body information.
  • one test to check that it uses .content when the body is empty. And it hides the content information.
  • one test to check the code does not break when there is no .body nor .content.

This is the file https://github.com/scanapi/scanapi/blob/master/tests/unit/test_hide_utils.py where you should add the tests.

Let me know how it goes!

@hebertjulio
Copy link
Member Author

hebertjulio commented May 26, 2021

Hi @camilamaia, i don't understand why there are two gets method in line 8 of test_hide_utils.py file. is that correct?

def response(requests_mock):
requests_mock.get("http://test.com", text="data")
return requests.get("http://test.com")

@camilamaia
Copy link
Member

@hebertjulio yes, this is mocking the requests.get function! Let me try to give you an overview about it.

Unit tests don't call external services, external APIs. Unit tests must test only the unit. In other words, they should test only the method itself. Unit tests should not test anything external to the method. Not any other method that is called inside, any other class, any other service.

Ok, but what if the method I am testing calls an external API? What should I do? Here is where the mock enters! You mock the external call. Wait, what?

Yes, let's say your method calls

def my_method:
   requests.get("http://test.com")

with mock, we say:

hey, method, every time you would call this line requests_mock.get("http://test.com"), now you will use my mock instead!

requests_mock.get("http://test.com", text="data")
return requests.get("http://test.com")

So, instead of the code try to real do a GET request to http://test.com, it will use the mock. It will always receive a Fake Response object where the text attribute of it will be data.

It is a bit complex in the beginning to understand mock. Maybe these docs can help you

https://requests-mock.readthedocs.io/en/latest/pytest.html#pytest
https://realpython.com/python-mock-library/
https://blog.onedaytesting.com.br/test-doubles/

@hebertjulio
Copy link
Member Author

@camilamaia creating the tests I found what can be a problem when the sensitive data of a body is overwritten, following the implemented logic, when I don't have the body attribute, I have to work with the content attribute, but this field cannot be overwritten as well as the body attribute. I believe that the overwritten has to occur in the printing of the report and not as an intermediate process.

def _override_body(http_msg, secret_field):
body = json.loads(http_msg.body.decode("UTF-8"))
if secret_field in body:
body[secret_field] = SENSITIVE_INFO_SUBSTITUTION_FLAG
http_msg.body = json.dumps(body).encode("utf-8")

@camilamaia
Copy link
Member

camilamaia commented May 26, 2021

@hebertjulio indeed. But in the earlier versions of requests, you can set the attribute _content and it should work!

http_msg._content = json.dumps(body).encode("utf-8")

In the Python shell, I got an example to show how we can change the attribute in fact:

>>> import json
>>> import requests
>>> http_msg = requests.get("https://demo.scanapi.dev/api/v1/snippets/2/")
>>> http_msg.content
b'{"url":"http://demo.scanapi.dev/api/v1/snippets/2/","id":2,"highlight":"http://demo.scanapi.dev/api/v1/snippets/2/highlight/","owner":"admin","title":"Print","code":"print(\xe2\x80\x9cabc\xe2\x80\x9d)","linenos":false,"language":"python","style":"vs"}'
>>> http_msg.text
'{"url":"http://demo.scanapi.dev/api/v1/snippets/2/","id":2,"highlight":"http://demo.scanapi.dev/api/v1/snippets/2/highlight/","owner":"admin","title":"Print","code":"print(“abc”)","linenos":false,"language":"python","style":"vs"}'
>>> body = http_msg.body if hasattr(http_msg, "body") else http_msg.content
>>> body = json.loads(body.decode("UTF-8"))
>>> body
{'url': 'http://demo.scanapi.dev/api/v1/snippets/2/', 'id': 2, 'highlight': 'http://demo.scanapi.dev/api/v1/snippets/2/highlight/', 'owner': 'admin', 'title': 'Print', 'code': 'print(“abc”)', 'linenos': False, 'language': 'python', 'style': 'vs'}
>>> body['title'] = "SENSITIVE_INFO"
>>> body
{'url': 'http://demo.scanapi.dev/api/v1/snippets/2/', 'id': 2, 'highlight': 'http://demo.scanapi.dev/api/v1/snippets/2/highlight/', 'owner': 'admin', 'title': 'SENSITIVE_INFO', 'code': 'print(“abc”)', 'linenos': False, 'language': 'python', 'style': 'vs'}
>>> http_msg._content = json.dumps(body).encode("utf-8")
>>>
>>> http_msg.content
b'{"url": "http://demo.scanapi.dev/api/v1/snippets/2/", "id": 2, "highlight": "http://demo.scanapi.dev/api/v1/snippets/2/highlight/", "owner": "admin", "title": "SENSITIVE_INFO", "code": "print(\\u201cabc\\u201d)", "linenos": false, "language": "python", "style": "vs"}'
>>> http_msg.text
'{"url": "http://demo.scanapi.dev/api/v1/snippets/2/", "id": 2, "highlight": "http://demo.scanapi.dev/api/v1/snippets/2/highlight/", "owner": "admin", "title": "SENSITIVE_INFO", "code": "print(\\u201cabc\\u201d)", "linenos": false, "language": "python", "style": "vs"}'

Reference: https://stackoverflow.com/a/26342225/8298081

@camilamaia camilamaia merged commit b6dc048 into scanapi:master May 27, 2021
@camilamaia camilamaia mentioned this pull request May 27, 2021
@hebertjulio hebertjulio deleted the 370 branch May 28, 2021 14:06
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.

Hide response content sensitive info Fix hide_response body
2 participants