vmware-tanzu / pinniped

Pinniped is the easy, secure way to log in to your Kubernetes clusters.
https://pinniped.dev
Apache License 2.0
565 stars 66 forks source link

Multiple IDP Support in Supervisor for different pools of users accessing same cluster #960

Closed anjaltelang closed 1 year ago

anjaltelang commented 2 years ago

Ability for Pinniped Supervisor to support federating access to Kubernetes cluster(s) (single/multiple) for users from Multiple Identity providers.

Details in Additional Context below.

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

Describe the solution you'd like A clear and concise description of what you want to happen.

Describe alternatives you've considered Deploying multiple Supervisor instances as a stop gap solution

Are you considering submitting a PR for this feature?

Additional context Context A cluster administrator trying to configure and setup authentication to multiple kubernetes clusters has to deal with many challenges including the need to configure Identities from multiple Identity providers. For instance, there maybe a need to provide cluster access for a set of platform admins who can configure the cluster and then allow access to developer/testers using a different identity provider. There may also be a need to have separate Identity providers for users and service accounts. It might also be that an organization will need to setup same identity providers with different configurations for OIDC such as with and without password grant flow.

Currently the Pinniped Supervisor only supports working with a single Identity provider. Admins can work around this by using Identity providers such as Dex or Okta that can have multiple connectors for supporting multiple Identity sources. This however adds complexity to the deployment and makes it difficult to support the usecases for the cluster administrator.

Notes:

  1. Transformations include: Filtering which is removing certain Usernames and Groups defined by Identity Provider, Altering user names as well as group names, Policy for allowing or disallowing users and groups.
  2. IDPs/IDP refer to the actual Identify Providers that handle Identity management for Users and not the IDP Custom Resource created in Pinniped.

Goals The purpose of this feature is to add support in the Pinniped Supervisor to configure multiple identity providers. This should include support for:

  1. Single Identity provider with multiple different configurations
  2. Multiple identity provider configurations (>1 IDPs)
  3. Ability to handle overlapping identities from two or more IDPs. For e.g An organization may have a Developer called “Mo” who uses GitHub and also has a GitLab account. Mo should be able to access the cluster using either of the identities and it should be known which identity Mo is using for a cluster at a given time. Additionally, this feature should also support handling of overlapping Group names from different IDPs.

Non Goals

  1. There may be Identity transforms(For e.g. Filtering, Altering or applying Policy to group names or usernames) that can be applied to the Identities we get from the providers such as the ability to add upcasing/downcasing of usernames and groups or the ability to add users in groups before giving cluster access. These are not currently in scope of the work to support Multiple Identity providers and can be reviewed and added at a later time based on feedback from users.
  2. Changes to ActiveDirectoryIdentityProvider, LDAPDirectoryIdentityProvider, OIDCIdentityProvider CRD Spec Schemas are not in scope

Use cases

  1. Some organizations might keep their administrator accounts in one IDP with regular user accounts in another IDP
  2. An administrator might like to allow human users to log in via OIDC with two-factor authentication, while allowing only non-human service accounts from LDAP (or OIDC password grant) to authenticate for CI/CD use cases.
  3. Support configuring OIDC clients differently for different types of user or for different clusters, we need to allow multiple OIDCIdentityProviders. For example, two-factor auth is required for admins but not for deployment service accounts.
  4. Support configuring LDAP clients differently for human accounts and service accounts in different branches of the LDAP tree When using AD or LDAP as your upstream IDP, each LDAPIdentityProvider or ActiveDirectoryIdentityProvider can only have one user search base. 

enj commented 2 years ago

This story needs to be expanded with what the expectations are around the isolation of identities (if any) from these different sources.

anjaltelang commented 2 years ago

Isolation/tenancy is per IDP Client app.

enj commented 2 years ago

Ability for the Pinniped Supervisor to federate access to a single Kubernetes cluster for users from multiple identity providers.

Why a single Kubernetes cluster? Pinniped is all about multi-cluster auth?

A managed service provider could create a FederationDomain for each customer (potentially attached to each customer’s own IDP), to configure auth separately for each customer’s clusters.

I thought this was explicitly out of scope because a single supervisor process does not provide sufficient security/DOS isolation between federation domains (all keys held in memory, all sessions held in memory, etc).


I believe the answer is "no" to this particular use case, but I wanted to ask/record it here:

Sometimes admins need to combine multiple IDPs into a single IDP, i.e. they want to use one IDP to get username information and a different IDP/SCIM to get the group information. For example, they may want users to login via OIDC and have groups synced in from LDAP. Note that this is the only way to to configure IDPs in OpenShift, and Rancher has a similar approach.

anjaltelang commented 2 years ago

Ability for the Pinniped Supervisor to federate access to a single Kubernetes cluster for users from multiple identity providers.

Why a single Kubernetes cluster? Pinniped is all about multi-cluster auth?

@enj Yes absolutely it is a multi-cluster use case. It maybe the framing but what I am trying to convey is that admin should be able to configure access for 'a cluster' as well as for 'multiple clusters' from different Identity providers. I will make the change in the wording above. Thanks!

anjaltelang commented 2 years ago

A managed service provider could create a FederationDomain for each customer (potentially attached to each customer’s own IDP), to configure auth separately for each customer’s clusters.

I thought this was explicitly out of scope because a single supervisor process does not provide sufficient security/DOS isolation between federation domains (all keys held in memory, all sessions held in memory, etc).

@enj In that case I am okay to move this for post MVP work.

anjaltelang commented 2 years ago

Sometimes admins need to combine multiple IDPs into a single IDP, i.e. they want to use one IDP to get username information and a different IDP/SCIM to get the group information. For example, they may want users to login via OIDC and have groups synced in from LDAP. Note that this is the only way to to configure IDPs in OpenShift, and Rancher has a similar approach.

@enj This is not scoped for MVP but if we hear this requirement from users we can add this support at a later time.

margocrawf commented 2 years ago

Ability to configure different search bases in the same IDP as separate IDPs, for example different domains across an AD forest. This feature capability should be handled at the IDP level.

We already get this for free if we support Multiple identity provider configurations (>1 IDPs). I'm not sure what it means for it to be a non-goal.

margocrawf commented 2 years ago

Can we specify how exactly we want to accomplish this goal:

Ability to handle overlapping identities from two or more IDPs. For e.g An organization may have a Developer called “Mo” who uses GitHub and also has a GitLab account. Mo should be able to access the cluster using either of the identities and it should be known which identity Mo is using for a cluster at a given time.

Since this is a non-goal

There may be additional Identity transforms that can be applied to the Identities we get from the providers such as the ability to add upcasing/downcasing of usernames and groups or the ability to add users in groups before giving cluster access. These are not currently in scope of the work to support Multiple Identity providers and can be reviewed and added at a later time based on feedback from users.

anjaltelang commented 2 years ago

Ability to configure different search bases in the same IDP as separate IDPs, for example different domains across an AD forest. This feature capability should be handled at the IDP level.

We already get this for free if we support Multiple identity provider configurations (>1 IDPs). I'm not sure what it means for it to be a non-goal.

@margocrawf I believe @enj had mentioned that this type of use case is best handled at IDP level vs adding anything specific in our design to handle this scenario. If Multiple IDP MVP can cover this use case, that is great but it is a non-goal.

anjaltelang commented 2 years ago

Can we specify how exactly we want to accomplish this goal:

Ability to handle overlapping identities from two or more IDPs. For e.g An organization may have a Developer called “Mo” who uses GitHub and also has a GitLab account. Mo should be able to access the cluster using either of the identities and it should be known which identity Mo is using for a cluster at a given time.

Since this is a non-goal

There may be additional Identity transforms that can be applied to the Identities we get from the providers such as the ability to add upcasing/downcasing of usernames and groups or the ability to add users in groups before giving cluster access. These are not currently in scope of the work to support Multiple Identity providers and can be reviewed and added at a later time based on feedback from users.

  • Should our manner of handling overlapping identities be a prefix, suffix, or something else?
  • Also is filtering (e.g. only the Gitlab users in this specific group are allowed to log in) a goal or non-goal and can it be added to the appropriate section on this issue? (I think we came to a decision about this but I can't remember what it was...)

I feel this is a design discussion on how to handle keeping the identities separate from the two IDPs. Filtering is also a part of transforms and not part of the goals

cfryanr commented 2 years ago

I have some questions and thoughts regarding this one requirement from above:

Ability to handle overlapping identities from two or more IDPs.

My understanding from other discussions is that @anjaltelang would prefer if we could somehow automatically always make it impossible for usernames and group names to conflict when there are multiple IDPs configured.

Some related background:

Let's say there are two external IDPs configured. Let's call the IDPs "A" and "B".

The problem that I'm pointing out here:

Here's a modest proposal to change the MVP requirements. Pinniped could:

Any thoughts @anjaltelang @margocrawf @enj @shamus, or anyone else?

enj commented 2 years ago

Ability to handle overlapping identities from two or more IDPs.

My understanding from other discussions is that @anjaltelang would prefer if we could somehow automatically always make it impossible for usernames and group names to conflict when there are multiple IDPs configured.

+1

* The usernames and group names will partly determine how the admin writes their RBAC policies. Pinniped has no opinion on how RBAC is configured, and Pinniped's only goal in this regard is to make it possible to refer to usernames and group names as they were configured for extraction from the external IDP. For example, if an enterprise decides that usernames should be email addresses so they can automatically define/synchronize RBAC policy based on corporate email addresses, then Pinniped should allow that.

In my experience the specific strings used matter very little to admins, they just need to be predictable so that automation can be written to handle them. Security conscious admins assign RBAC to group UUIDs and forgo any username based RBAC policies (Azure AD does this by default IIRC).

* To make conflicts impossible, I think Pinniped would need to modify all usernames and all group names from both IDPs in some way that guarantees that they are put into separate logical namespaces. Or am I missing something? For example, Pinniped could prefix all usernames and group names from IDP A with "A:" and similarly prefix all usernames and group names from IDP "B" with "B:".

(Lets assume that prefixing is the approach we want to take)

A simple approach to handle this is to disallow : in all usernames and filter out all groups with a : in their name (OpenShift always did this and it was never an issue for customers). The first IDP's usernames are left unmodified and the second is automatically prefixed with a string that ends in a :.

* I think it is not sufficient to only prefix names from one of the IDPs.

See above.

* It seems to me that the reality of what it takes to automatically make identity conflicts impossible goes against another design goal of Pinniped: admin users should be able to configure what the usernames and group names look like, so that they can retain control over how usernames and group names appear in audit logs, RBAC policies, etc.

I think as long as usernames are understandable by a human (for the purpose of audit logs) and groups named are predictable, admins have everything they need.

For example, Rancher generates UIDs for both usernames and group names and uses these to assign RBAC policies. The admin assigns policy through an interface, and the system handles the mapping. The extra field of the user info is used to include the human readable username for the purpose of audit logs.

* For example, if Pinniped automatically prefixed all usernames from IDP A with "A:", then it would be impossible for the admin user to define RBAC policies based on corporate email addresses

I do not believe automation around RBAC would have any harder time with a:email vs email. I also think the approach I described below handles this case well.

* Make it easy for an admin to configure Pinniped with multiple IDPs such that identity conflicts are impossible in a way that lets them control the resulting names.
* Maybe even make conflict avoidance the default behavior when it is not specifically configured otherwise. For example, by automatically prefixing all usernames and group names with the name/type of the IDP when no other prefix is configured, or something similar.

(Again, lets assume that prefixing is the approach we want to take)

The simplest approach I can think of:

  1. Disallow : as described above
  2. First IDP identities are never prefixed
  3. All other IDP identities are prefixed with idp_name: (there is no way to opt-out or change this prefix, but the admin can name the IDP however they want to; IDPs of different types with the same name are explicitly disallowed within a single federation domain; system and pinniped are disallowed as IDP names to prevent conflicts with well-known identities)
* Make it possible for the admin to configure Pinniped such that usernames and/or group names from certain IDPs are not changed by Pinniped.

-1, I do not believe this is necessary.

In my experience, multiple IDPs is a relatively rare and advanced use case, of which overlapping identities is a minor use case. Most commonly, I have seen it used to workaround deficiencies of the IDP (i.e. the OIDC IDP doesn't support password grant so just use LDAP directly to get around the limitation). We do not need to make it easy to create unsafe configurations to handle this scenario.

In the approach described above, and even today via multiple supervisors, it is possible to create overlaps if one is determined:

  1. Create federation domain X and assign it IDP A
  2. Create federation domain Y and assign it IDP B
  3. Have clusters trust both X and Y with multiple JWT authenticators

This requires some extra YAML, but I think the extra friction is desirable here.

I remain skeptical that allowing multiple JWT authenticators is a good idea, but it is what we do today. I would feel better if the only way to have multiple JWT authenticators required running multiple concierges under different API groups, but that is a different topic.

anjaltelang commented 2 years ago

Ability to handle overlapping identities from two or more IDPs.

My understanding from other discussions is that @anjaltelang would prefer if we could somehow automatically always make it impossible for usernames and group names to conflict when there are multiple IDPs configured.

+1

* The usernames and group names will partly determine how the admin writes their RBAC policies. Pinniped has no opinion on how RBAC is configured, and Pinniped's only goal in this regard is to make it possible to refer to usernames and group names as they were configured for extraction from the external IDP. For example, if an enterprise decides that usernames should be email addresses so they can automatically define/synchronize RBAC policy based on corporate email addresses, then Pinniped should allow that.

In my experience the specific strings used matter very little to admins, they just need to be predictable so that automation can be written to handle them. Security conscious admins assign RBAC to group UUIDs and forgo any username based RBAC policies (Azure AD does this by default IIRC).

* To make conflicts impossible, I think Pinniped would need to modify all usernames and all group names from both IDPs in some way that guarantees that they are put into separate logical namespaces. Or am I missing something? For example, Pinniped could prefix all usernames and group names from IDP A with "A:" and similarly prefix all usernames and group names from IDP "B" with "B:".

(Lets assume that prefixing is the approach we want to take)

A simple approach to handle this is to disallow : in all usernames and filter out all groups with a : in their name (OpenShift always did this and it was never an issue for customers). The first IDP's usernames are left unmodified and the second is automatically prefixed with a string that ends in a :.

+1 on both disallowing ':' I think not prefixing the first IDP in the list will also help users that are upgrading from older version to newer version. They can have the first IDP as their default IDP and add more as needed.

* I think it is not sufficient to only prefix names from one of the IDPs.

See above.

* It seems to me that the reality of what it takes to automatically make identity conflicts impossible goes against another design goal of Pinniped: admin users should be able to configure what the usernames and group names look like, so that they can retain control over how usernames and group names appear in audit logs, RBAC policies, etc.

I think as long as usernames are understandable by a human (for the purpose of audit logs) and groups named are predictable, admins have everything they need.

For example, Rancher generates UIDs for both usernames and group names and uses these to assign RBAC policies. The admin assigns policy through an interface, and the system handles the mapping. The extra field of the user info is used to include the human readable username for the purpose of audit logs.

* For example, if Pinniped automatically prefixed all usernames from IDP A with "A:", then it would be impossible for the admin user to define RBAC policies based on corporate email addresses

I do not believe automation around RBAC would have any harder time with a:email vs email. I also think the approach I described below handles this case well.

* Make it easy for an admin to configure Pinniped with multiple IDPs such that identity conflicts are impossible in a way that lets them control the resulting names.
* Maybe even make conflict avoidance the default behavior when it is not specifically configured otherwise. For example, by automatically prefixing all usernames and group names with the name/type of the IDP when no other prefix is configured, or something similar.

(Again, lets assume that prefixing is the approach we want to take)

The simplest approach I can think of:

  1. Disallow : as described above
  2. First IDP identities are never prefixed
  3. All other IDP identities are prefixed with idp_name: (there is no way to opt-out or change this prefix, but the admin can name the IDP however they want to; IDPs of different types with the same name are explicitly disallowed within a single federation domain; system and pinniped are disallowed as IDP names to prevent conflicts with well-known identities)

+1 On the suggestion above

* Make it possible for the admin to configure Pinniped such that usernames and/or group names from certain IDPs are not changed by Pinniped.

-1, I do not believe this is necessary.

I don't see an explicit need for this.

In my experience, multiple IDPs is a relatively rare and advanced use case, of which overlapping identities is a minor use case. Most commonly, I have seen it used to workaround deficiencies of the IDP (i.e. the OIDC IDP doesn't support password grant so just use LDAP directly to get around the limitation). We do not need to make it easy to create unsafe configurations to handle this scenario.

In the approach described above, and even today via multiple supervisors, it is possible to create overlaps if one is determined:

  1. Create federation domain X and assign it IDP A
  2. Create federation domain Y and assign it IDP B
  3. Have clusters trust both X and Y with multiple JWT authenticators

I would not want to allow overlap. Also considering the fact that multiple Fed domains cause this, why don't we allow only a single Federation domain with Multiple IDPs associated with them?

This requires some extra YAML, but I think the extra friction is desirable here.

I remain skeptical that allowing multiple JWT authenticators is a good idea, but it is what we do today. I would feel better if the only way to have multiple JWT authenticators required running multiple concierges under different API groups, but that is a different topic.

cfryanr commented 2 years ago

Thanks for the responses.

I could imagine an approach similar to what you are describing being sufficient for the MVP of multiple IDP support. It's not as flexible as I would prefer to advance Pinniped's vision of supporting any IDP, any cluster, and any authZ system. But it could meet the MVP requirements.

Assuming that we wanted to move forward with an approach like that, here are some thoughts on implementation details for that approach.

  1. Disallow : as described above

I think we would need to be careful how we go about reserving a character and disallowing that character in usernames and groups. For example, if we chose : to be the reserved character, then we would be causing problems even with our own code. The default behavior of our OIDCIdentityProvider already puts : characters into usernames. And : is a perfectly reasonable character to put into a group name in almost any IDP. I'm not aware of any character that is generally avoided or generally disallowed by IDPs which we could ultimately rely upon. I could imagine that we allow the admin user to choose that separator for their FederationDomain, and default it to something reasonable like #. The default would hopefully work for most cases, and when it doesn't then an admin could change it. Whatever separator is configured could be both disallowed by Pinniped in usernames and groups names from all IDPs for that FederationDomain, and also used by Pinniped as the separator for automatic prefixing for IDPs in that FederationDomain, as you described.

  1. First IDP identities are never prefixed

It might be nice instead for the user to choose which IDP in the list is the one which does not get auto-prefixed. It feels subtle for the ordering in the list to have this implied side effect. Perhaps the user can pick 0 or 1 IDPs for which they would like to disable auto-prefixing (not more than 1). By default, all listed IDPs could get auto-prefixed.

In an upgrade situation, a FederationDomain would have no IDPs listed on it, since we did not previously allow listing IDPs on FederationDomains. In that case, if we want to allow for smooth upgrades, we could keep our existing behavior of letting the FederationDomain automatically pick the one and only defined IDP (and error whenever there is more than one IDP). In this case, it would also be safe to not prefix that IDP's identities. If the admin later updates the FederationDomain to explicitly list the pre-existing IDP along with some new second IDP, then they could choose to disable prefixing for the pre-existing IDP if they would like to keep pre-existing usernames and group names unchanged for that IDP.

  1. All other IDP identities are prefixed with idp_name: (there is no way to opt-out or change this prefix, but the admin can name the IDP however they want to; IDPs of different types with the same name are explicitly disallowed within a single federation domain

That should be an easy validation. That would imply that you also can't list the same IDP twice in the same FederationDomain, which seems okay.

As a side note, IDP CRs can be named using lower case alphanumeric characters, - or ., and must start and end with an alphanumeric character, and are limited to 253 characters. So that rule would then effectively become the allowed names of the auto-prefixes too (not including the separator). Seems reasonable.

system and pinniped are disallowed as IDP names to prevent conflicts with well-known identities)

We could instead (or additionally) prevent this at login time. If any user in any way got one of those special prefixes during a login, then we could handle it and output a nice log message. We could reject the login or drop the offending group or however we decide to handle it. This would also handle the case where the one IDP which is not being auto-prefixed returned a username or group name starting with these disallowed prefixes.

enj commented 2 years ago

(I jumped around a bit)

  1. Disallow : as described above

I think we would need to be careful how we go about reserving a character and disallowing that character in usernames and groups. For example, if we chose : to be the reserved character, then we would be causing problems even with our own code. The default behavior of our OIDCIdentityProvider already puts : characters into usernames. And : is a perfectly reasonable character to put into a group name in almost any IDP. I'm not aware of any character that is generally avoided or generally disallowed by IDPs which we could ultimately rely upon. I could imagine that we allow the admin user to choose that separator for their FederationDomain, and default it to something reasonable like #. The default would hopefully work for most cases, and when it doesn't then an admin could change it. Whatever separator is configured could be both disallowed by Pinniped in usernames and groups names from all IDPs for that FederationDomain, and also used by Pinniped as the separator for automatic prefixing for IDPs in that FederationDomain, as you described.

I did a quick experiment with Okta/GitHub/GitLab/JumpCloud. All of them disallow : in usernames (and HTTP basic auth disallows it in the spec) and only JumpCloud/Okta allowed : in group names (I was surprised by JumpCloud here because I am pretty sure it violates the LDAP DN RFC 4514). Basically I do not think this restriction causes problems in practice, because I would have heard of it during my time at Red Hat since there is simply no way to have a : in a username or a group name in OpenShift (and I did hear complaints about many other limitations).

I am confident that we can find ways to make such a restriction work with our existing defaulting.

I am against any configuration around the character used for the separator because it makes it impossible to rely on consistency across envs that folks can build on. For example, RBAC against a synthetic group pinniped:authenticated that all users belong to in all envs. This is not possible if the separator can be arbitrarily changed. Configuration should be backed by evidence of its need, and I do not believe we have that here.

  1. All other IDP identities are prefixed with idp_name: (there is no way to opt-out or change this prefix, but the admin can name the IDP however they want to; IDPs of different types with the same name are explicitly disallowed within a single federation domain

That should be an easy validation. That would imply that you also can't list the same IDP twice in the same FederationDomain, which seems okay.

We might just simply want to disallow the same name to be used across IDPs altogether. They are logically the same resource with different config.

As a side note, IDP CRs can be named using lower case alphanumeric characters, - or ., and must start and end with an alphanumeric character, and are limited to 253 characters. So that rule would then effectively become the allowed names of the auto-prefixes too (not including the separator). Seems reasonable.

Probably worth thinking about if we want to restrict this further.

system and pinniped are disallowed as IDP names to prevent conflicts with well-known identities)

We could instead (or additionally) prevent this at login time. If any user in any way got one of those special prefixes during a login, then we could handle it and output a nice log message. We could reject the login or drop the offending group or however we decide to handle it. This would also handle the case where the one IDP which is not being auto-prefixed returned a username or group name starting with these disallowed prefixes.

I think we can figure out the specifics in the full proposal. We will probably want multiple levels of validation.

  1. First IDP identities are never prefixed

It might be nice instead for the user to choose which IDP in the list is the one which does not get auto-prefixed. It feels subtle for the ordering in the list to have this implied side effect. Perhaps the user can pick 0 or 1 IDPs for which they would like to disable auto-prefixing (not more than 1). By default, all listed IDPs could get auto-prefixed.

In an upgrade situation, a FederationDomain would have no IDPs listed on it, since we did not previously allow listing IDPs on FederationDomains. In that case, if we want to allow for smooth upgrades, we could keep our existing behavior of letting the FederationDomain automatically pick the one and only defined IDP (and error whenever there is more than one IDP). In this case, it would also be safe to not prefix that IDP's identities. If the admin later updates the FederationDomain to explicitly list the pre-existing IDP along with some new second IDP, then they could choose to disable prefixing for the pre-existing IDP if they would like to keep pre-existing usernames and group names unchanged for that IDP.

That sounds reasonable.

cfryanr commented 1 year ago

This has been superseded by several other GitHub issues.