Remove managed_groups check in groups API

allow group admins to make group changes, even though manage_groups config may clobber them
This commit is contained in:
Min RK
2025-02-13 12:43:22 +01:00
parent c473a35459
commit 0a27724540
3 changed files with 32 additions and 17 deletions

View File

@@ -469,8 +469,19 @@ which is a list of group names the user should be a member of:
- If `None` is returned, no changes are made to the user's group membership - If `None` is returned, no changes are made to the user's group membership
If authenticator-managed groups are enabled, If authenticator-managed groups are enabled,
all group-management via the API is disabled, groups cannot be specified with `load_groups` traitlet.
and roles cannot be specified with `load_groups` traitlet.
:::{warning}
When `manage_groups` is True,
managing groups via the API is still permitted via the `admin:groups` scope (starting with 5.3),
but any time a user logs in their group membership is completely reset via the login process.
So it only really makes sense to make manual changes via the API that reflect upstream changes which are not automatically propagated, such as group deletion.
:::
:::{versionchanged} 5.3
Prior to JupyterHub 5.3, all group management via the API was disabled if `Authenticator.manage_groups` is True.
:::
(authenticator-roles)= (authenticator-roles)=

View File

@@ -3,6 +3,7 @@
# Copyright (c) Jupyter Development Team. # Copyright (c) Jupyter Development Team.
# Distributed under the terms of the Modified BSD License. # Distributed under the terms of the Modified BSD License.
import json import json
from warnings import warn
from tornado import web from tornado import web
@@ -35,6 +36,11 @@ class _GroupAPIHandler(APIHandler):
def check_authenticator_managed_groups(self): def check_authenticator_managed_groups(self):
"""Raise error on group-management APIs if Authenticator is managing groups""" """Raise error on group-management APIs if Authenticator is managing groups"""
warn(
"check_authenticator_managed_groups is deprecated in JupyterHub 5.3.",
DeprecationWarning,
stacklevel=2,
)
if self.authenticator.manage_groups: if self.authenticator.manage_groups:
raise web.HTTPError(400, "Group management via API is disabled") raise web.HTTPError(400, "Group management via API is disabled")
@@ -73,9 +79,6 @@ class GroupListAPIHandler(_GroupAPIHandler):
@needs_scope('admin:groups') @needs_scope('admin:groups')
async def post(self): async def post(self):
"""POST creates Multiple groups""" """POST creates Multiple groups"""
self.check_authenticator_managed_groups()
model = self.get_json_body() model = self.get_json_body()
if not model or not isinstance(model, dict) or not model.get('groups'): if not model or not isinstance(model, dict) or not model.get('groups'):
raise web.HTTPError(400, "Must specify at least one group to create") raise web.HTTPError(400, "Must specify at least one group to create")
@@ -115,7 +118,6 @@ class GroupAPIHandler(_GroupAPIHandler):
@needs_scope('admin:groups') @needs_scope('admin:groups')
async def post(self, group_name): async def post(self, group_name):
"""POST creates a group by name""" """POST creates a group by name"""
self.check_authenticator_managed_groups()
model = self.get_json_body() model = self.get_json_body()
if model is None: if model is None:
model = {} model = {}
@@ -143,7 +145,6 @@ class GroupAPIHandler(_GroupAPIHandler):
@needs_scope('delete:groups') @needs_scope('delete:groups')
def delete(self, group_name): def delete(self, group_name):
"""Delete a group by name""" """Delete a group by name"""
self.check_authenticator_managed_groups()
group = self.find_group(group_name) group = self.find_group(group_name)
self.log.info("Deleting group %s", group_name) self.log.info("Deleting group %s", group_name)
self.db.delete(group) self.db.delete(group)
@@ -157,7 +158,6 @@ class GroupUsersAPIHandler(_GroupAPIHandler):
@needs_scope('groups') @needs_scope('groups')
def post(self, group_name): def post(self, group_name):
"""POST adds users to a group""" """POST adds users to a group"""
self.check_authenticator_managed_groups()
group = self.find_group(group_name) group = self.find_group(group_name)
data = self.get_json_body() data = self.get_json_body()
self._check_group_model(data) self._check_group_model(data)
@@ -176,7 +176,6 @@ class GroupUsersAPIHandler(_GroupAPIHandler):
@needs_scope('groups') @needs_scope('groups')
async def delete(self, group_name): async def delete(self, group_name):
"""DELETE removes users from a group""" """DELETE removes users from a group"""
self.check_authenticator_managed_groups()
group = self.find_group(group_name) group = self.find_group(group_name)
data = self.get_json_body() data = self.get_json_body()
self._check_group_model(data) self._check_group_model(data)

View File

@@ -2238,13 +2238,15 @@ async def test_auth_managed_groups(request, app, group, user):
app.authenticator.manage_groups = True app.authenticator.manage_groups = True
request.addfinalizer(lambda: setattr(app.authenticator, "manage_groups", False)) request.addfinalizer(lambda: setattr(app.authenticator, "manage_groups", False))
# create groups # create groups
r = await api_request(app, 'groups', method='post') r = await api_request(
assert r.status_code == 400 app,
'groups',
method='post',
data=json.dumps({"groups": {"groupname": [user.name]}}),
)
assert r.status_code == 201
r = await api_request(app, 'groups/newgroup', method='post') r = await api_request(app, 'groups/newgroup', method='post')
assert r.status_code == 400 assert r.status_code == 201
# delete groups
r = await api_request(app, f'groups/{group.name}', method='delete')
assert r.status_code == 400
# add users to group # add users to group
r = await api_request( r = await api_request(
app, app,
@@ -2252,7 +2254,7 @@ async def test_auth_managed_groups(request, app, group, user):
method='post', method='post',
data=json.dumps({"users": [user.name]}), data=json.dumps({"users": [user.name]}),
) )
assert r.status_code == 400 assert r.status_code == 200
# remove users from group # remove users from group
r = await api_request( r = await api_request(
app, app,
@@ -2260,7 +2262,10 @@ async def test_auth_managed_groups(request, app, group, user):
method='delete', method='delete',
data=json.dumps({"users": [user.name]}), data=json.dumps({"users": [user.name]}),
) )
assert r.status_code == 400 assert r.status_code == 200
# delete groups
r = await api_request(app, f'groups/{group.name}', method='delete')
assert r.status_code == 204
# ----------------- # -----------------