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

Allow multiple rules for one URL #157

Closed
wawan93 opened this issue Mar 6, 2019 · 8 comments
Closed

Allow multiple rules for one URL #157

wawan93 opened this issue Mar 6, 2019 · 8 comments

Comments

@wawan93
Copy link

wawan93 commented Mar 6, 2019

If URL matches by multiple rules, Oathkeeper returns 500 error Expected exactly one rule but found multiple rules. I think, this is bad idea.
For example, if we want to add two rules: /<.*> with token introspection and /swagger.json without introspection.
Now we can't just add this 2 rules. This enforces us to create many rules for every /* route to avoid overlap.
This error is not obvious. It will appear only in runtime, Oathkeeper don't test it during rules import.
This is not internal, not server, and not exactly error - so, returning 500 error is wrong

My proposal is to add priority field to rule definition, and to apply rule with the highest priority for given URL.

Alternative solutions:

  1. Use PCRE instead of default golang's RE2 to allow using negative lookahead in rules definition
  2. Add exclude_url field in match
  3. Just don't return 500 and apply first matched rule
@aeneasr
Copy link
Member

aeneasr commented Mar 6, 2019

Is there a good library for negative lookahead in Go? I generally agree that there should be some way to have <.+> while also being able to exclude some paths. The 500 error still makes sense because what we want to avoid is that something accidentally is allowed although a rule denied it. Precedence with priority is usually not a good idea in security contexts because it's very easy to lose track of what's happening.

We could probably also add something like oathkeeper rules test ... which would run some tests (e.g. with credentials or whatever) against a set of rules. This would be very equal to just running a bunch of curls.

We can obviously not detect rule conflicts automatically on start up because we would have to brute force URLs against your rules which is not practical.

@wawan93
Copy link
Author

wawan93 commented Mar 6, 2019

I've already lose track of what's happening. That's why I got this error and that's why I propose this. For me, priority field is way more obvious, than complicated regexps or plenty files with rules for every route.

@aeneasr
Copy link
Member

aeneasr commented Mar 6, 2019

I disagree on priority. Priorities for security sensitive rules can lead to serious security issues. It's better to disallow stuff explicitly.

What we could obviously do is to support more than regex matching and have stuff like glob matching which is also a bit easier to use and also supports negative lookahead (I think): https://github.com/gobwas/glob

@aeneasr
Copy link
Member

aeneasr commented Apr 5, 2019

Closing in favor of #167

@aeneasr aeneasr closed this as completed Apr 5, 2019
@stszap
Copy link
Contributor

stszap commented Oct 22, 2019

We encountered exactly the same problem today. As a temporary hack I used this pattern (edited) for swagger https://example.com/api/v1/swagger.json and this for the actual api endpoints https://example.com/api/v1/<[\\w\\-\\/]+>. It works in our case because none of the endpoints has "." symbol. Hope it helps somebody.

@aeneasr
Copy link
Member

aeneasr commented Oct 23, 2019

Thank you for pointing that out - it is certainly helpful as a workaround until #167 lands!

@nitedani
Copy link

nitedani commented Jul 18, 2022

I can't figure out how to make this work.

I have 2 upstream services.
I want /auth/** to proxy to service A (kratos-ui).
I want everything else to be proxied to service B.

How is #167 helping with this?

@llbarbosas
Copy link

I can't figure out how to make this work.

I have 2 upstream services.
I want /auth/** to proxy to service A (kratos-ui).
I want everything else to be proxied to service B.

Any updates?

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

5 participants