Bug 52302 - Remove call to /univention/get/modules for every page load
Remove call to /univention/get/modules for every page load
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: Portal
UCS 4.4
Other Linux
: P5 normal (vote)
: UCS 4.4-6-errata
Assigned To: Dirk Wiesenthal
Florian Best
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2020-11-03 05:31 CET by Dirk Wiesenthal
Modified: 2021-09-16 00:25 CEST (History)
2 users (show)

See Also:
What kind of report is it?: Feature Request
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Wiesenthal univentionstaff 2020-11-03 05:31:21 CET
All users, even anonymous users, make a call to /univention/get/modules which involves a UMC-web-server <-> UMC-server request.

This can be omitted. This request could be done, e.g., if we know that the user is logged in and in the group "Domain Admins".
Comment 1 Dirk Wiesenthal univentionstaff 2020-11-03 05:34:38 CET
Only user that were in "Domain Admins" during the start of the portal server are considered. This is a heuristic. But better than doing it always.

dataport/pullcord b359a2f613
Comment 2 Florian Best univentionstaff 2020-11-09 13:17:10 CET
Please use univention.lib.misc.custom_groupname('Domain Admins', ucr).
Please introduce a UCR variable which defines the allowed groups.
Comment 3 Dirk Wiesenthal univentionstaff 2020-11-17 21:51:31 CET
Fixed in
  univention-portal 3.0.2-13A~4.4.0.202011172128

UCRV: portal/admin_groups. Comma separated value that names all groups that are considered "portal admins"
Comment 4 Florian Best univentionstaff 2020-11-18 13:40:03 CET
OK: the request is prevented if the user is not part of portal/admin_groups
~OK: I would have named the UCR variable: portal/admin-groups
FYI: there is debugging code pushed
FYI: there is itertools.chain()
FYI: a not existing group will cause this exception, and prevents the portal server from being started:
Traceback (most recent call last):
  File "/usr/bin/univention-portal-server", line 504, in <module>
    app = make_app()
  File "/usr/bin/univention-portal-server", line 494, in make_app
    admins = [grp.getgrnam(admin_group_name.strip())[3] for admin_group_name in admin_groups]  # [["Administrator"], ["admin1", "admin2"]]
KeyError: 'getgrnam(): name not found: foo'

OK: YAML
Comment 5 Florian Best univentionstaff 2020-11-18 16:19:44 CET
OK: variable renaming
OK: error handling

univention-portal (3.0.2-14)
220098478464 | Bug #52302: Changelog
eece1d0e1f69 | Bug #52302: Some minor code fixes
Comment 7 technik 2021-09-15 23:12:00 CEST
In order to gain editing rights in the (web) portal, a pre-defined group containing the domain admin group is not sufficient, since the user must be member of the domain admin group as well. This breaks the groups the user belongs to, because the membership of the domain admin group needs to be set up twice which is not a good idea at all.
Comment 8 Dirk Wiesenthal univentionstaff 2021-09-15 23:33:37 CEST
(In reply to technik from comment #7)
> In order to gain editing rights in the (web) portal, a pre-defined group
> containing the domain admin group is not sufficient, since the user must be
> member of the domain admin group as well. This breaks the groups the user
> belongs to, because the membership of the domain admin group needs to be set
> up twice which is not a good idea at all.

I am not sure if I understood your scenario correctly. But let me say that this bug was fixed under the impression of massive performance issues due to the lockdown and thousands and thousands of user logins at the same time.

Maybe doing this may help as a workaround:

ucr set portal/admin-groups="Domain Admins,portal-admins,My Other Portal Admins"
service univention-portal-server restart

This may not be ideal, but it may work?

In UCS 5.0, this should not be an issue. Groups in groups should work for admin computation.

Same with the following issue: Granting and / or revoking admin rights requires a restart of the service in UCS 4.4. Again, this is a tradeoff to gain performance. UCS 5.0 does not have this issue.
Comment 9 technik 2021-09-16 00:25:06 CEST
> ucr set portal/admin-groups="Domain Admins,portal-admins,My Other Portal
> Admins"
> service univention-portal-server restart
> 
> This may not be ideal, but it may work?

Thanks for your quick reply :)

UCS 5.0 is used where this error occurs and your workaround sadly does not fix it. 

I want to use a self-defined group, since it is way more convenient to set up groups for rdp access on windows clients ths way instead of providing every single user. Otherwise removing a user from a client machine takes way more time than a removal in its group, especially in terms of security.

For the record:

ucr get portal/admin-groups

returned the group I defined and used to be empty before. The default value is "Domain Admins" which is also part of this (self-defined) group. So leaving this empty should be ok since it matches the group needed for this.

The portal-admin group does not exist here, could this be the reason for it?