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

Grant Admin Rights to custom SAML group #561

Open
jgallucci32 opened this issue Nov 15, 2021 · 7 comments
Open

Grant Admin Rights to custom SAML group #561

jgallucci32 opened this issue Nov 15, 2021 · 7 comments

Comments

@jgallucci32
Copy link

Is your feature request related to a problem? Please describe

When using SAML SSO, users must be added to the 'admin' group on the IdP as any changes made locally to the Nextcloud instance will be overwritten when the users logs in. There is no way to change or map a group to the 'admin' group in Nextcloud within the instance. This means the backend must have a group called 'admin' which does not clearly indicate what the group is for as it is likely the IdP is shared between multiple applications.

Describe the behaviour you'd like

There should be in option in the SAML configuration to grant members of a Group admin rights to the Nextcloud application. This way the IdP could have a group such as nextcloud-admins be associated with members who should have admin rights to Nextcloud.

Describe alternatives you've considered

Currently you can create a group within your IdP called 'admin'. For example, if using Keycloak you can create a local group called 'admin' and map the user to that group (you can do this with Roles as well).

Additional context

This community topic also describes the issue
https://help.nextcloud.com/t/how-to-give-saml-users-admin-rights/115468

@fschrempf
Copy link
Contributor

This means the backend must have a group called 'admin' which does not clearly indicate what the group is for as it is likely the IdP is shared between multiple applications.

Currently you can create a group within your IdP called 'admin'. For example, if using Keycloak you can create a local group called 'admin' and map the user to that group (you can do this with Roles as well).

I don't really see the problem here. In Keycloak you can create a client-specific role admin and then use either "Composite Roles" or group to assign this specific role from a global role/group, e. g. nextcloud-admin.

@Mercbot7
Copy link

Mercbot7 commented May 9, 2022

This may work in Keycloak, however how would this be done when not using Keycloak and using something like Okta? We do not want a VERY generic group called 'admin' in our IdP for a single application.

@hasechris
Copy link

I became aware of this issue because I too was looking for an answer to OP's question.

I don't really see the problem here. In Keycloak you can create a client-specific role admin and then use either "Composite Roles" or group to assign this specific role from a global role/group, e. g. nextcloud-admin.

Hmm... Please excuse my directness, but "works for me" has never been a good answer in IT. It just shows that someone cant or wont change his point of view, maybe because he likes his point of view a bit to much.

From an administrative point of view, having an admin/operator/normaluser group with the same technical name for each application is an absolute hindrance to building an understandable user and group structure. Then an admin simply has 5 groups with the same name "admin". With an active maintenance of the user backend, no one really understands this anymore.

In my humble opinion this is an absolute basic functionality of a user backend, to be able to use authorization groups with different administration rights. What else was the central user backend developed for?

Translated with www.DeepL.com/Translator (free version)

@Sx3
Copy link

Sx3 commented Aug 5, 2022

yes it is ,But for Azure Active Directory we can only get the Group ID.so if there is a way to map the NextCloud groups with Azure Group IDs will be helpful.

@kevinmccurdybrd
Copy link

For context, this is a significant limitation for us. We're planning to use SAML for Azure deployed instances of Nextcloud. As a direct result of this limitation, I must now add LDAP as well as SAML to make my groups work as designed, as I can not do it with SAML natively. (I'm using the same identifier for SAML and LDAP so the accounts become "one and the same".

I would MUCH rather put this thing on an island (ie no internal network access whatsoever, and no access to LDAP) but because of this specific issue, I can not delegate the "admin" role to admins without LDAP, decreasing the overall security of this new solution.

Ma27 added a commit to Ma27/user_saml that referenced this issue Sep 23, 2022
Fixes nextcloud#561

As stated in the issue, it's not desirable to have a group called
`admin` in the SAML backend which doesn't indicate to which service
admin permissions are granted.

This is orthogonal to `saml-attribute-mapping-group_mapping` which
simply maps all groups from a SAML attribute to Nextcloud groups, i.e.
the attribute's value MUST contain a group called `admin` to make sure
that users get admin rights in Nextcloud.

When enabled, the name of (another) attribute must be specified which
contains a list of SAML-specific groups, e.g.

    ["nextcloud-admins", "nextcloud-marketing"]

that can be mapped to e.g.

    ["admin", "marketing"]
Ma27 added a commit to Ma27/user_saml that referenced this issue Sep 23, 2022
Fixes nextcloud#561

As stated in the issue, it's not desirable to have a group called
`admin` in the SAML backend which doesn't indicate to which service
admin permissions are granted.

This is orthogonal to `saml-attribute-mapping-group_mapping` which
simply maps all groups from a SAML attribute to Nextcloud groups, i.e.
the attribute's value MUST contain a group called `admin` to make sure
that users get admin rights in Nextcloud.

When enabled, the name of (another) attribute must be specified which
contains a list of SAML-specific groups, e.g.

    ["nextcloud-admins", "nextcloud-marketing"]

that can be mapped to e.g.

    ["admin", "marketing"]

Signed-off-by: Maximilian Bosch <[email protected]>
@kevinmccurdybrd
Copy link

@blizzz Could we get an update on this? We really could use this functionality (and frankly may play a role if we move into Enterprise support or not for NC). @Ma27 provided a PR (#659), you countered that #545 was in development (since replaced with #662).

Without this functionality we have to use a mix of AzureAD SAML and LDAP to get desired group mappings and such, requiring on-premise connectivity. Having this issue resolved (and group passthrough working in general) would offer a huge advantage to what I'm sure is a large number of users of this suite. This is the difference between on-prem connectivity required or not, and would allow us to up the overall security of our final solution significantly.

@blizzz
Copy link
Member

blizzz commented Apr 3, 2023

Priorities, I am afraid. #662 is still on the list.

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

Successfully merging a pull request may close this issue.

7 participants