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

Mutator issuing JWT with custom claims #228

Closed
piotrmsc opened this issue Jul 25, 2019 · 6 comments
Closed

Mutator issuing JWT with custom claims #228

piotrmsc opened this issue Jul 25, 2019 · 6 comments

Comments

@piotrmsc
Copy link

piotrmsc commented Jul 25, 2019

Is your feature request related to a problem? Please describe.

Feature request :) open for contribution ;-)

Describe the solution you'd like
Oathkeeper has id token mutator which issues JWT and with id token calls the upstream. What would be nice to have is a mutator that issues JWT but allows to add custom claims to that JWT, based on input provided from different service.

Additional claims could be used in the mutator by for example fetching information (claims) from a different place (can be a rest/graphql/grpc service) which provides information for the given request.

This could be used in scenarios when you want to call the upstream in the oathkeeper with some additional context such as tenant using claims in in JWT and perhaps even headers.
Describe alternatives you've considered

Additional context

Add any other context or screenshots about the feature request here.

@aeneasr
Copy link
Member

aeneasr commented Jul 25, 2019

Any ideas on the API / configuration side?

@piotrmsc
Copy link
Author

If we think of new mutator then I think the current API (the interface) does not have to be changed.
New mutator would just fetch additional claim from the given place (http service,grps,graphql or even some kind of storage) and add it in the Mutate method of https://github.com/ory/oathkeeper/blob/master/pipeline/mutate/mutator.go#L39.

If we think of extending current JWT mutator then I think we have the same situation as with new one.

I think that then access rule should have configuration extended to point from where fetch additional claims perhaps on the mutator level. What do you think?

@aeneasr
Copy link
Member

aeneasr commented Jul 29, 2019

For this to work we decided to:

  • Change mutator from single to multiple (similar to authenticator)
  • Introduce an RPC Mutator (or "enhancer" - name TBD)
  • Update the ID Token Mutator to make it possible to define custom claims easily

Mutator changes

We would allow our mutators to handle multiple steps. Mutators are called from top to bottom. If a mutator fails with an error, we send a 403 or 500 error code and stop propagation.
Mutators can mutate the state of AuthenticationSession.

  {
    "id": "test-rule-2",
    "upstream": { ... },
    "match": { ... },
    "authenticators": [ ... ],
    "authorizer": [ ... ],
-    "mutator": {
-      "handler": "id_token"
-    }
+   "mutators": [
+     {
+        "handler": "enhancer",
+      },
+     {
+        "handler": "id_token",
+      }
+   ]
+},

RPC Mutator

We still need a good name here.

Our RPC Mutator would basically make an upstream call to an API with AuthenticationSession as payload and expect AuthenticationSession as a response. It should also forward request headers and maybe set stuff like "X-Forwarded-For".

func (a *RPCMutator) Mutate(r *http.Request, session *authn.AuthenticationSession, config json.RawMessage, _ pipeline.Rule) (http.Header, error) {
	var b bytes.Buffer

	_ = json.NewEncoder(&b).Encode(session)

	req, _ := http.NewRequest("POST", config.URL, &b)

	// for all headers
	req.Header.Add(r.Header.Get(key)

	res, _ = http.Do(req)
	// ...
	json.NewDecoder(res.Body).Decode(session)

	return nil, nil
}

I think it would be a good idea to support multiple protocols. For now however, HTTP is enough:

  • http(s)
  • grpc
  • ...

ID Token MUtator

We should support tempaltes in the ID Token Mutator, similar to how Cookies and Headers are doing it.

The config could look like:

{
	"handler": "id_token",
	"config": {
		"claims": {
			"user": "{{ print .Subject }}",
			"original_issuer": "{{ print .Extra.iss }}",
			"original_audience": "{{ print .Extra.aud }}",
		}
	}
}

I think that modifying values exp, iss, iat, sub should not have an effect as that could be used maliciously.

@piotrmsc
Copy link
Author

piotrmsc commented Jul 31, 2019

If mutators array is present I think mutator field should be ignored and mutators should have omitEmpty on struct for backward compatibility.

@aeneasr
Copy link
Member

aeneasr commented Jul 31, 2019

Yeah, I think we can also simply deprecate mutator completely!

@jakkab
Copy link
Contributor

jakkab commented Aug 8, 2019

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

3 participants