BTBurke / caddy-jwt

JWT middleware for the Caddy server
MIT License
113 stars 40 forks source link

Extra claims from auth0 cause invalid headers #28

Closed q7r closed 6 years ago

q7r commented 6 years ago

Auth0 provides extra claims in the following way:

  "https://auth.mydomain.com/app_metadata": {
    "authorization": {
      "user": "o1ek",
      "groups": [
        "group1",
        "group2"
      ]
    }

It causes the following errors:

14/Aug/2017:15:04:09 +0200 [ERROR 502 /] net/http: invalid header field name "Token-Claim-HTTPS://AUTH.MYDOMAIN.COM/APP_METADATA.AUTHORIZATION.GROUPS"
14/Aug/2017:15:04:10 +0200 [ERROR 502 /favicon.ico] net/http: invalid header field name "Token-Claim-HTTPS://AUTH.MYDOMAIN.COM/APP_METADATA.AUTHORIZATION.USER"
jimjimovich commented 6 years ago

This is a problem for me as well. Auth0 now forces this namespacing of extra claims.

The offending character seems to be the slash (/). Would it be possible to convert all slashes to another character?

BTBurke commented 6 years ago

My bad, I had notifications turned off and didn't see this issue pop up. Should be an easy fix. Are these metadata fields useful? My initial inclination is just to drop any claim information with a / in it and not pass it via header.

jimjimovich commented 6 years ago

Yes, these headers are extremely necessary for me. Auth0 forces any custom claims to be namespaced with a valid URL. (https://auth0.com/docs/api-auth/tutorials/adoption/scope-custom-claims)

My first thought was to simply replace any / with something like _ so that o1ek's example would change from "Token-Claim-HTTPS://AUTH.MYDOMAIN.COM/APP_METADATA.AUTHORIZATION.USER" to "Token-Claim-HTTPS:__AUTH.MYDOMAIN.COM_APP_METADATA.AUTHORIZATION.USER"

That works as a valid header, but is still pretty ugly.

What would be really excellent would be a configuration option that would allow you to remove this namespace. Something like:

namespace_replace [namespace] [repacevalue]

This would be very flexible and allow for something like this:

from "Token-Claim-HTTPS://AUTH.MYDOMAIN.COM/APP_METADATA.AUTHORIZATION.USER" to "Token-Claim-APP_METADATA.AUTHORIZATION.USER"

jimjimovich commented 6 years ago

By the way, the app metadata thing seems to be something specific to the OP's app. Auth0 forces this namespace for any custom claims. For example:

{ email: "joe@example.com", ... https://www.site.com/name: "Joe", https://www.site.com/state: "Indiana" }

These currently create headers like Token-Claim-HTTPS://WWW.SITE.COM/NAME

These headers work internally inside of Caddy (like in templates) but throw errors for some things like the proxy directive.

BTBurke commented 6 years ago

I was wondering about that, because I looked up the RFC for HTTP headers and / is an allowed character in headers.

BTBurke commented 6 years ago

I think the right answer for this is probably to URL encode the claim key, perhaps with an optional directive to strip the URL from the header up to the last /

So with no configuration you would get:

Token-Claim-HTTPS%3A%2F%2FAUTH.MYDOMAIN.COM%2FAPP_METADATA.AUTHORIZATION.USER

With something like this:

mysite.com {
...
strip_header
...
}

You get:

Token-Claim-APP_METADATA.AUTHORIZATION.USER
jimjimovich commented 6 years ago

That sounds great. The URL encoding would make a sane default, and the strip_header option would be even better.

The only thing that comes to mind with URL encoding is that some existing claims/headers might change on people with a Caddy upgrade. That might be a bit unexpected for people.

BTBurke commented 6 years ago

This is ready to go in v3.0.0. Unfortunately, there's no way to force a specific plugin version in the Caddy build server so this introduces some breaking changes. Hopefully, it won't cause people a lot of problems.

Breaking changes:

So for the above example you'll get

Token-Claim-Https:%2F%2Fauth.mydomain.com/app_metadata.authorization.user: o1ek

With strip_header you get:

Token-Claim-App_metadata.authorization.user: o1ek