Reinstate recentchanges_view POST handler#11897
Reinstate recentchanges_view POST handler#11897mekarpeles merged 2 commits intointernetarchive:masterfrom
recentchanges_view POST handler#11897Conversation
Adds auth checks to handler. Use is restricted to admins and super-librarians
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Pull request overview
Restores the recentchanges_view POST handler used by the Recent Changes UI to “Undo All” (e.g., undoing merges), adding authentication and restricting access to admins and super-librarians.
Changes:
- Add
recentchanges_view.POSThandler to perform an undo and redirect back to the change page - Require a logged-in user and membership in admin/super-librarians usergroups for this endpoint
- Gate undo behind the
undofeature flag
| @@ -16,6 +16,7 @@ | |||
| render_template, | |||
| safeint, | |||
| ) # TODO: unused import? | |||
There was a problem hiding this comment.
The inline comment # TODO: unused import? is inaccurate here: safeint is used later in this module (e.g., for limit/offset parsing). Please remove or update the comment to avoid misleading future cleanup.
| ) # TODO: unused import? | |
| ) |
| allowed_usergroups = ['/usergroup/admin', '/usergroup/super-librarians'] | ||
| if not (user := get_current_user()) or not ( | ||
| user.is_member_of_any(allowed_usergroups) | ||
| ): |
There was a problem hiding this comment.
Consider moving allowed_usergroups to a module-level constant (e.g., ALLOWED_USERGROUPS: list[str] = [...]) like other handlers do. This avoids recreating the list per request and makes the authorization policy easier to locate/reuse.
| ) | ||
|
|
||
| id = int(id) | ||
| change = web.ctx.site.get_change(id) |
There was a problem hiding this comment.
change = web.ctx.site.get_change(id) can return None (as handled in GET), and not all changesets are undoable (can_undo() may be false, e.g. an already-undone merge or an undo changeset). Calling change._undo() unconditionally can raise an exception or allow unintended repeat/invalid undos. Please add the same not-found handling as GET and gate _undo() behind change.can_undo() (returning an appropriate 4xx / permission-denied response when it’s not undoable).
| change = web.ctx.site.get_change(id) | |
| change = web.ctx.site.get_change(id) | |
| if not change: | |
| web.ctx.status = "404 Not Found" | |
| return render.notfound(web.ctx.path) | |
| if not change.can_undo(): | |
| return render_template( | |
| "permission_denied", | |
| web.ctx.path, | |
| "Permission denied to undo this change.", | |
| ) |
Closes #11894
Restores the
recentchanges_viewPOST handler, which was thought to be unused. Authentication is now required for this endpoint. Usage is restricted to admins and super-librarians.Technical
Testing
Screenshot
Stakeholders