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

OIDC integration should support arbitrary claims for authorization subjects #512

Closed
jokraehe opened this issue Oct 7, 2019 · 21 comments
Closed
Labels
community-interest Issues which were explicitly asked for by the Ditto community.
Milestone

Comments

@jokraehe
Copy link
Contributor

jokraehe commented Oct 7, 2019

The current implementation reads the "sub" claim only to create authorization subjects from a JSON web token. We should provide configuration options to support any claims.

Implementation suggestion (see below for discussion how we got there) for the configuration "syntax":

google = {
  issuer = "accounts.google.com"
  auth-subjects = [
    "{{ jwt:sub }}",
    "{{ jwt:sub }}/{{ jwt:scope }}",
    "{{ jwt:sub }}/{{ jwt:scope }}@{{ jwt:client_id }}",
    "{{ jwt:sub }}/{{ jwt:scope }}@{{ jwt:non_existing }}",
    "{{ jwt:roles/support }}"
  ]
}
@w4tsn
Copy link
Contributor

w4tsn commented Apr 27, 2020

Is there any advance in this topic?

@thjaeckle thjaeckle added the community-interest Issues which were explicitly asked for by the Ditto community. label Apr 27, 2020
@thjaeckle
Copy link
Member

No, we don't have that on our short term agenda currently.

@JulianFeinauer
Copy link
Contributor

Just to get the issue clear. Is the aim to get arbitrary claims as the authorization subject or should the claims be used otherwise, also?

@ffendt
Copy link
Contributor

ffendt commented May 15, 2020

I think the idea behind this was to provide configuration on how (multiple) authorization subjects would be generated from the claims of a JWT. So basically a mini-DSL for generating the subjects.

So just a small example to clarify what I think this means:
For a token

{
  "sub": "ffendt",
  "client_id": "ditto-users",
  "scp": [
    "user",
    "admin"
   ],
   ...
}

and a configuration for the issuer [sub]+[...scp][-@client_id] (where '...' -> 'each array value' and '-' -> 'optional'), Ditto would generate a list of auth subjects:

[
  "ffendt+user@ditto-users",
  "ffendt+user",
  "ffendt+admin@ditto-users",
  "ffendt+admin"
]

@w4tsn
Copy link
Contributor

w4tsn commented May 15, 2020

@JulianFeinauer @ffendt

Just to get the issue clear. Is the aim to get arbitrary claims as the authorization subject or should the claims be used otherwise, also?

In my understanding the main goal is to support e.g. roles and group claims in subjects such that I may add a OIDC role or group to a policy entry instead of a user id. This would already be much more flexible and allow access management being made from e.g. keycloak instead of ditto policies directly. With this I could write policy entries reflecting roles and add these roles to users to give them more capabilites.

Apart from that your idea @ffendt sounds interesting. Could you elaborate a bit more about the use-case that this could enable? I don't have a good feeling about what possibilities this would open up ;)

@JulianFeinauer
Copy link
Contributor

This is an interesting topic and I see two different tasks in it. I agree with @ffendt in the sense that in some systems it may be preferable to use e.g. the email instead of the subject e.g. for consistency with other systems.
On the other hand I really lik @w4tsn idea of moving roles or groups in a policy.
Probably the most generic and probably "simple" way of achieving that would be to have some kind of "filters" which could be added to policies based on metadata that the user could have. So besides the prefix and subject as it is right now we could add a key-value map to the user with attribues. And from oauth we could do a generic mapping e.g. of groups and roles in there so that a filter on "METADATA["role_admin"] != null" (we could use arbitrary engine and syntax here, js, JEXL, ...) would allow all users with oatuh role admin.

@Yannic92
Copy link
Contributor

I'm not sure what the benefit of having METADATA is vs sticking to the already existing mechanism of multiple auth subjects.

If I understood @ffendt correctly you could do the same thing by using the subjects.

and a configuration for the issuer [sub]+[...scp][-@client_id] (where '...' -> 'each array value' and '-' -> 'optional'), Ditto would generate a list of auth subjects:

[
"ffendt+user@ditto-users",
"ffendt+user",
"ffendt+admin@ditto-users",
"ffendt+admin"
]

If I got this idea right, it allows to configure any structure of the subject that will be generated. This means you could for example configure [-sub+][scp] which would then generate the following list of subjects:

[
  "user",
  "ffendt+user",
  "admin",
  "ffendt+admin"
]

This way you also have a subject which reflects only the role (admin and user) taken from the scp.

Did I miss something of your approach @JulianFeinauer?

@JulianFeinauer
Copy link
Contributor

@Yannic92 thanks for the response. Then I missunderstood @ffendt a bit, I guess. I though he wants to genreate only ONE "subject string". But if we generate multiple ones I agree, then its better than my suggestion and easier :)
Sorry for the confusion!

@w4tsn
Copy link
Contributor

w4tsn commented Oct 15, 2020

It took me a moment to fully understand the described approach of @ffendt and I think it's a solid approach!

So one of the next steps then is to propose a actual DSL for this then? Some ideas / implementation notes:

As additional note about the nature of claims: they are auth provider dependent and since they are JSON could have any type such as array, string, boolean, long, int, (nested) object. Therefore the DSL has to support something like JSON pointer syntax and the value has to be parsed accordingly. E.g. keycloak supports to put a list of attribute values into something like resource_access.${client_id}.roles.

If the value of a claim is a list we can assume to use all it's contents for the auth subjects. No need to specify it explicitly with [...scp] because it's not useful to specify a certain list entry (the order could be unreliable). We could of course use this to enforce or loosen the claim parsing: allowing a claim to be either string or array or the other way around reject the JWT / claim if it doesn't match the configured type.

@thjaeckle
Copy link
Member

If the value of a claim is a list we can assume to use all it's contents for the auth subjects. No need to specify it explicitly with [...scp] because it's not useful to specify a certain list entry (the order could be unreliable).

I agree on that.

Regarding the DSL: yes, that one would have to be defined. I just searched if there already is a standard/library for defining such things, but I didn't find any.
We could of course use a "Json transformation" library like e.g. Jolt, but it's really crazy what that library and its JSON based DSL can do (also crazy difficult to learn).

So I guess defining our own DSL and parser would be the best option.

@thjaeckle
Copy link
Member

The DSL would be part of the openid-connect-issuers configuration in "gateway.conf" (and could be specified/overwritten via environment variable or system property).

I guess it would make sense to "break" the config format in order to e.g. define for a issuer - so change from plain string to object in the config.

In order to start simple, I would suggest:

  • to configure a list of JsonPointers (in [...] braces) combined with optional "static" characters in between
  • if any of the used JsonPointers of one entry can't be resolved/found in the JWT, that complete entry is not added
  • each added [<pointer>] may be of type string or type array of strings
    • for type string a single auth subject is extracted
    • for type array of strings multiple auth subjects are extracted

Example config:

google = {
  endpoint = "accounts.google.com"
  auth-subjects-extractors = [
    "[sub]",
    "[sub]+[scp]",
    "[sub]+[scp]@[client_id]",
    "[sub]+[scp]@[non_existing]",
    "[roles/support]"
  ]
}

Example token:

{
  "sub": "ffendt",
  "client_id": "ditto-users",
  "scp": [
    "user",
    "admin"
   ],
  "roles": {
    "support": [
      "call-center-agent",
      "support.admin"
    ]
  }
}

Example extracted auth subjects (to be used in policy "subjects"):

ffendt
ffendt+user
ffendt+admin
ffendt+user@ditto-users
ffendt+admin@ditto-users
call-center-agent
support.admin

WDYT? Could this be a simple start already solving 80% of the use cases?

@JulianFeinauer
Copy link
Contributor

This looks very good @thjaeckle . Woudl at least solve 100% of my use cases (use email and probably use roles / groups). 👍

@w4tsn
Copy link
Contributor

w4tsn commented Oct 15, 2020

@thjaeckle I'd like a shorter config option. Just throwing in some ideas:

claims = [...]
auth-subjects = [...]
claim-mapper = [...]
subject-mapper = [...]
parser-rules = [...]

My favorit being simply auth-subjects since that's what we are looking for to extract from the JWT. This actually gives me another idea: would it be interesting to allow fixed subjects in this list such that one could bind policy entries directly to auth-providers? Not sure how useful this is but we e.g. have a strict realm-per-customer infrastructure where it could potentially become a use-case to give "basic permissions" to everything coming from one provider.

@thjaeckle
Copy link
Member

Yes, auth-subjects is totally fine here 👍

I think fixed subjects would "automatically" be supported if you don't use the [...] notation inside an entry.
That could really proof useful as some kind of "all users" group for all users which could successfully authenticate with a specific issuer.

@jokraehe
Copy link
Contributor Author

I like your approach @thjaeckle, but I have a little issue with the placeholder syntax. I would prefer braces style that we use in connectivity configuration. Maybe even with a type declaration (string/array).

google = {
  issuer = "accounts.google.com"
  auth-subjects = [
    "{{ sub:string }}",
    "{{ sub:string }}/{{ scope:array }}",
    "{{ sub:string }}/{{ scope:array }}@{{ client_id:string }}",
    "{{ sub:string }}/{{ scope:array }}@{{ non_existing:string }}",
    "{{ roles/support:array }}"
  ]
}

@thjaeckle
Copy link
Member

@jokraehe that syntax is also good for me 👍
However I don't think the ":string" or ":array" is of much help here - as potentially all claims could be either string or array and we must be able to handle either string or array.

Another proposal, use "json" or "claim" as prefix:

google = {
  issuer = "accounts.google.com"
  auth-subjects = [
    "{{ claim:sub }}",
    "{{ claim:sub }}/{{ claim:scope }}",
    "{{ claim:sub }}/{{ claim:scope }}@{{ claim:client_id }}",
    "{{ claim:sub }}/{{ claim:scope }}@{{ claim:non_existing }}",
    "{{ claim:roles/support }}"
  ]
}

@jokraehe
Copy link
Contributor Author

I had that exact same thought on the prefix 👍
The type would make the config more like a contract to which the provider has to conform. I assume one provider would always return e.g. the "scope" claim as array. But you are right, it isn't of much help here and the convention to allow string or array for any claim is also fine by me.

@thjaeckle
Copy link
Member

I think we reached an agreement on the format (I updated the summary accordingly).
I still don't know when the Ditto team will get the chance to work on this, so I suggest, if this is time-critical for someone, to provide a PullRequest.

Implementation hints:

  • the config interface OAuthConfig would need to be enhanced to contain the new issuer and auth-subjects config entries
  • create new Placeholder implementation for ClaimPlaceholder extends Placeholder<JsonObject> (with claim: prefix) which extracts arbitrary claims from a passen in JWT JsonObject
    • add a new facctory method to PlaceholderFactory
  • enhance JwtAuthenticationProvider which creates an AuthenticationResult based on a passed in JsonWebToken to make use of the
    • new configured auth-subjects patterns
    • in combination with the new introduced ClaimPlaceholder in order to resolve claims from placeholders
    • pass in the OAuthConfig which the JwtAuthenticationFactory holds when creating new JwtAuthenticationProvider instances

@thjaeckle thjaeckle added this to the 2.0.0 milestone Dec 15, 2020
@oscarfh
Copy link
Contributor

oscarfh commented Jan 15, 2021

Hi,
Is there any updates on this regarding @JulianFeinauer's suggestion in #509 (comment) ?

@thjaeckle
Copy link
Member

@oscarfh not that I am aware of.
As Ditto team we have no interest in supporting multi-tenancy in Ditto, so that one would have to come from the community.

@oscarfh
Copy link
Contributor

oscarfh commented Jan 18, 2021

Ok, thanks a lot for the feedback, @thjaeckle !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-interest Issues which were explicitly asked for by the Ditto community.
Projects
None yet
Development

No branches or pull requests

7 participants