get groups from authenticator; improve group mgmt on admin page #3307
Conversation
* Authenticator can supply a group field with group memberships for user Groups will be created if no already existing * Manage groups via admin page * Add and delete groups * assign and revoke groups to/from users
|
Thanks for submitting your first pull request! You are awesome! 🤗 |
jupyterhub/apihandlers/groups.py
Outdated
| self.db.commit() | ||
| deleted.append(group) | ||
| self.write(json.dumps([self.group_model(group) for group in deleted])) | ||
| self.set_status(201) |
There was a problem hiding this comment.
What is the correct return code for bulk delete
There was a problem hiding this comment.
has to be 200, as 204 does not allow to have a body
| @@ -195,7 +203,7 @@ async def get(self, name): | |||
| @admin_only | |||
| async def post(self, name): | |||
| data = self.get_json_body() | |||
| user = self.find_user(name) | |||
There was a problem hiding this comment.
No need for the full User Object?!
jupyterhub/apihandlers/users.py
Outdated
| @@ -244,7 +259,7 @@ async def delete(self, name): | |||
|
|
|||
| @admin_only | |||
| async def patch(self, name): | |||
| user = self.find_user(name) | |||
There was a problem hiding this comment.
Do we really need the full User object here?
There was a problem hiding this comment.
Is there a downside to consistency? In nearly all cases, the user is in memory already, and this adds only a single dict lookup to return the object.
jupyterhub/apihandlers/groups.py
Outdated
| self.db.commit() | ||
| deleted.append(group) | ||
| self.write(json.dumps([self.group_model(group) for group in deleted])) | ||
| self.set_status(201) |
jupyterhub/apihandlers/users.py
Outdated
| @@ -244,7 +259,7 @@ async def delete(self, name): | |||
|
|
|||
| @admin_only | |||
| async def patch(self, name): | |||
| user = self.find_user(name) | |||
There was a problem hiding this comment.
Is there a downside to consistency? In nearly all cases, the user is in memory already, and this adds only a single dict lookup to return the object.
jupyterhub/apihandlers/users.py
Outdated
| else: | ||
| setattr(user, key, value) | ||
| self.db.commit() | ||
| user_ = self.user_model(user) | ||
| user_['auth_state'] = await user.get_auth_state() | ||
| user_['auth_state'] = await self._user_from_orm(user).get_auth_state() |
There was a problem hiding this comment.
I see by this call to _user_from_orm that the above find_user really is required, so it should probably be put back.
| @@ -734,6 +735,15 @@ async def auth_to_user(self, authenticated, user=None): | |||
| if admin is not None and admin != user.admin: | |||
| user.admin = admin | |||
| self.db.commit() | |||
| # add user to the given groups | |||
| if groups is not None: | |||
|
As #3548 is merged, I close this PR. |
Authenticator can supply a group field with group memberships for user
Groups will be created if no already existing
Manage groups via admin page
Self clearing fixtures for user(s) and group(s)