Skip to content
This repository has been archived by the owner on Jan 7, 2024. It is now read-only.

Send headers for POST requests sent directly to server #26

Merged
merged 2 commits into from
Oct 23, 2018

Conversation

redshiftzero
Copy link
Contributor

fix #25

this works but i'm marking this wip because the tests didn't fail when this issue was introduced (due to our reliance on VCR cassettes). we need to make a change to the tests also in order to detect any other defects

@kushaldas
Copy link
Contributor

I was thinking vcr checks for all values, it seems it does not :(

We should not get this error in the proxy side as we check based on all input except the dynamic token or the totp value.

@redshiftzero
Copy link
Contributor Author

I marked this WIP because as a general rule I don't like to resolve a bug without adding a regression test. there may be other issues that were not detected during merge of #21

@redshiftzero redshiftzero changed the title [wip] Send headers for POST requests sent directly to server Send headers for POST requests sent directly to server Oct 22, 2018
@redshiftzero
Copy link
Contributor Author

Regenerating all VCR cassettes would find defects like #25 - I've done this in 80b529d to ensure that there are no other regressions in the functionality for the SDK when one is not using the securedrop-proxy.

One can verify the fix in this PR by following this procedure:

Reproduce the bug

  1. Reverting adac87f
  2. Run the server dev container
  3. rm data/test-star-add-remove.yml
  4. python -m pytest -v tests/test_api.py::TestAPI::test_star_add_remove
  5. See that the test fails.

Verify the fix

  1. Apply the change in adac87f
  2. rm data/test-star-add-remove.yml
  3. With the server dev container still running:
python -m pytest -v tests/test_api.py::TestAPI::test_star_add_remove
  1. The test should now pass.

Maintainers of this project should be careful to ensure that VCR cassettes and other mock request/responses are regenerated when PRs are proposed modifying the SDK logic.

Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

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

This works as expected. To test, we have to comment out the @vcr decorator in the setUp function.

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

Successfully merging this pull request may close these issues.

headers not sent on POST requests sent directly to server
2 participants