Allow clients to be issued ID Tokens valid for other clients

[ericchiang]

This is what Google calls "Cross-client Identity". Allow clients to be issued ID Tokens which are valid for a different client.

Example: "client1" allows "client2" to create ID Tokens on it's behalf. "client2" uses a custom scope to request such a token, and the returned ID Token holds the following claims:

{
    "iss": "{{ dex URL }}",
    "aud": "userID",
    "aud": "client1",
    "azp": "client2"
}

Where azp is the authorized party (the party that requested the ID Token) and aud is the client the ID Token is valid for.

[bobbyrullo]

Proposal

Data Model

Clients have a new field called "cross_client_authorizers" which is a list of client_ids. If a client_id is in this list it can mint tokens on behalf of the given client. There are no reciprocal minting rights.

Data Storage

It's probably fine to store as a comma-delimited list of strings; could also be another table, but that seems like overkill.

CRUD API

REST methods for getting/adding/removing to be added to worker and overlord APIs

• Worker API: full CRUD per client for authorizers - can't add during creation of client (don't want to deviate from dynamic client reg API)
• Admin API: can only add during creation of client (for bootstrapping)

Authentication

• Worker API: The Client must use their credentials to make changes
• Admin API: The admin key must be used.

OIDC Interaction

Consider two clients:

• Minter: This is the client that the user is authenticating with
• Mintee: This is the client that Minter is trying to mint a token for. In Mintee's list of cross_client_authorizers is Minter's client_id

When the user authenticates with Minter minter will add a scope of audience:server:client_id:$MINTEES_CLIENT_ID

When the ID token is returned to the user, it will have within it's aud the client_id's of the Minter and Mintee. The azp field will have Minter's client_id

UI

There will have to be added UI to the auth page letting the user know the extra scope being added

Refresh Token

Because of the complications w/r/t multiple audiences (eg. refresh tokens are scoped to a single client) I suggest that requests for cross-client minting with offline access will be rejected for now.

[bobbyrullo]

cc: @ericchiang @joeatwork @sym3tri

[bobbyrullo]

Alternatives to Not Allowing Refresh Tokens

1. We could allow refresh tokens to be bound to multiple clients. This is not my favorite idea: it goes against the spec, and complicates our data model.
2. Instead of returning both Mintee and Minter in the aud we could just have mintee
3. building on that, we could allow multiple scopes, and only allow refresh tokens if there's one.
4. In the multiple scope world, you'd have to add the scope for your own client, eg. mintee would have to add itself as a scope
5. We could return both mintee and minter in the scope but only give a refresh token for mintee
6. We could have a special scope for offline access of other clients.

[sym3tri]

• Is this claim name part of the OIDC spec? If not we should consider a different name? audience:server:client_id:$MINTEES_CLIENT_ID
• I'm not a fan of the terms Minter and Mintee in the code/docs/config.
• For the UI, I think it should be informational only (no action required by the user)
• I support rejecting mutli-audience refresh tokens for the time-being.

[bobbyrullo]

• Is this claim name part of the OIDC spec? If not we should consider a different name? audience:server:client_id:$MINTEES_CLIENT_ID

It's not part of any spec, it comes from Google's implementation of more-or-less the same thing. The advantages of naming it the same thing is that it's familiar to people, and could help in an effort to rally around something and make it a spec. The disadvantage is that it's not exactly the same (Google has a bunch of unspecified behavior and restrictions)

What don't you not like about it? I'm happy to use something different, but need a bit more direction.

Also note: that's not the claim name, that's the name of the scope.

• I'm not a fan of the terms Minter and Mintee in the code/docs/config.

Sure, that was just expedient and explains what is happening fairly well for the purposes of this proposal.

• For the UI, I think it should be informational only (no action required by the user)

This is part of the authentication flow, so whatever action they would normally have to take, they take. They still need to actually authenticate somehow.

• I support rejecting mutli-audience refresh tokens for the time-being.

Cool - the reason I called it out was because it could be useful in cases where someone can't kubectl auth login but we can consider it for the future.

[ericchiang]

It's probably fine to store as a comma-delimited list of strings; could also be another table, but that seems like overkill.

Might be nice have a separate table. Make it easier to do things programmatically through SQL for migrations, queries ("tell me all the clients that can make refresh tokens on my behalf"), etc. Just a thought.

Other than that lgtm.

[sym3tri]

Might be nice have a separate table.

FWIW postgres has had pretty good array support for a while now. We could have the best of both worlds (no extra table AND easily queryable). This comes at the cost of adding more postgres-specific implementation (but that may be ok if longer-term we decide to have a separate driver per supported storage type).

http://www.postgresql.org/docs/current/static/arrays.html

[sym3tri]

@bobbyrullo thanks for responding. I think we're on the same page. all LGTM.

[bobbyrullo]

This comes at the cost of adding more postgres-specific implementation

Yeah, that would be a bummer

...may be ok if longer-term we decide to have a separate driver per supported

true, but it would be nice if the various SQL implementations shared as much code as possible.

I think a table a separate table is probably the way to go.

[ericchiang]

@bobbyrullo cool with closing this now that #426 is in?

[bobbyrullo]

I still have at least another PR to deal with refresh tokens. I'll close it out when the work is done

[bobbyrullo]

With #465, this is closed. Still need UI work, for which I will open a separate issue.