goauthentik / authentik

The authentication glue you need.
https://goauthentik.io
Other
13.75k stars 926 forks source link

website/docs: Add note about single group per role #12169

Closed pklaschka closed 2 days ago

pklaschka commented 4 days ago

Details

This change adds an admonition to document the fact that every role can only ever be assigned to a single group at the same time. Since this is surprising based on a traditional understanding of role-based models, I've decided to make this a :::warning.

I'm undecided on the best place for this information, but for now, decided on putting it into the context of the action that can fail: assigning a role to a group.

While this does not close the issue, it documents this behavior to at least address the "needs documentation" aspect of #10983 .


Checklist

If an API change has been made

If changes to the frontend have been made

If applicable

netlify[bot] commented 4 days ago

Deploy Preview for authentik-docs ready!

Name Link
Latest commit 979149238dd011230acb7ce5b8193cc199a86ed3
Latest deploy log https://app.netlify.com/sites/authentik-docs/deploys/67433779f3578f0008f0f7af
Deploy Preview https://deploy-preview-12169--authentik-docs.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] commented 4 days ago

Deploy Preview for authentik-storybook ready!

Name Link
Latest commit 979149238dd011230acb7ce5b8193cc199a86ed3
Latest deploy log https://app.netlify.com/sites/authentik-storybook/deploys/6743377997f17b000814d858
Deploy Preview https://deploy-preview-12169--authentik-storybook.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

tanberry commented 2 days ago

Hi @pklaschka thanks so much for this PR. It also helped me catch the fact that I need to update the docs to use the new "dual-select" UI component that is now in the UI for selecting roles. I think we introduced it in version 2024.8, but I failed to update the docs about assigning roles to a group. Which version are you running?

On your point, though, about not being able to assign a single role (say Role X) to multiple groups... I don't think that is correct. I was able to assign a role called Multiple Group Role to two different Groups.

Maybe you mean "at the same time" as in during same action? That is true; one has to go to each Group separately, and assign the role.

codecov[bot] commented 2 days ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 92.43%. Comparing base (3d5a189) to head (9791492). Report is 18 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #12169 +/- ## ========================================== - Coverage 92.67% 92.43% -0.24% ========================================== Files 761 761 Lines 38025 38025 ========================================== - Hits 35239 35149 -90 - Misses 2786 2876 +90 ``` | [Flag](https://app.codecov.io/gh/goauthentik/authentik/pull/12169/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goauthentik) | Coverage Ξ” | | |---|---|---| | [e2e](https://app.codecov.io/gh/goauthentik/authentik/pull/12169/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goauthentik) | `48.55% <ΓΈ> (-0.62%)` | :arrow_down: | | [integration](https://app.codecov.io/gh/goauthentik/authentik/pull/12169/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goauthentik) | `24.83% <ΓΈ> (ΓΈ)` | | | [unit](https://app.codecov.io/gh/goauthentik/authentik/pull/12169/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goauthentik) | `90.21% <ΓΈ> (ΓΈ)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goauthentik#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

pklaschka commented 2 days ago

Hi @tanberry , thanks for your quick response!

On your point, though, about not being able to assign a single role (say Role X) to multiple groups... I don't think that is correct. I was able to assign a role called Multiple Group Role to two different Groups.

If I assign a role (Role X) to a group (Group 1) while it is already assigned to a different group (Group 2), I get a 400 Bad Request with the message

["Roles can only be used with a single group."]

in the underlying call to PATCH /api/v3/core/groups/[group-id]/. In the UI, it shows a non-descriptive error message (which this documentation is, at best, a band-aid for, but better than nothing):

image

When I unassign the role from its current group assignment (Group 2) and try the same thing again (assigning Role X to Group 1), it works.

This is the case in 2024.10.4, at least… So on my end, the behavior matches the behavior in #10983 (where @BeryJu confirmed that limitation).

pklaschka commented 2 days ago

My best guess would be that this limitation stems from this OneToOneField:

https://github.com/goauthentik/authentik/blob/5e72ec9c0c9d826c048627180d96a3e94df7bedd/authentik/rbac/models.py#L38-L47

But I have no experience with Django, so this is nothing more than an educated guess by a non-Django dev (and also makes limited sense as it's technically a 1:n instead of a 1:1 relationship – whereas the expectation would be N:M) πŸ˜‰ .

tanberry commented 2 days ago

Aha, I learned a ton here, including the fact that I need to do a search on the docs_needed label! :-)

I want to do a bit more research about exactly which version this changed in (I suspect it is above in your notes but I need more coffee), and see if Notes are needed about that.

But in meantime, let's merge your PR, and then I will open one to update the steps to use the dual-select component.