Add secure middleware to enable more header options #618
Conversation
| @@ -836,3 +837,73 @@ func TestRequestSignaturePostRequest(t *testing.T) { | |||
| assert.Equal(t, 200, st.rw.Code) | |||
| assert.Equal(t, st.rw.Body.String(), "signatures match") | |||
| } | |||
|
|
|||
| func TestHttpBrowserXssFilterTrue(t *testing.T) { | |||
There was a problem hiding this comment.
It looks like this tests a third-party package but doesn't really test the behavior of this package. What kinds of regressions would this test catch?
There was a problem hiding this comment.
Well, other tests further up in the file test the proxy. This tests that the middleware is functioning on top of the proxy. I definitely don't have 100% coverage here. I only am testing one of the secure middleware options, for example. More tests certainly could be written!
These were sufficient for me to verify that the proxy with middleware worked for our purposes, though.
There was a problem hiding this comment.
I'm not sure this tests any of the code you added. As far as I can tell, the test imports a third-party package, instantiates its middleware, and verifies that the middleware behaves as expected. It doesn't really exercise the code you added, or any of the code in this package.
There was a problem hiding this comment.
Yes, that's how main() does it too. Remember how you told me to wrap the middleware outside of NewOAuthProxy? This is the result. As far as I can see, this is the only way I can actually do this sort of thing. As a golang neophyte, I welcome being proven wrong.
I needed to be have XSS and nosniff headers set to help compliance scans not complain, so I followed through with the suggestion in #384.
Please let me know if you have any suggestions or things that I could do to help this get merged in. We would love to be able to just use upstream for this.
Thanks!