search

socketry / async-http

MIT License
298 stars 45 forks source link

Add specs to replicate missing auth. #58

Closed samshadwell closed 3 years ago

samshadwell commented 4 years ago

As discussed in Slack, adds some specs to test the "proxy server requires auth, no auth given" case.

After running locally, I'm confirming that the happy path auth succeeds, missing auth gets a broken pipe.

ioquatix commented 4 years ago

Thanks for this I will review it next week.

ioquatix commented 3 years ago

Sorry, I have not had time to review this "next week". But I checked it today and it looks good. Thanks for your effort and apologies for not touching base sooner.

ioquatix commented 3 years ago

I have merged and reviewed this PR.

Firstly thanks for adding the specs, they were helpful.

I'm not certain of how we should handle this, but... I've made the proxy endpoint raise an exception if the internal connect fails, rather than responding with a 407.

I think that the nature of the internal proxy should not leak out.

I'm not certain I'm happy with this approach, but it seemed like the most simple one.

Can you check the specs (in particular the authentication failure case/407) and let me know what you think?

samshadwell commented 3 years ago

Took a look, using an exception instead of conflating the return status makes sense to me! Thanks for doing this 👍