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

[WIP] proposal: extending ldap support for different types of users #19

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kleesc
Copy link
Member

@kleesc kleesc commented Jul 8, 2022

No description provided.


## Risks and Mitigations

- The implementation of this proposal should not affect or block any of Quay.io's deployments. Any features, when implemented should be feature flagged, as not every deployment uses a federated identity service like LDAP.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

conversely, this feature should be easy to add to quay deployments already using LDAP- the specification of the LDAP groups for superusers or restricted users should be done independently from configuring the LDAP provider itself.


Another use case would be to limit who can or cannot create
organization, as it is currently not enforced at all. (i.e any user is
able to create and squat on a organization's namespace).
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how generic can we get here? do we have a generic way to check these entitlements without having to add checks all over our code?

Comment on lines +54 to +63
### Restricted users

Similarly, we can define another different set of LDAP users, with
restricted access to content. One of the use case would be to give
temporary, or read-only access to such user (for auditing purposes,
for example).

Another use case would be to limit who can or cannot create
organization, as it is currently not enforced at all. (i.e any user is
able to create and squat on a organization's namespace).
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This use case is fundamental to many enterprise users of Quay, this includes installations that rely on the Quay database to provide authentication and not LDAP. As a result we need to make this new user persona configurable outside of LDAP search queries.

Open to opinions here but one way to do it is to either define all users to "restricted users" by default and manually enumerate the type of users who do not fall into this category, very much like we do with superusers today. We could call them privileged users.


Restricted users are a subset of Quay users whose capabilites will be a subset of current regular Quay users (excluding superusers). Some limitations we're going to impose are:
- Inability to create organizations
- Inability to write (Feature flagged. Should this be enforced on owner's namespace, or just enforced through quota?)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking enforced on the owner's namespace, otherwise we'd be coupling quota to security.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will also prevent users from writing to their own org, right?

- Inability to write (Feature flagged. Should this be enforced on owner's namespace, or just enforced through quota?)
- Feature flag/whitelist to default new users as restricted (when not using LDAP, or other federated provider)

Like superusers, restricted users are defined in LDAP using a filter or group. When LDAP is not used, unlike how superusers are defined in a list in Quay's configuration, restricted users are defined based on a provided whitelist (i.e users are defined as restricted unless specified in that configuration list).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will we handle situations where users are members of multiple ldap groups that have different levels of permissions? For example, a group called admin that contains a user bob and a group called users that also contains bob. Guessing highest level of permission will be what is granted?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd would expect it to be that: highest level of permission will be what is granted.


In addition to current superuser capabilities, this implementation will add the ability for superusers to access contents from outside its permission scope. This is a very specific use case and would only be enabled if the given FEATURE flag is set. This would allow superusers, on top of taking ownership of other namespaces, read, write, and delete contents from other namespaces, without breaking backward compatibility (i.e exising installations should be able to opt-in to get this behavior).

In terms of permission, access to resources not owned by the user would be granted based on whether or not that user has the existing superuser grant (scope) or not. This means that superusers would essentially bypass standard authentication in these cases. TODO: security implications.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this include user orgs?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any concerns about a superuser being able to sidestep auditing?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really sidestepping auditing? I read here that it would be sidestepping standard authentication. And I am not sure why it would have anything to do how the user authenticates. The superuser still always has to authenticate with credentials. Is it possible that this was meant to say "authorization"? If so, the standard authorization should be adjusted to always grant the superuser to anything it request. It is the superuser after all.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that this was meant to say "authorization"?

I think so. Superuser actions should still be audited though, but giving them the ability to do anything makes it sound like they could then tinker with auditing...which would defeat the purpose of auditing the actions.

Copy link
Contributor

@bcaton85 bcaton85 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments. Also had another question. If the superuser permission is removed from LDAP, would there be any gap in time to where that takes effect? Would it happen on their next action that needs to be authenticated or would it be on the next time they log in?


## Constraints

- This proposal would only be for deployments making use of a federated identity with Quay (LDAP, keystone, ...)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this specific to LDAP? How will the other identity providers be implemented?


### Restricted users

Similarly, we can define another different set of LDAP users, with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to give write permissions to a user in the restricted LDAP group through the Quay UI? What permissions model takes precedence?

federated user identity interface (`data/users/`) to select a specific
set of users. For example, being able to return a list of superusers
from an LDAP deployment. Another example, for a Quay deployment using
LDAP, Quay could limit creation of organizations to a specific set of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see retrieving a list of superusers being referenced often here. What is retrieving that list going to be used for?

Also how are the permissions going to be checked? Are they going to be validated against the LDAP server on each request? Are they going to be retrieved initially and stored in a JWT and check the permissions on the JWT for future requests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants