aeneasr commented 4 years ago

First of all, thank you for using ORY Oathkeeper and placing trust in us! This issue's purpose is to discuss structural changes to ORY Oathkeeper.

ORY Oathkeeper is used in production at many companies, handling tons of traffic every day. As such, we naturally receive a lot of bug reports and pull requests. Let's take a look at areas of improvement. Some of these changes will not be backwards compatible, which is why we want you to discuss them with us!

All of the listed changes are breaking changes, which is why it is important to get your feedback! When giving feedback, please keep in mind that we want to build the best product possible for you! This sometimes includes breaking things to move forward :)

Non-Goals

Before we start I want to define some non-goals. One of the non-goals is to implement an "identity-aware" proxy like Pomerium. The reason for that is that we don't want to have user interfaces in Oathkeeper itself. It will be possible (in future versions) to get what Pomerium does (plus a ton more) by combining ORY Oathkeeper and ORY Kratos. If an identity-aware proxy is all that's needed, Pomerium is also probably the better choice!

Support negative lookahead

Neither glob nor regex currently support negative lookahead, but it is quite important to support them as it allows for much easier rule definition. See also https://www.linuxjournal.com/content/bash-extended-globbing

More Proxies

https://github.com/ory/oathkeeper/pull/265

Logging

This release aims to better support debugging by adding a lot of debug logs. We understand that debugging ORY Oathkeeper is time-consuming and frustrating and we want to provide the best tooling possible to fix that!

This is a non-breaking change.

Error Handling

The error handling is quite complex to understand, configure, and debug. The following config is a real-world example:

errors:
  fallback:
    - json

  handlers:
    redirect:
      enabled: true
      config:
        to: http://127.0.0.1:4455/auth/login
        when:
          -
            error:
              - unauthorized
              - forbidden
            request:
              header:
                accept:
                  - text/html
    json:
      enabled: true
      config:
        verbose: true

All it does is return JSON unless the a real user using a browser is making the request, in which case we redirect. We need something better!

These change will not be backwards compatible.

Redirect

One thing we can improve is to decide what to do based on the accept header. If it includes application/json, we return JSON (what else would we return?). If it includes text/html we show an HTML error.

To redirect the browser on error, we could define something like:

errors:
  browser:
    when:
      unauthorized:
        redirect_to: http://127.0.0.1:4455/auth/login
     # forbidden
     # ...

This works per-route. It might still be cool to redirect based on content-type - maybe we want to redirect to another application/json endpoint?

errors:
  browser:
    when:
      unauthorized:
        redirect_to: http://127.0.0.1:4455/auth/login
     # forbidden
      request:
        accepts:
          - application/json
        contentType:
          - application/json

Todo: Actually the old logic makes more sense because we can have multiple WHENs with an OR. The question is, how likely is it that we have one rule for all eventuality?

Custom HTML Errors

We could allow custom HTML error pages using Go Templates:

errors:
  browser:
    when:
      unauthorized:
        use_template: file://to/mytemplate.html.gotmpl
     # forbidden
     # ...

The template has access to the error and maybe AuthenticationSession and can return any valid HTML:

<html>
  <!-- ... -->
</html>

Custom JSON Errors

We could allow custom JSON errors using JsonNet:

errors:
  json:
    when:
      unauthorized:
        response: file://to/error.jsonnet
     # forbidden
     # ...

With a JsonNet file like:

// Edit me!
{
  {"message": "go away", "code": ctx.error.code}
}

AuthenticationSession

The AuthenticationSession has grown in scope. It contains more and more fields which are sometimes even used as "temporary storage" or something that can be modified over the lifetime of a request. This was not the struct's intention and it's thus time to think of a better way of solving this.

We want to solve that by first renaming AuthenticationSession to RequestContext with the following layout:

{
  "subject": "12345", // This is the "user" / "service account" / "..." that made the request.
  "request": {
    "headers": {
        "authorization": "...",
        "host": "...",
        "user-agent": "...",
    }, // The original HTTP Request Headers
    "method": "GET", // Or post, put, ...
    "path": "/foo/bar?foo=bar"
    "match": {/*...*/} // Regex Capture groups
  },
  "response": {
    "headers": {
      "x-custom-header": "1234"
    }
  }
}

This context is available in all JsonNet definitions (see sections below) as the global variable ctx.

Go Templates (Breaking Change)

Go Templates are not great for building JSON. While this is readable

mutators:
  id_token:
    enabled: true
    config:
      issuer_url: http://127.0.0.1:4455/
      jwks_url: file:///etc/config/oathkeeper/id_token.jwks.json
      claims: |
        {
          "session": {{ .Extra | toJson }}
        }

it becomes much less readable and hard to debug - especially with nested objects. As hinted in other issues (#423) JsonNet is a good alternative here:

mutators:
  id_token:
    enabled: true
    config:
      issuer_url: http://127.0.0.1:4455/
      jwks_url: file:///etc/config/oathkeeper/id_token.jwks.json
      claims: |
        { "session": ctx.extra, "user": ctx.subject /* , ... */ }

With JsonNet one could also extract values from extra. Because JsonNet is often loaded from file, we should also be able to specify a file location to load the JsonNet code from:

mutators:
  id_token:
    enabled: true
    config:
      issuer_url: http://127.0.0.1:4455/
      jwks_url: file:///etc/config/oathkeeper/id_token.jwks.json
      claims: file://...
      # claims: https://...

JsonNet has many cool features and no sideeffects or security implications. It supports linting, formatting, and we can even provide a test framework. They're much better and easier to use!

Authenticators

`cookie_session` (Breaking Change)

Instead of using extra_form and subject_from with weird JsonPaths

  cookie_session:
    enabled: true
    config:
      check_session_url: http://kratos:4433/sessions/whoami
      preserve_path: true
      extra_from: "@this"
      subject_from: "identity.id"
      only:
        - ory_kratos_session

let's just use JsonNet

  cookie_session:
    enabled: true
    config:
      check_session_url: http://kratos:4433/sessions/whoami
      response:
        parse: |
          { "subject": response.sub, "extra", response }
        # parse: file:// ...
      only:
        - ory_kratos_session

Because we're already changing the config, we also want to improve the config layout for preserving paths and the original HTTP method:

  cookie_session:
    enabled: true
    config:
      request:
        preserve:
          method: true # Otherwise this becomes a GET request, see https://github.com/ory/kratos/issues/270
          path: true
        headers: |
          {
            Authorization: ctx.subject
          }

Authorizers

`remote_json`

This authorizer makes a remote rpc call with a JSON body and expects 403 or 200. Currently, the config is as follows:

  remote_json:
    enabled: true
    config:
      remote: https://...
      payload: |
        { "some": "go template {{.Subject}}" }

Besides switching to JsonNet support instead of Go Templates, we want to improve the way the request is made by making it possible to also define custom headers:

  cookie_session:
    enabled: true
    config:
      remote: https://...
      request:
        body: |
          { user: ctx.subject }
        headers: |
          {
            Authorization: ctx.subject
          }

`remote`

This authorizer makes a remote rpc call and pipes through the body. It expects 403 or 200. Currently, the config is as follows:

  remote_json:
    enabled: true
    config:
      remote: https://...
      headers:
        SomeHeader: "go template {{.Subject}}"

Besides switching to JsonNet support instead of Go Templates, we want to improve the way the request is made by making it possible to also define custom headers:

  cookie_session:
    enabled: true
    config:
      remote: https://...
      request:
        body: |
          { user: ctx.subject }
        headers: |
          {
            Authorization: ctx.subject
          }

`keto_engine_acp_ory` (Breaking Change)

This release will remove the authorizer. You should use remote_json instead:

Mutators

`header`

Right now the header mutator is configured with Go Templates like so (example):

mutators:
  header:
    enabled: true
    config:
      headers:
        X-User-Id: {{ .Subject }}

This will stay very similar but the value is no longer a Go Template but instead a JsonNet. You can get the above by doing:

mutators:
  header:
    enabled: true
    config:
      headers:
        X-User-Id: ctx.subject

`cookie`

Right now the cookie mutator is configured with Go Templates like so (example):

mutators:
  cookie:
    enabled: true
    config:
      cookies:
        myUserAsCookie: {{ .Subject }}

This will stay very similar but the value is no longer a Go Template but instead a JsonNet. You can get the above by doing:

mutators:
  cookie:
    enabled: true
    config:
      cookies:
        X-User-Id: ctx.subject

This allows you to also use more sophisticated means in the future, such as singing the cookie value.

`id_token`

The claims config no longer accepts Go Templates but instead uses JsonNet and loads files from file/https sources:

mutators:
  id_token:
    enabled: true
    config:
      claims: |
        { "session": ctx.extra, "user": ctx.subject /* , ... */ }
      # claims: https://...
      # claims: https://...

`mutator`

Will be removed, see Hydration

Hydration

The hydrator mutator has been added with the intention of making calls to other services and fetching data (e.g. from Stripe). These don't really mutate the response, but add extra data to the AuthenticationSession.

To fix this, the general idea is to add another step to the pipeline called "Hydration". The steps are now:

Check if request can be authenticated (using e.g. oauth2 token introspection)
Hydrate the RequestContext (new! here you can make e.g. HTTP calls to fetch additional user info)
Check if the request is authorized (you can use the additional user info here!!)
Mutate the request (e.g. add id_token)

Instead of using handlers here we think a good idea would be to write JsonNet instead:

hydrator: file://...

The JsonNet gets an extra function called http_request which allows us to make an http request somewhere if needed:

// here is some jsonnet code that makes an HTTP call somewhere. probably is only going to work with JSON requests.

Alternatively, this also has handlers but I'm not sure if that's actually that useful as we always want to transform the RequestContext.

Caching

Caching is a hard problem and even more so in Ory Oathkeeper. Besides a CVE it has lead to several PRs:

Add caching to components who do not have it
Improve caching key flexibility
Add Redis
...

Another interesting thinking is:

Thank you. I am unsure though wether I can agree here. I think it points to a deeper issue though which is manifesting in this pr: It is not possible to define the cache key reliably. Actually, one would probably like to define the cache key themselves (e.g. subject + scope) to make the system more efficient. For example, the AuthSession might contain vital info such as a "permissions" array. Yet, it may also contain a counter or timestamp which is changing for most of the requests - invalidating the cache if included.

We are still not there yet with a new concept for Ory Oathkeeper, but I think this is a very interesting problem that could very well warrant a realignment on Ory Oathkeeper which would be to make access control at the reverse proxy as efficient and flexible as possible.

I think this too could benefit from JsonNet, as JsonNet is typable, lintable, and can produce errors (go templating fulfills none of these properties).

Unfortunately, for this PR in particular, I don't think the current implementation can be accepted because it still bears too many risks and this particular type of issue has already caused a CVE in Ory Oathkeeper which is why I am so hyper-sensible about this topic: https://github.com/ory/oathkeeper/security/advisories/GHSA-qvp4-rpmr-xwrr

https://github.com/ory/oathkeeper/pull/885#discussion_r779336841

So I think this is a sweetspot for this project. Making access control powerful to configure, whlie also very efficient.

iAziz786 commented 4 years ago

Most of the changes are regarding JsonNet. Personally I think it's a win for readability. When I first encountered Go template I was certainly faced a lot of frustrations.

I liked the idea of removing keto_engine_acp_ory gives us a lot of room to add any arbitrary data.

I'm little confused between the differences between remote_json and remote authorizes. In the JsonNet they look identical so won't be better to have only one?

aeneasr commented 4 years ago

Remote passes the request through as-is, and remote_json allows you to modify the request body but it's required that it's json!

aeneasr commented 4 years ago

We'll also address this: https://github.com/ory/oathkeeper/pull/265

duytruong commented 4 years ago

I, personally, don’t want to add UI to Oathkeeper because it’s hard to keep track the changes.

Logging: Will you add capability to debug the jsonnet output when request comes to Oathkeeper (log jsonnet output stdout)?

Hydrator: Will you allow to use additional info to be used in the mutators (e.g. add additional data to jwt created by id_token)? Allow the hydrator to add extra data and pass to next mutator is a useful feature, I have a use case that need to add extra data (role info) to the jwt created by id_token mutator, so it’s good to have ability to make a http request somewhere (to our authz service in my case) to enrich the RequestContext and then enrich the jwt by using data from RequestContext.

aeneasr commented 4 years ago

Logging: Will you add capability to debug the jsonnet output when request comes to Oathkeeper (log jsonnet output stdout)?

Yes!

Hydrator: Will you allow to use additional info to be used in the mutators (e.g. add additional data to jwt created by id_token)? Allow the hydrator to add extra data and pass to next mutator is a useful feature, I have a use case that need to add extra data (role info) to the jwt created by id_token mutator, so it’s good to have ability to make a http request somewhere (to our authz service in my case) to enrich the RequestContext and then enrich the jwt by using data from RequestContext.

Yes!

ArthurKnoep commented 4 years ago

Regarding the custom json error, I had created a thread on the forum some time ago, so I'm pretty happy with this announcement :grinning:.

A while ago, I was thinking of a new authenticator which would work a bit like your remote authorizer.

In this authenticator you'll tell oathkeeper how to extract the token from the request (http header, query param, post multipart, cookie, ...) and then oathkeeper will query an url set in the configuration (either global or per rules) and will pass the extracted token in the query. The external service will have to respond a 200 OK and a json with the subject in it. This could allow us to implement our own introspection service.

mogthesprog commented 4 years ago

Hey,

Great update, looking forward to Jsonnet instead of Go-templates. remote/remote_json over the specific keta integration makes so much sense too.

I'm particularly interested in your non-goal though (sorry!).

It will be possible (in future versions) to get what Pomerium does (plus a ton more) by combining ORY Oathkeeper and ORY Kratos.

I personally wouldn't compare Oathkeeper+Kratos to pomerium, and am glad to see that the scope of these two services is a superset of Pomerium. I think the key differentiator is the Mutator paradigm, i think it's crucial in maintaining different security domains when dealing with application ingress.

Anyway, I'm wondering if i can help get there, I have some time to contribute too (with a bit of guidance).

I'm specifically interested in the pervasive nature of the beyond-corp definition of front-end infrastructure, and how Kratos and Oathkeeper can achieve that. I like the /sessions/whoami endpoint in Kratos, and i think offloading the session management to a different system significantly lowers the operational overhead for oathkeeper, and given it's on the critical path we need to keep it simple.

I'm wondering how you plan on introducing this support in Oathkeeper + Kratos and if you're looking for help?

Cheers!

FredrikAppelros commented 4 years ago

Most of these changes would not affect how we use Oathkeeper or at least only introduce trivial changes needed to the configuration. I think it sounds like you are heading in a good direction and think that the switch to Jsonnet is welcome!

aeneasr commented 4 years ago

Hey thank you for your insightful comment! So this actually is already possible - check out the guide for combining ORY Oathkeeper with ORY Kratos over here: https://www.ory.sh/kratos/docs/guides/zero-trust-iap-proxy-identity-access-proxy

mogthesprog commented 4 years ago

Hey thank you for your insightful comment!

No worries! I've been following the project for a while now, and finally, have time to play to see what the project(s) can do.

check out the guide for combining ORY Oathkeeper with ORY Kratos over here: https://www.ory.sh/kratos/docs/guides/zero-trust-iap-proxy-identity-access-proxy

So this is the guide I used to play around with above. The guide works, but it's not clear how i evolve that configuration for what I'm looking for:

The burden of authentication lies with SecureApp+Kratos, not oathkeeper
Let's say I wanted to take secureApp and used it for user management and hosted it at, accounts.mydomain.com.
If I then wanted to deploy my application to myapp.mydomain.com behind oathkeeper, say, and rely on the users stored in Kratos (managed via accounts.myapp.com), how could i do this today? I don't think it's possible without myapp integrating with Kratos? That is I can't rely on oathkeeper to ensure my users are authenticated prior to ingressing the traffic to myapp.

I could be missing something here though as I'm still digging. Also, if there's a better place to discuss this, let me know (don't want to clog up this issue unless this is the right place)

casell commented 4 years ago

Can't wait to see this happening! I agree the switch to JsonNet will make everything easier to configure.

If I can add something, I'd like to see error_handlers have computed URL and access to request e.g. redirect to something like: /login?origin={{request.url}}

aeneasr commented 4 years ago

Ah yeah that makes sense, this could be used for ?return_to right?

We still have two issues left I think:

The error handling right now is pretty strong, it has a lot of potential uses. But it's hard to configure. Maybe we could provide a high-level and low-level config? I think the current proposal doesn't really catch all the nuances. An alternative would obviously be something with JsonNet but let's not overdo it with the library :)
There are issues where someone wants to use basic and bearer authorization for example, or two introspection authenticators. I think we should allow this, as it's currently not really possible.

casell commented 4 years ago

Ah yeah that makes sense, this could be used for ?return_to right?

You hit the point!

We still have two issues left I think:

The error handling right now is pretty strong, it has a lot of potential uses. But it's hard to configure. Maybe we could provide a high-level and low-level config? I think the current proposal doesn't really catch all the nuances. An alternative would obviously be something with JsonNet but let's not overdo it with the library :)

Yes as often with flexibility comes complexity, so probably the 2 configs will be the good approach: i.e. 95% of use cases covered with high level, 100% with low level (to say low level is a superset of high level)

There are issues where someone wants to use basic and bearer authorization for example, or two introspection authenticators. I think we should allow this, as it's currently not really possible.

So somewhat chaining multiple authN (with rules like sufficient/required) and/or authZ (with similar rules)?

aeneasr commented 4 years ago

Yeah so right now we don't allow alternatives, so if one authenticator validates bearer token than that's the only one allowed to validate the bearer token. I think we can fix this on a code-level though with some intelligent grouping of authenticators.

Maybe it's also just too complicated regarding errors, and we just try it easy for now and introduce more flexibility as people ask for it.

aeneasr commented 4 years ago

The new pipeline will looks like this:

ecktom commented 4 years ago

Should we consider https://github.com/ory/oathkeeper/issues/461 in some way for this diagram?

aeneasr commented 4 years ago

461 is now implementable since #486 :)

tnm0113 commented 4 years ago

Sorry for asking but have the hydrator and new pipeline been implemented in next-gen branch ?

aeneasr commented 4 years ago

Not yet

On 4. Sep 2020, at 10:19, toanguyen notifications@github.com wrote:

Sorry for asking but have the hydrator and new flow been implemented in next-gen branch ?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

shreeharsha-factly commented 4 years ago

We have an application Kavach which supports many to many relationships between organisations and users. And we have some other applications which use Kavach for user info and multi organisation support. We create keto policies where the subject contains kavach user id and not kratos subject.

Policy Example:

{
  id: orgs:${org_id}:admins
  subjects: [${kavach_user_id}]
  resources: [resources:${org_id}:<.*>]
  actions: [actions:${org_id}:<.*>]
  effect: allow
}

By using a Hydrator mutator we are adding kavach user id as X-User header but all mutators run after the authorizer. So we are not able to use the remote Authorizer. So that’s why we have to write keto policy check logic in our apps and not in Oathkeeper.

Is there any way we can configure the flow of the different Handlers for example we want to run Mutators(specifically Hydrator) before the Authorizer?

aeneasr commented 4 years ago

Check out https://github.com/ory/oathkeeper/issues/441#issuecomment-665581211 - would that pipeline solve your woes?

shreeharsha-factly commented 4 years ago

yeah that pipeline logic will solve our problem.

Hydrator will use AuthenticationSession or RequestContext any update on that?

aeneasr commented 4 years ago

We'll probably end up with a context object which is mutable by all parts of the pipeline!

schreddies commented 4 years ago

Could we add some flexibility to the whole hydrator mechanism - with making it possible to be before (and after) each step (authentication, authorization and mutation)? For example to rule would be like this:


    "id": "asdf",
    "version": "v0.36.0-beta.4",
    "upstream": {
      "url": "wherever"
    },
    "match": {
      "url": "http://awesomeapp.com/awesomness",
      "methods": [
        "GET"
      ]
    },
   "hydrators":[
        "handler": "hydrator1",
        "config": {
          "api": {
            "url": "http://hydrator1.com"
          }
        }
   ],
    "authenticators": [
      {
        "handler": "oauth2_introspection",
        "config": {
          "required_scope": [
            "openid",
            "offline"
          ]
      }
    ],
   "hydrators":[
        "handler": "hydrator2",
        "config": {
          "api": {
            "url": "http://hydrator2.com"
          }
        }
   ],
    "authorizer": {
      "handler": "allow"
    },
    "mutators": [
      {
        "handler": "header",
        "config": {
          "headers": {
            "X-User": "{{ print .Subject }}"
          }
        }
      }
    ]
  },```

aeneasr commented 4 years ago

Thank you for the idea! I think we would probably need some smart way of doing this without increasing configuration bloat by too much. Maybe a custom codable pipeline using JsonNet?

aeneasr commented 3 years ago

https://github.com/google/cel-go could potentially really help out here

fugu13 commented 3 years ago

We're using Oathkeeper more and more where I work (ModusBox), and getting ready to expand that use considerably as part of a centralized IAM setup for hosted customer APIs, and while we can get existing Oathkeeper to do what we need we're particularly interested in some of the JSonnet mechanisms planned for NextGen, both because they're more flexible in ways we'd be able to take advantage of and because we've got a lot of expertise in JSonnet (we used it as the basis for an open source data transformation platform, DataSonnet).

I know you don't give out timelines, so I've got a somewhat different question: how can we help, particularly around JSonnet support? We don't have extensive resources to deploy but I could definitely spend a modest portion of my time on features we know we'd use.

aeneasr commented 3 years ago

The best option would be to come up with a proposal for JsonNet implementation - particularly around the config schema and how you plan (or not) to keep backwards compatibility.

It's been a while since we opened this issue, it's still on our list, but we just don't have anyone dedicated to maintaining larger changes and refactorings in the oathkeeper codebase...

fugu13 commented 3 years ago

I could work up a proposal like that. Very loosely, where I'd be inclined to start would be a new mutator under the existing approach that works similar to the new Hydrator proposal in the description. That would be a test bed for incorporating Jsonnet generally, plus provide some useful and flexible new capabilities pretty quickly. I'll see about writing something up with more detail

aeneasr commented 3 years ago

I think that makes sense! I was also thinking if we should use JsonNet as a programming language so add some custom builtin functions such as making HTTP calls. However, I'm not sure if that's a good idea really as JsonNet is really not a programming language as it's difficult to test. My idea was a bit like this (pseudo code!):

local jwt = import "authenticators.jwt.jsonnet"
local cc = import "authenticators.client_credentials.jsonnet"

function (request, ctx) {
  local session = jwt(request)
  if !session
    session = cc(request)
  end

  // do the same stuff for authorizers

  {
    upstream: {
      header: {
         X-User-Id: session.sub,
         X-Auth-Signature: sign(session.sub),
      }
    }
  }
}

But yeah I think it will be pretty difficult for people to get into this syntax.

An alternative to that would be a JavaScript VM which more or less does the same thing, but here again, the problem is most likely one of testing. To enable testing with this approach, what we would need would be something like this:

# test.yml
rule: path/to/rule.jsonnet
input:
  url: ....
  method: ...
  headers:
    Authorization: ...

mock:
  authenticator:
    jwt:
      success: true
      session: ...

expect:
  response:
    ...

But yeah I'm not sure if this overcomplicates things. However, we've seen in the past that people want more and more and more customizations for the individual flows, adding flags and configs everywhere. Maybe this is a better approach?

Another alternative would be to use the traefik way of https://github.com/traefik/yaegi which is Go code and probably easier to learn and write than jsonnet. We could still use jsonnet transformations though for some things (e.g. request body modification).

aeneasr commented 2 years ago

By the way, if anyone in this thread wants to do this as part of a full time position at Ory - please head over to https://www.ory.sh/jobs - we're actively looking for a FTE maintainer here (just apply for backend or fullstack and say you've seen this comment on oathkeeper issue #744 ) :)

stuartpb commented 2 years ago

Just dropping in to connect that a next-generation Oathkeeper would probably be an excellent time to implement a renaming of the project (#804).

achedeuzot commented 2 years ago

Hi ! Just chiming in about requesting if multiple same-type authenticators would be possible ? We have a use-case with different rules to match incoming /decision requests. Depending on the rule, we'd like to use different authenticators. As the current key of the authenticator is the type of the authenticator, we're stuck with only one of each.

So our current workaround was to mutate headers in our reverse proxy (nginx) before it hits oathkeeper so we can match on different authenticators but it's quite cluncky and fragile. Once we've used up all the different authenticators, we can't have any new system to authenticate users. Would it be possible to have authenticators with a user-chosen name as the key and a sub-key as the type. e.g.:

authenticators:
  my_first_cookie_session:
    enabled: true
    type: cookie_session
    config:
      only:
        - sessionid
  my_second_cookie_session:
    enabled: true
    type: cookie_session
    config:
      only:
        - jsession

This way, in the rules, I can reference different authenticators depending on the rule (which can match different headers or parameters).

Thanks for creating oathkeeper, it's served us well ;)

aeneasr commented 2 years ago

Thank you for the feedback! We absolutely want to support multiple handlers in the future, and also make them reusable across the configurations - I think that's a great idea :)

schreddies commented 2 years ago

@achedeuzot It seems like great option/feature for device verification/ authentication before user.

cmmoran commented 9 months ago

It may because I'm late to this game but if we're using dlclark/regexp2, lookahead/lookbehind (positive and negative) are supported.

The only issue I could find is that we've hard-coded the delimiters for url regexp patterns to be '<','>'. This conflicts with the (?< syntax for lookbehinds. A quick-fix naive approach is to do take our current:

func (re *regexpMatchingEngine) compile(pattern string) error {
    if re.table == nil {
        re.table = crc64.MakeTable(polynomial)
    }
    if checksum := crc64.Checksum([]byte(pattern), re.table); checksum != re.checksum {
        compiled, err := compiler.CompileRegex(pattern, '<', '>')
        if err != nil {
            return err
        }
        re.compiled = compiled
        re.checksum = checksum
    }
    return nil
}

and give it a slight face-lift:

func (re *regexpMatchingEngine) compile(pattern string) error {
    if re.table == nil {
        re.table = crc64.MakeTable(polynomial)
    }
    if checksum := crc64.Checksum([]byte(pattern), re.table); checksum != re.checksum {
        startDelim := byte('<')
        endDelim := byte('>')
        if strings.Contains(pattern, "(?>") || strings.Contains(pattern, "(?<") {
            if strings.ContainsRune(pattern, '{') && strings.ContainsRune(pattern, '}') {
                startDelim = byte('{')
                endDelim = byte('}')
            } else {
                return errors.Errorf("attempted to use regex 'possessive match' or regex 'lookbehind' without changing delimiters from '<...>' to '{...}' in: %s", pattern)
            }
        }
        compiled, err := compiler.CompileRegex(pattern, startDelim, endDelim)
        if err != nil {
            return err
        }
        re.compiled = compiled
        re.checksum = checksum
    }
    return nil
}

This would have little-to-no impact on current regex url patterns (because they're broken anyway). However, there would be a way to fix them by changing the start/end delimiters in the access-rules.yml to '{','}' if/when the pattern is broken because of the use of (?> or (?< syntax. (possessive match or either lookbehind type)

I have a fix (with tests) in my personal fork https://github.com/cmmoran/oathkeeper/tree/fix/regexp-possessive-match-and-lookbehinds

I'll create a PR in case there's other folks struggling with this.

EDIT: Apparently my initial solution was naive. Using '{','}' as delimiters when "(?<" or "(?>" exist within the pattern works wonderfully; until your pattern contains "(?<" or "(?>" and an "occurence(s)" check: {1}, {2,}, {12,14-19}, etc.

I toyed with different possibilities for simplifying this but came up short every time. The only solution that I could come up with was increasing the alphabet available to allow the use of characters that take up more than a single byte. This will require a change to https://github.com/ory/ladon because oathkeeper's regular expression engine uses it. The change to ladon is required to be able to support multi-byte delimiters. For example, in my fork I landed on allowing '<<' and '>>' to be the regex delimiters. Internally (to oathkeeper), I replace '<<' with '«' and '>>' with '»' and in ladon I changed the delimiterIndicies and CompileRegex functions slightly to use rune instead of byte and []rune instead of string. The params to CompileRegex have not changed. I realize this means that any delimiterStart, delimiterEnd byte passed will experience precision loss when converted to rune(delimiterStart) or rune(delimiterEnd) but I have not found a use-case where the results are adverse. But I am perfectly willing to be wrong on this. Handling parsing the regex via runes eliminates the need for manually dealing with the byte length of the characters of the pattern string. The end result is that with the change I made to ladon, the regex pattern can be parsed cleanly; even with multi-byte delimiters.

wiliamjcj commented 1 month ago

Hey guys, sorry for the off topic, not sure where would be the right place to ask this. Any ideas on when a new release version is coming up? Last version is v0.40.7 feb 29 right?

ory / oathkeeper

Lessons learned & ORY Oathkeeper NextGen #441

Non-Goals

Support negative lookahead

More Proxies

Logging

Error Handling

Redirect

Custom HTML Errors

Custom JSON Errors

AuthenticationSession

Go Templates (Breaking Change)

Authenticators

`cookie_session` (Breaking Change)

Authorizers

`remote_json`

`remote`

`keto_engine_acp_ory` (Breaking Change)

Mutators

`header`

`cookie`

`id_token`

`mutator`

Hydration

Caching

461 is now implementable since #486 :)

ory / oathkeeper

Lessons learned & ORY Oathkeeper NextGen #441

Non-Goals

Support negative lookahead

More Proxies

Logging

Error Handling

Redirect

Custom HTML Errors

Custom JSON Errors

AuthenticationSession

Go Templates (Breaking Change)

Authenticators

cookie_session (Breaking Change)

Authorizers

remote_json

remote

keto_engine_acp_ory (Breaking Change)

Mutators

header

cookie

id_token

mutator

Hydration

Caching

461 is now implementable since #486 :)

`cookie_session` (Breaking Change)

`remote_json`

`remote`

`keto_engine_acp_ory` (Breaking Change)

`header`

`cookie`

`id_token`

`mutator`