-
Notifications
You must be signed in to change notification settings - Fork 324
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
No Longer Able To Allows Options Requests Without API Key #143
Comments
Sorry for the delay. Unfortunately, I can't reproduce this. I just tried from master and a fresh api-umbrella v0.8 install, but in both cases, after setting up the sub-URL settings, I was able to eliminate the API key requirements on the OPTIONS requests. But we'll see if we can get to the bottom of what's going on. A few questions for you:
For reference, here's my example I tested that seemed to be working as expected: And the resulting request without an API key:
|
That's for getting back to me. Do you have other sub-URL settings o this API backend defined, or just the one for OPTIONS? No, just this one. Does your API backend match multiple URL prefixes, or just one? Just one. Do you have multiple API backends configured in API Umbrella, or just one? Just this one, at the moment. Was this on a system that was upgraded to the latest version of API Umbrella, or a fresh install? This is a fresh install, configured from scratch. I adjusted our configuration to be in line with what you configured. At first, I was seeing the exact same results but after restarting api-umbrella, OPTIONS requests started working. So, I gradually started re-applying our configuration. Everything works fine until I try to apply a require role to the backend. Then I start seeing 502 Bad Gateway errors. Restarting api-umbrella has no impact. If I remove that configuration, the call starts working again. Aside from no live upstream connection errors in the router, I couldn't find any other error messages in the logs:
So, it looks like it may be the role functionality that is causing these issues. |
Very sorry for the delay following up (I've been out of town). And sorry for the issue too! But thanks for the great debugging. The fact that the roles affects things certainly seems odd, but I can definitely look into that and see about getting that fixed. I'll try to look into this sometime in the next couple days. |
I've only just done a cursory glance at the source code, but my theory is since the sub-URL doesn't specify any required roles, the role validator middleware is getting the value set at the API kevel. When it tries to get the roles for the anonymous user, it, of course, has none, so it fails to match against the required roles for the API, and thus returns an API key unauthorized error. Sometimes no roles means inherit from the parent while others it means override the parent with no roles required, like in my case. I think the best way to model this would be similar to how you do with API keys and https now: break it into two fields.
So, if I inherit:
And if I override:
Thoughts? If we can't implement this entire logic tree this time around, can we at least offer some way to address the current scenario without breaking current functionality? Thanks! |
Yup, you're right. I was also looking into this yesterday and I think we need an option to specify whether roles are inherited or overridden. Sorry for the delay on addressing this bug, but I think this should be pretty easy to implement and I should finally be able to get to this tomorrow or the following day. Out of curiosity, were you definitely using using roles with your previous OPTIONS setup? Because when I looked back at the history of the code, I'm just not sure how this ever would have been functioning properly with role requirements also in place. It's certainly possible I'm missing some subtle change we made in the code, but I was mostly just wondering if this was definitely working before. In any case, we can certainly get this fixed and add testing around this situation. Sorry for the trouble! |
When we set up the sub url, there would not have been any roles configured. I am guessing we didn't re-test it from a browser when we updated our API configuration to include roles. Most of the time, we were hitting the API server-side, so he wouldn't have noticed any problems. So, I'd say it likely never worked with roles. Thanks. |
Sorry for the delay. This should be fixed in master by NREL/api-umbrella-gatekeeper#17 and and NREL/api-umbrella-web#17 I'll try to get v0.9 released in the next couple weeks to get this out in the released packages, but in the meantime, you could also try deploying from master (and give me a shout if anything doesn't work with that). Here are the details on the changes made as part of this:
Let me know if any of these updates don't make sense for your use-case or you have any other questions. I'm also mulling over adding a simple option to the global settings level for something like "Allow pre-flight CORS requests without API keys." I've been a little hesitant about getting too far down into the rabbit-hole of adding CORS-specific options, since I think a lot of that is best handled by the API backend itself, or it can now be setup with the various more generic options we have (like "Default Response Headers" setting a So my thinking is that it might be worth introducing this type of checkbox that would do a few things automatically:
I almost think all these settings could be enabled by default for most API backends since it's probably what's desired if you're dealing with CORS requests. However, I'm a tad reluctant to default to all this without having the administrator explicitly enable it, since it seems like there's a possibility it might open up some unanticipated security issues. For example, if you had non-CORS OPTIONS endpoints or a mis-configured API backend that responded to all HTTP verbs (including OPTIONS) without being aware, then disabling all the key and role requirements might lead to some inadvertent openings. But if we make it opt-in, a simple checkbox might simplify this configuration. Anyway, let me know if you have any thoughts or feedback on any of this. Thanks for reporting the issue! |
Your updates should work perfectly for us. I agree, I think it would make a lot of sense to offer some automation around handling pre-flight CORS requests. However, what if you simply called the checkbox 'CORS Support (Browser Requests)' though. Desiring CORS support basically implies all the rules you are proposing, so why not give it a simple intuitive name. You won't be checking that box if you're using OPTIONS for a different use case. |
how did u solve it ??,i have the same error |
@GUI We were using your second recommendation in #116: creating a sub-url rule to allow OPTIONS requests without providing an API key. However, this appears to no longer work in the latest version of API Umbrella.
We are now getting 403 errors on the OPTIONS calls again:
Thanks!
The text was updated successfully, but these errors were encountered: