diff --git a/docs/source/reference/authenticators.md b/docs/source/reference/authenticators.md index 46b09f59..eb1d95ec 100644 --- a/docs/source/reference/authenticators.md +++ b/docs/source/reference/authenticators.md @@ -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 authenticator-managed groups are enabled, -all group-management via the API is disabled, -and roles cannot be specified with `load_groups` traitlet. +groups 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)= diff --git a/jupyterhub/apihandlers/groups.py b/jupyterhub/apihandlers/groups.py index e908b2a3..84055000 100644 --- a/jupyterhub/apihandlers/groups.py +++ b/jupyterhub/apihandlers/groups.py @@ -3,6 +3,7 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. import json +from warnings import warn from tornado import web @@ -35,6 +36,11 @@ class _GroupAPIHandler(APIHandler): def check_authenticator_managed_groups(self): """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: raise web.HTTPError(400, "Group management via API is disabled") @@ -73,9 +79,6 @@ class GroupListAPIHandler(_GroupAPIHandler): @needs_scope('admin:groups') async def post(self): """POST creates Multiple groups""" - - self.check_authenticator_managed_groups() - model = self.get_json_body() 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") @@ -115,7 +118,6 @@ class GroupAPIHandler(_GroupAPIHandler): @needs_scope('admin:groups') async def post(self, group_name): """POST creates a group by name""" - self.check_authenticator_managed_groups() model = self.get_json_body() if model is None: model = {} @@ -143,7 +145,6 @@ class GroupAPIHandler(_GroupAPIHandler): @needs_scope('delete:groups') def delete(self, group_name): """Delete a group by name""" - self.check_authenticator_managed_groups() group = self.find_group(group_name) self.log.info("Deleting group %s", group_name) self.db.delete(group) @@ -157,7 +158,6 @@ class GroupUsersAPIHandler(_GroupAPIHandler): @needs_scope('groups') def post(self, group_name): """POST adds users to a group""" - self.check_authenticator_managed_groups() group = self.find_group(group_name) data = self.get_json_body() self._check_group_model(data) @@ -176,7 +176,6 @@ class GroupUsersAPIHandler(_GroupAPIHandler): @needs_scope('groups') async def delete(self, group_name): """DELETE removes users from a group""" - self.check_authenticator_managed_groups() group = self.find_group(group_name) data = self.get_json_body() self._check_group_model(data) diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index 6cd90231..428f1e59 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -2238,13 +2238,15 @@ async def test_auth_managed_groups(request, app, group, user): app.authenticator.manage_groups = True request.addfinalizer(lambda: setattr(app.authenticator, "manage_groups", False)) # create groups - r = await api_request(app, 'groups', method='post') - assert r.status_code == 400 + r = await api_request( + 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') - assert r.status_code == 400 - # delete groups - r = await api_request(app, f'groups/{group.name}', method='delete') - assert r.status_code == 400 + assert r.status_code == 201 # add users to group r = await api_request( app, @@ -2252,7 +2254,7 @@ async def test_auth_managed_groups(request, app, group, user): method='post', data=json.dumps({"users": [user.name]}), ) - assert r.status_code == 400 + assert r.status_code == 200 # remove users from group r = await api_request( app, @@ -2260,7 +2262,10 @@ async def test_auth_managed_groups(request, app, group, user): method='delete', 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 # -----------------