All Astronauts Posted December 9, 2020 Posted December 9, 2020 Granted, it does what it says, but the reality is you are mainly concerned with Authorization headers being sent and you are assuming if CGI then no auth headers. You may wish to start checking for valid auth headers instead - that can be dealt with with CGI implementations - both .htaccess and Nginx confs - it's only a rewrite rule for the most part, not that hard. It's a hard warning when setting up API keys now (telling admins to set an ip address to be safe), and a little digging shows that when you guys eventually go ahead and turn on Zapier integration for the masses it is going to tell CGI mode servers to get bent and I imagine there will be a non-trivial number of users who will be a little miffed when that's the first they learn of it.
bfarber Posted December 9, 2020 Posted December 9, 2020 We warn admins if they may need to do something but we won't know if they have auth headers when configuring things in the AdminCP up front. What do you suggest exactly?
All Astronauts Posted December 9, 2020 Author Posted December 9, 2020 The one place it matters right now: New Rest API keys. You check for cgi, and if it is running, you then flash this: Quote Your server does not support authentication headers. As you will need to pass the API key in the URL, we recommend you set up IP address restriction. That took away a few minutes of my life and got me down this rabbit hole. My server (in question) does support auth headers so I had to actually check what the hell was happening, is there something wrong with my server, and etc., etc., but no, it is just that you don't bother to actually verify headers at all, you just check if cgi is running and then assume there are no Auth headers available, and push out that warning. A warning based on a guess. At a minimum changing that lang key to something more accurate such as "We detected PHP CGI and you might not have auth headers..." Granted, for the IPS API stuff, if you have auth headers that work then that message is irrelevant - you aren't forcing it off in the authorizationHeader() method in \IPS\Request - so everything will still work, but again, you are already have code for checking headers right there, why not actually check for valid headers during the key setup method in the first place and have it be factually accurate instead of pushing out a guess? As it stands now in your unreleased-to-the-masses Zapier integration, that cgi check is a flat out middle finger. Unless that changes, as currently coded, if you detect cgi you will not allow Zapier to be configured and used, whether I have valid headers or not. The best outcome here is a validateAuthHeaders method that makes a curl call and then you can re-use \IPS\Request\authorizationHeader() to validate things are good to go. If not, you might want to maybe alert the Invision Community self-hosted user base that there is a rather sticky server requirement for Zapier use. It's a derpy plugin to get around the testSettings() hitch in the Zapier integration for me but I'm struggling to see why I need to do this at all. CoffeeCake and The Old Man 1 1
Solution bfarber Posted December 10, 2020 Solution Posted December 10, 2020 I've raised your suggestion internally for further consideration. All Astronauts 1
All Astronauts Posted July 23, 2021 Author Posted July 23, 2021 Bump now that Zapier is back on the self-hosted menu plus that @Daniel F comment routed through @Jordan Invision Nathan Explosion and Jordan Miller 2
Recommended Posts