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

No Longer Able To Allows Options Requests Without API Key #143

Closed
darylrobbins opened this issue May 22, 2015 · 10 comments
Closed

No Longer Able To Allows Options Requests Without API Key #143

darylrobbins opened this issue May 22, 2015 · 10 comments

Comments

@darylrobbins
Copy link
Contributor

@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:

{
    "error": {
        "code": "API_KEY_MISSING",
        "message": "No api_key was supplied. Get one at http://demo.api.mygrocerydeals.com"
    }
}

Thanks!

@GUI
Copy link
Member

GUI commented May 25, 2015

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:

  • Do you have other sub-URL settings o this API backend defined, or just the one for OPTIONS?
  • Does your API backend match multiple URL prefixes, or just one?
  • Do you have multiple API backends configured in API Umbrella, or just one?
  • Was this on a system that was upgraded to the latest version of API Umbrella, or a fresh install?

For reference, here's my example I tested that seemed to be working as expected:

screen shot 2015-05-25 at 4 50 46 pm
screen shot 2015-05-25 at 4 50 54 pm

And the resulting request without an API key:

$ curl -skD - -X OPTIONS -o /dev/null "https://10.10.33.77/httpbin/get"
HTTP/1.1 200 OK
Server: nginx
Date: Mon, 25 May 2015 22:55:45 GMT
Content-Type: text/html; charset=utf-8
Content-Length: 0
Connection: keep-alive
X-RateLimit-Limit: 1000
X-RateLimit-Remaining: 997
allow: HEAD, OPTIONS, GET
access-control-allow-origin: *
access-control-allow-credentials: true
access-control-allow-methods: GET, POST, PUT, DELETE, PATCH, OPTIONS
access-control-max-age: 3600
x-varnish: 27
age: 0
via: 1.1 varnish-v4
x-cache: MISS
accept-ranges: bytes

@darylrobbins
Copy link
Contributor Author

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:

2015/05/26 18:50:54 [error] 1460#0: *102 no live upstreams while connecting to upstream, client: 10.0.46.30, server: demo.api.domain.com, request: "OPTIONS /v2/deal/search HTTP/1.1", upstream: "http://api_umbrella_gatekeeper_backends/v2/deal/search", host: "demo.api.domain.com"

So, it looks like it may be the role functionality that is causing these issues.

@GUI
Copy link
Member

GUI commented Jun 9, 2015

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.

@darylrobbins
Copy link
Contributor Author

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.

  1. should I inherit or override the roles from the parent?
  2. what are the roles for the sub URL

So, if I inherit:

  • Any roles added should be considered in addition to what is required by the parent
  • If I specify none, then this sub url doesn't change the role settings

And if I override:

  • Any roles added are the only roles required for this sub url
  • If I leave it blank, then no roles are required like in my case

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!

@GUI
Copy link
Member

GUI commented Jun 12, 2015

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!

@darylrobbins
Copy link
Contributor Author

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.

@GUI
Copy link
Member

GUI commented Jun 15, 2015

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:

After giving all this a second look, this led to few other changes in how roles get applied in the sub-url settings context. A few of these changes are backwards incompatible, but in configuration scenarios I don't think would have been very common. And hopefully the new approach is more logical overall. The changes made are:

  • Roles from sub-URL settings now are appended by default to any original roles, rather than overwriting them. This seems more intuitive and less dangerous, since you're less likely to accidentally lift all role restrictions by simply adding a sub-url setting.
  • There's a new option to explicitly have sub-URL roles override the parent roles, but by default, they are appended.
  • When multiple roles are defined on the API, now the API key must have all of the roles in order to access the API, rather than any single role. I don't think the previous approach of only requiring a single role was very intuitive, and I think this approach makes more sense particularly with sub-URL roles appending to the base roles by default.
  • Fix crash possible if API backend was setup with role requirements but api keys not required. While this combination of settings doesn't make sense, it may have been encountered with an API backend that required a role, but then a sub-URL setting trying to disable key requirements (for CORS preflight requests as an example).
  • Remove special handling of "admin" role that previously granted access to all APIs. This wasn't really documented anywhere, and I don't think we have a strong use-case for this kind of global admin role, so it seems safest to remove this so a user isn't accidentally granted global access.

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 Access-Control-Allow-Origin: * header). And while I think these pre-flighted requests can now be handled properly via the sub-URL settings, the necessary configuration seems a bit complex and non-obvious for a common setup (and particularly since the API backend can't handle this specific setup, since it stems from the API Umbrella proxy layer wanting API keys).

So my thinking is that it might be worth introducing this type of checkbox that would do a few things automatically:

  • Disable api key requirements for OPTIONS requests.
  • Disable any required roles for OPTIONS requests.
  • Append or set X-Api-Key to the Access-Control-Allow-Headers response header on OPTIONS responses.

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!

@GUI GUI closed this as completed Jun 15, 2015
@darylrobbins
Copy link
Contributor Author

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.

@sandeepkottagundu
Copy link

sandeepkottagundu commented May 30, 2017

I have to run user api it will provide documentation but it will give 403 error
err

@Hoda2017
Copy link

how did u solve it ??,i have the same error

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

No branches or pull requests

4 participants