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

using yaml for token config #181

Open
gfrankliu opened this issue Jul 25, 2023 · 27 comments
Open

using yaml for token config #181

gfrankliu opened this issue Jul 25, 2023 · 27 comments

Comments

@gfrankliu
Copy link

I tried below yaml token config:

# use jwt token from auth provider 1 if exists, otherwise trigger oauth from provider 2
eas:
  plugins:
    - type: jwt
      header_name: my-jwt-header
      config:
        secret: https://www.provider1.com/public_key-jwk.json
        options:
          audience: /special/audience/for/me
          issuer: https://www.provider1.com
      pcb:
        skip:
          - query_engine: jp
            query: $.req.headers.my-jwt-header
            rule:
              method: regex
              value: /^bearer/i
              negate: true
    - type: oidc
      issuer:
        discover_url: https://www.provider2.com/.well-known/openid-configuration
      client:
        client_id: aaaaa
        client_secret: bbbb
      scopes:
        - openid
        - email
        - profile
      pkce:
        enabled: true
        code_challenge_method: S256

When I test to send a curl request with header my-jwt-header, external-auth-server throws below errors about json:

{"level":"info","message":"starting verify for plugin: jwt","service":"external-auth-server","timestamp":"2023-07-25T23:12:10.380Z"}
{"level":"error","message":"Lexical error on line 1. Unrecognized text.\n$.req.headers.my-jwt-header\n---------------^","service":"external-auth-server","stack":"Error: Lexical error on line 1. Unrecognized text.\n$.req.headers.my-jwt-header\n---------------^\n    at Parser.parseError (/home/eas/app/node_modules/jsonpath/generated/parser.js:166:15)\n    at Parser.parser.yy.parseError (/home/eas/app/node_modules/jsonpath/lib/parser.js:13:17)\n    at Object.parseError (/home/eas/app/node_modules/jsonpath/generated/parser.js:341:28)\n    at Object.next (/home/eas/app/node_modules/jsonpath/generated/parser.js:595:25)\n    at Object.lex (/home/eas/app/node_modules/jsonpath/generated/parser.js:605:22)\n    at lex (/home/eas/app/node_modules/jsonpath/generated/parser.js:194:28)\n    at Parser.parse (/home/eas/app/node_modules/jsonpath/generated/parser.js:207:26)\n    at JSONPath.nodes (/home/eas/app/node_modules/jsonpath/lib/index.js:118:26)\n    at JSONPath.query (/home/eas/app/node_modules/jsonpath/lib/index.js:94:22)\n    at Assertion.jsonpath_query (/home/eas/app/src/assertion/index.js:41:23)","timestamp":"2023-07-25T23:12:10.380Z"}

Is query_engine jp not supported in yaml format?

BTW, our jwt header doesn't have "bearer" in it. I manually added it in my manual curl test. If the real client sends the jwt header without "bearer", will that be a problem for external-auth-server? Is there a query rule that can check the existence of a header?

@travisghansen
Copy link
Owner

$ is a special char in yaml so you may need to quote or escape it in your yaml.

As long as the header_name is not authorization I think you can set the scheme attribute to empty string "" and it should work (https://github.com/travisghansen/external-auth-server/blob/master/PLUGINS.md?plain=1#L157)

@gfrankliu
Copy link
Author

I tried quoting the string: query: "$.req.headers.my-jwt-header" but got the same error.

@gfrankliu
Copy link
Author

Is below ok to check if the header doesn't exist?

          - type: jwt
            header_name: my-jwt-header
            scheme: ""
            pcb:
              skip:
                - query_engine: jp
                  query: "$.req.headers.my-jwt-header"
                  rule:
                    method: regex
                    value: /./
                    negate: true

@travisghansen
Copy link
Owner

Actually, I just tested it and you've found a bug in the library (https://github.com/dchester/jsonpath) the problem has nothing to do with the $ it's the - in the query. Based on my research this should work $.req.headers."my-jwt-header" but does not. Further it appears that library has fallen into bitrot so it seems unlikely to get fixed anytime soon :(

As an alternative use another query engine. For example if you allow eval you can use the js engine:

            query_engine: js
            query: "try { return data.req.headers[\"my-jwt-header\"]; } catch (e) { return undefined; }"

@gfrankliu
Copy link
Author

Glad you were able to reproduce and thanks for the alternative way. I tried it and got:

{"level":"error","message":"cannot use potentially unsafe query_engine 'js' unless env variable 'EAS_ALLOW_EVAL' is set","service":"external-auth-server","stack":"Error: cannot use potentially unsafe query_engine 'js' unless env variable 'EAS_ALLOW_EVAL' is set\n    at Object.json_query (/home/eas/app/src/utils.js:425:15)\n    at Assertion.query (/home/eas/app/src/assertion/index.js:63:29)\n    at Assertion.assert (/home/eas/app/src/assertion/index.js:78:28)\n    at Function.assertSet (/home/eas/app/src/assertion/index.js:18:36)\n    at processPipeline (/home/eas/app/src/server.js:392:42)\n    at /home/eas/app/src/server.js:483:7\n    at new Promise (<anonymous>)\n    at _verifyHandler (/home/eas/app/src/server.js:313:12)\n    at processTicksAndRejections (node:internal/process/task_queues:96:5)\n    at async verifyHandler (/home/eas/app/src/server.js:97:12)","timestamp":"2023-07-29T04:25:28.859Z"}

Then I set env EAS_ALLOW_EVAL to 1, but got another error when sending a request without jwt header:

{"level":"error","message":"Cannot read properties of undefined (reading 'case_insensitive')","service":"external-auth-server","stack":"TypeError: Cannot read properties of undefined (reading 'case_insensitive')\n    at Assertion.assert (/home/eas/app/src/assertion/index.js:83:14)\n    at processTicksAndRejections (node:internal/process/task_queues:96:5)\n    at async Function.assertSet (/home/eas/app/src/assertion/index.js:18:20)\n    at async processPipeline (/home/eas/app/src/server.js:392:26)","timestamp":"2023-07-29T04:27:45.564Z"}


Finally sending a request with real jwt header:

{"level":"error","message":"Cannot read properties of undefined (reading 'case_insensitive')","service":"external-auth-server","stack":"TypeError: Cannot read properties of undefined (reading 'case_insensitive')\n    at Assertion.assert (/home/eas/app/src/assertion/index.js:83:14)\n    at processTicksAndRejections (node:internal/process/task_queues:96:5)\n    at async Function.assertSet (/home/eas/app/src/assertion/index.js:18:20)\n    at async processPipeline (/home/eas/app/src/server.js:392:26)","timestamp":"2023-07-29T04:39:11.117Z"}

(seems same error as if I were to not send jwt header at all.

@travisghansen
Copy link
Owner

Did you remove the rule block?

@gfrankliu
Copy link
Author

Yes I did:

          - type: jwt
            header_name: x-goog-iap-jwt-assertion
            scheme: ""
            config:
              secret: https://www.gstatic.com/iap/verify/public_key-jwk
              options:
                audience: /projects/1111111111/global/backendServices/222222
                issuer: https://cloud.google.com/iap
            pcb:
              skip:
                - query_engine: js
                  query: "try { return data.req.headers[\"x-goog-iap-jwt-assertion\"]; } catch (e) { return undefined; }"

@gfrankliu
Copy link
Author

BTW, the bug you found in jsonpath doesn't seem to exist in jsonata, so if I use query_engine: jsonata, I am able to use $.req.headers."my-jwt-header" like you suggested. Now the question is how I can have a pcb rule like:

                  rule:
                    method: eq
                    value: undefined

so I can skip if the header doesn't exist. I tried but the engine seems to treat "undefined" as literal string match :(

@travisghansen
Copy link
Owner

You still need the rule with the js engine as well FYI. I think the regex syntax you had may work if you simply to /.*/ but I would need to test to be sure..

@gfrankliu
Copy link
Author

Tried but didn't work.

@travisghansen
Copy link
Owner

I’ll look at this later in the day and get you a working example.

@travisghansen
Copy link
Owner

I think this will work with the existing codebase:

                - query_engine: js
                  query: "try { return data.req.headers[\"my-jwt-header\"]; } catch (e) { return ''; }"
                  rule:
                    method: regex
                    value: /\w/i
                    negate: true

Give it a try and let me know. I have some minor fixes to commit to make this work better due to javascript oddities but the above should do what you want.

@gfrankliu
Copy link
Author

It doesn't seem to work. When I send a request without jwt header, I expect to see "skipping plugin due to pcb", something like:

{"level":"info","message":"starting verify for plugin: jwt","service":"external-auth-server","timestamp":"2023-08-02T05:11:39.347Z"}
{"level":"info","message":"skipping plugin due to pcb assertions: jwt","service":"external-auth-server","timestamp":"2023-08-02T05:11:39.348Z"}

but I saw below instead:

{"level":"info","message":"starting verify for plugin: jwt","service":"external-auth-server","timestamp":"2023-08-02T05:26:56.251Z"}
{"level":"warn","message":"failed assertion: {\"query_engine\":\"js\",\"query\":\"try { return data.req.headers[\\\"my-jwt-header\\\"]; } catch (e) { return ''; }\",\"rule\":{\"method\":\"regex\",\"value\":\"/\\\\w/i\",\"negate\":true}} against value: undefined","service":"external-auth-server","timestamp":"2023-08-02T05:26:56.251Z"}

@gfrankliu
Copy link
Author

Maybe I don't need this pcb rule. I assume the default jwt plugin will already do the same: skip to next plugin if jwt header doesn't exist.

@travisghansen
Copy link
Owner

Yes it will. I typically would use the skip also with a stop (this helps ensure the response to applicable clients is more focused and relevant to the client). For example you don’t really want to redirect a machine to an oauth endpoint.

I think I know why you’re getting that, let me try 1 more variation and send it your way.

@travisghansen
Copy link
Owner

Yeah, update the query:

query: "try { return data.req.headers[\"my-jwt-header\"] || ''; } catch (e) { return ''; }"

@gfrankliu
Copy link
Author

Yes it will. I typically would use the skip also with a stop (this helps ensure the response to applicable clients is more focused and relevant to the client). For example you don’t really want to redirect a machine to an oauth endpoint.

Can you clarify the skip/stop? My understanding is: "skip" will go to next plugin without running the current plugin, but "stop" will NOT stop running the current plugin, it will still run the current plugin, just stop AFTER it finishes and won't go to next plugin.

In my test case, since "skip" is implicit behavior of jwt plugin (it will directly go to next plugin if the jwt header doesn't exist), I only need a "stop" pcb where jwt header exists?

"try { return data.req.headers["my-jwt-header"] || ''; } catch (e) { return ''; }"

Yes, this works! Thanks!

@travisghansen
Copy link
Owner

A stop without the skip will result in the 2nd plugin never getting executed. Basically you want to skip if the header is not present at all and stop if the header is present regardless of success/failure. stop will end the pipeline even if the result is a failure.

@gfrankliu
Copy link
Author

I tried below pcb "stop", without "skip":

            pcb:
              stop:
                - query_engine: js
                  query: "try { return data.req.headers[\"my-jwt-header\"] || ''; } catch (e) { return ''; }"
                  rule:
                    method: regex
                    value: /\w/i

Now if I send a request without the jwt header, it actually goes to the next plugin:

{"level":"info","message":"starting verify for plugin: jwt","service":"external-auth-server","timestamp":"2023-08-02T16:58:50.394Z"}
{"level":"warn","message":"failed assertion: {\"query_engine\":\"js\",\"query\":\"try { return data.req.headers[\\\"my-jwt-header\\\"] || ''; } catch (e) { return ''; }\",\"rule\":{\"method\":\"regex\",\"value\":\"/\\\\w/i\"}} against value: \"\"","service":"external-auth-server","timestamp":"2023-08-02T16:58:50.395Z"}
{"level":"info","message":"starting verify for plugin: oidc","service":"external-auth-server","timestamp":"2023-08-02T16:58:50.395Z"}

If I send the request with jwt header, it will stop at jwt plugin, whether the header is valid or not. In case an invalid jwt header, the plugin returns 401 and stops there. The problem is ingress-nginx will trigger the oauth redirect anyway when it gets 401 from "external-auth-server". I am using this nginx ingress config example. Client side gets 302 location: https://eas.example.com/auth/nginx/auth-signin?rd= from nginx where "rd=" is empty.

@travisghansen
Copy link
Owner

Oh right, that should work. The performance gains of using skip in this scenario are likely minimal.

Regarding the empty rd value maybe nginx snippet syntax has changed? I actually don't use nginx much so not entirely sure on that one :(

@gfrankliu
Copy link
Author

The empty rd in this case is expected because we put a stop at the jwt plugin and rd only makes sense for oauth/oidc plugins.
jwt plugin returns 401 but without a redirect URL to nginx (expected since jwt doesn't have a redirect URL).

What I tried to say in last post was the "stop" didn't prevent a redirect in this case :(

@travisghansen
Copy link
Owner

nginx doesn't allow for a redirect per-se in how it handles forward auth. So I have to respond with a 401 and then the extra config kicks in. It's rather stupid how nginx handles the pattern instead of just passing the non-2xx responses back to the client directly :(

@gfrankliu
Copy link
Author

Wondering if there is anything we can learn from how oauth2-proxy did it with nginx as documented here, with two annotations:

  annotations:
    nginx.ingress.kubernetes.io/auth-url: "https://$host/oauth2/auth"
    nginx.ingress.kubernetes.io/auth-signin: "https://$host/oauth2/start?rd=$escaped_request_uri"

They didn't do any nginx.ingress.kubernetes.io/configuration-snippet:

@travisghansen
Copy link
Owner

Give it a try and see what you get.

@gfrankliu
Copy link
Author

I can't try it until external-auth-server has the proper signin endpoint like oauth2-proxy does. Since external-auth-server supports multiple profiles, we will need additional query params to the signin endpoint, unlike oauth2-proxy where only ?rd=$escaped_request_uri is needed.

@travisghansen
Copy link
Owner

Well I do have an endpoint. Are you using ingress nginx or nginx ingress? Is eas exposed behind ingress nginx?

@gfrankliu
Copy link
Author

I am using ingress-nginx and already got it working using nginx.ingress.kubernetes.io/configuration-snippet as suggested in your example. Just trying to see if we can have a simplified ngnix signin endpoint that doesn't need nginx.ingress.kubernetes.io/configuration-snippet like what oauth2-proxy signin endpoint does.

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

2 participants