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

Add lifecycle hooks to run middleware #690

Open
tommysitu opened this issue Jan 26, 2018 · 11 comments
Open

Add lifecycle hooks to run middleware #690

tommysitu opened this issue Jan 26, 2018 · 11 comments
Assignees

Comments

@tommysitu
Copy link
Member

At the moment, middleware can only intercept and modify outgoing requests in capture mode. This is inadequate if someone wants to mask sensitive information in both request and response when capturing.

@JohnFDavenport
Copy link
Contributor

It makes sense to extend Hoverfly as there are probably other use-cases, but having to do masking in middleware seems like a leaky solution for larger teams as it has to be applied in real-time. Developers will forget and policy would be difficult to apply.

We could do with more use-cases on the general topic of masking.

@tommysitu
Copy link
Member Author

After reading this GitHub issue: arquillian/arquillian-organization#10 I get a feeling that it is not necessary a middleware feature

Problem
Request/response pair could contain sensitive data from capturing live endpoints. Storing them as plain text in Github is a bad idea. Manually removing them is error-prone and time-consuming.

Use case
The original use case from @lordofthejars goes like this:

  • In Capture mode, hoverfly should be able to send the original request and receive the original response, but the exported simulation data should have the sensitive data masked.
  • Switching to simulate mode and using the modified data shouldn't break the original tests.

Implications

  1. One should be able to define which fields in the request and response to obfuscate
  2. Data masking should be done before a request/response pair is stored.
  3. If data masking is performed by middleware, Capture mode should support middleware execution similar to Modify mode, but also be able to preserve the original request when sending to target service.
  4. Sensitive data in the request can be replaced by GlobMatcher
  5. Sensitive data in the response can be encrypted (but requires decryption in Simulate mode, and encryption key from the user) or replaced by random value (it might break the tests that assert an exact value on this field in Simulate mode)

@tommysitu tommysitu changed the title Should middleware intercept request and response in capture mode Add lifecycle hooks to run middleware Jul 18, 2018
@JohnFDavenport
Copy link
Contributor

The change of title reflects that we won't be adding explicit support for data masking in Hoverfly itself. Lifecycle hooks - yes. Data masking - no.

@tommysitu tommysitu added this to the v1.1.0 milestone Nov 23, 2018
@tommysitu
Copy link
Member Author

The current middleware does not support request modification in simulate mode, and not support response modification in the capture mode. The way it works is probably to tackle problems such as:

  • A delay middleware is only intended to be used for SIMULATE mode, and switching to CAPTURE mode should not introduce any latency.
  • A middleware which modifies requests is supposed to be used for CAPTURE mode, and you don't want to apply it again when Hoverfly is switched to SIMULATE mode, otherwise the requests could be modified twice.

Making middleware to apply on both request and response for simulate/capture/spy mode is desirable for some use cases but has the potential to break backward compatibility.

@tommysitu
Copy link
Member Author

tommysitu commented Feb 22, 2019

What I propose is that middleware should behave as it is, but you can pass an additional flag to force it to run at a certain point regardless of the mode. For example,

$ hoverctl middleware --binary python --script intercept_requests.py --hooks "PRE_REQUEST_HANDLE"

It allows to intercept requests using middleware before request matching in simulate mode.

Available hooks could be:

  • PRE_REQUEST_HANDLE: before hoverfly processes the requests, i.e. before matching in simulate mode, or before saving the requests in capture mode
  • POST_REQUEST_HANDLE: after the request is saved in capture mode, but before it's forwarded to the destination.
  • PRE_RESPONSE_HANDLE: before hoverfly saves the response for capture mode
  • POST_RESPONSE_HANDLE: after hoverfly saves the response for capture mode

@JohnFDavenport
Copy link
Contributor

JohnFDavenport commented Feb 22, 2019

Looks good to me, except you've got PRE_REQUEST_HANDLE, etc when I assume you meant PRE_REQUEST_HOOK.

It is probably better if neither HOOK nor HANDLE was added.

Two questions:

  1. Can the same middleware run multiple times if more than one hook is specified?
  2. Can the middleware get or derive any context information to tell it what sort of hook it is?

I assume the answer is No to both these questions, but it's worth checking.

@tommysitu
Copy link
Member Author

tommysitu commented Feb 25, 2019

The suffix_HANDLE in the name just provides clarity that the hook is before/after Hoverfly has processed the request/response.

For question 1, yes. You can pass comma seperated list of hooks, and invoke the same middleware at different hooks.

For question 2, maybe no initially, but it should be possible to pass context information into the middleware.

@JohnFDavenport
Copy link
Contributor

Ok. Q1 is good. but wrt Q2. I think that then compromises the feature.

What if the lifecycle stage could be passed in metadata as that shouldn't affect existing middleware, or to be 100% sure, pass it if --hooks is stated?

@lordofthejars
Copy link

Currently, we do not need this feature anymore because we bypassed this information into fake information manually. Of course, it might still be interesting to have this feature but let's say that it is not urgent for our use case.

@tommysitu tommysitu removed this from the v1.1 milestone Jun 4, 2019
@tommysitu tommysitu added this to the v1.2 milestone Jul 23, 2019
@tommysitu tommysitu removed this from the v1.2 milestone Mar 4, 2020
@kapishmalik
Copy link
Collaborator

@tommysitu if we need to build this feature, then I would like to take this up. You can assign this to me. I can start working on the same.

@kapishmalik
Copy link
Collaborator

@tommysitu you can assign this to me. I have started working on the same.

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

No branches or pull requests

4 participants