You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tooling.apache.org by "asf-tooling (via GitHub)" <gi...@apache.org> on 2026/05/08 21:58:30 UTC
Re: [I] Add release manager storage permissions (tooling-trusted-releases)
asf-tooling commented on issue #610:
URL: https://github.com/apache/tooling-trusted-releases/issues/610#issuecomment-4410151208
<!-- gofannon-issue-triage-bot v2 -->
**Automated triage** — analyzed at `main@2da7807a`
**Type:** `new_feature` • **Classification:** `actionable` • **Confidence:** `medium`
**Application domain(s):** `authentication_authorization`, `web_api_infrastructure`
### Summary
This issue requests a new 'CommitteeReleaseManager' permission that combines CommitteeMember with designated release managers (who may be committers, not PMC members). @dave2wave clarified two variants: (1) designated individuals, (2) designated committees where all members/committers get RM permissions. The last comment by @dave2wave (2026-03-01) suggests this may be handled via #656 and #801, introducing uncertainty about whether this issue is the right locus of work. No release manager concept currently exists in the codebase.
### Where this lives in the code today
#### `atr/user.py` — `is_committee_member` (lines 97-100)
_extension point_
Current committee membership check — a parallel is_release_manager function would be added nearby.
```python
def is_committee_member(committee: sql.Committee | None, uid: str) -> bool:
if committee is None:
return False
return any((member_uid == uid) for member_uid in committee.committee_members)
```
#### `atr/user.py` — `is_committer` (lines 103-106)
_extension point_
Current committer check — release manager permission would combine committee membership with designated committers.
```python
def is_committer(committee: sql.Committee | None, uid: str) -> bool:
if committee is None:
return False
return any((committer_uid == uid) for committer_uid in committee.committers)
```
#### `atr/web.py` — `Committer.prevent_confusing_ui_display` (lines 88-97)
_extension point_
@dave2wave noted check_access_release_manager and check_access_release_manager_get methods need to be added to web.py, following this pattern of access control.
```python
async def prevent_confusing_ui_display(self, project_key: str | safe.ProjectKey) -> None:
if not any((str(p.key) == str(project_key)) for p in (await self.user_projects)):
if self.is_admin:
# Admins can view all projects
# But we must warn them when the project is not one of their own
# TODO: This code is difficult to test locally
# TODO: This flash sometimes displays after deleting a project, which is a bug
await quart.flash("This is not your project, but you have access as an admin", "warning")
return
raise base.ASFQuartException("You do not have access to this project", errorcode=403)
```
#### `atr/web.py` — `Committer.prevent_confusing_ui_display_committee` (lines 99-110)
_extension point_
Existing committee access check pattern — new release manager checks would follow a similar structure.
```python
async def prevent_confusing_ui_display_committee(self, committee_key: safe.CommitteeKey, die: bool = True) -> bool:
if str(committee_key) not in self.committees:
if self.is_admin:
# Admins can view all committees
# But we must warn them when the committee is not one of their own
# TODO: As above, this code is difficult to test locally
await quart.flash("This is not your committee, but you have access as an admin", "warning")
return True
if die:
raise base.ASFQuartException("You do not have access to this committee", errorcode=403)
return False
return True
```
#### `atr/user.py` — `projects` (lines 109-132)
_needs modification_
Project access logic would need to account for designated release managers who aren't in committee_members or committers lists.
```python
async def projects(uid: str, committee_only: bool = False, super_project: bool = False) -> list[sql.Project]:
user_projects: list[sql.Project] = []
async with db.session() as data:
# Must have releases, because this is used in candidate_drafts
projects = await data.project(
status=sql.ProjectStatus.ACTIVE, _committee=True, _releases=True, _super_project=super_project
).all()
for p in projects:
if p.committee is None:
continue
# Allow access to test project in Test mode
# This means that the Test project will show in the user interface for everyone
if config.is_test_mode() and (p.committee.key == "test"):
user_projects.append(p)
continue
if committee_only:
if uid in p.committee.committee_members:
user_projects.append(p)
else:
if (uid in p.committee.committee_members) or (uid in p.committee.committers):
user_projects.append(p)
return user_projects
```
### Where new code would go
- `atr/models/sql.py` — new table or field on Committee/Project
Need a data model to store designated release managers per committee — either a new relationship table or a JSON field on Committee.
- `atr/user.py` — after is_committer function
New is_release_manager() function that checks if a user is a committee member OR a designated release manager.
- `atr/web.py` — after prevent_confusing_ui_display_committee method
@dave2wave explicitly stated check_access_release_manager and check_access_release_manager_get methods need to be added here.
- `atr/storage/__init__.py` — alongside existing permission checks
The storage layer centralizes authorization; a new release manager permission check would be added here per the architecture.
### Proposed approach
The implementation requires: (1) A data model change in `atr/models/sql.py` to store designated release managers per committee — likely a new `CommitteeReleaseManager` table linking committee keys to ASF UIDs of designated RMs who aren't already PMC members. (2) A new `is_release_manager(committee, uid)` function in `atr/user.py` that returns True if the user is a committee member OR is in the designated release managers list. (3) New `check_access_release_manager` and `check_access_release_manager_get` methods on the `Committer` class in `atr/web.py` that enforce the permission, raising 403 for unauthorized users (with admin override). (4) Applying these checks to the relevant release operation endpoints.
However, @dave2wave's last comment references issues #656 and #801 as possibly subsuming this work. The exact relationship is unclear without seeing those issues. The diffs below represent a starting point for the permission infrastructure, but the actual implementation should be coordinated with whatever approach is chosen in #656.
### Suggested patches
#### `atr/user.py`
Add is_release_manager function combining committee membership with designated RM status
````diff
--- a/atr/user.py
+++ b/atr/user.py
@@ -90,6 +90,25 @@ def is_committer(committee: sql.Committee | None, uid: str) -> bool:
return any((committer_uid == uid) for committer_uid in committee.committers)
+async def is_release_manager(committee: sql.Committee | None, uid: str) -> bool:
+ """Check if the user is a release manager for the given committee.
+
+ A release manager is either:
+ - A committee member (PMC member), OR
+ - A designated release manager (committer explicitly granted RM permissions)
+ """
+ if committee is None:
+ return False
+ if is_committee_member(committee, uid):
+ return True
+ # Check designated release managers from the database
+ async with db.session() as data:
+ # TODO: Implement query for designated release managers
+ # This requires the CommitteeReleaseManager model from sql.py
+ designated = await data.committee_release_managers(committee_key=committee.key, uid=uid)
+ return designated is not None
+
+
async def projects(uid: str, committee_only: bool = False, super_project: bool = False) -> list[sql.Project]:
user_projects: list[sql.Project] = []
async with db.session() as data:
````
#### `atr/web.py`
Add check_access_release_manager and check_access_release_manager_get methods as specified by @dave2wave
````diff
--- a/atr/web.py
+++ b/atr/web.py
@@ -78,6 +78,36 @@ class Committer:
return False
return True
+ async def check_access_release_manager(self, committee: sql.Committee) -> None:
+ """Check that the current user has release manager permissions for the committee.
+
+ Raises 403 if the user is not a committee member, not a designated release manager,
+ and not an admin.
+ """
+ if await user.is_release_manager(committee, self.asf_uid):
+ return
+ if self.is_admin:
+ await quart.flash("You have access as an admin, but are not a release manager", "warning")
+ return
+ raise base.ASFQuartException(
+ "You do not have release manager permissions for this committee", errorcode=403
+ )
+
+ async def check_access_release_manager_get(self, committee: sql.Committee) -> bool:
+ """Check release manager access for GET routes (non-fatal).
+
+ Returns True if the user has release manager access, False otherwise.
+ Does not raise an exception — intended for views that degrade gracefully.
+ """
+ if await user.is_release_manager(committee, self.asf_uid):
+ return True
+ if self.is_admin:
+ await quart.flash("You have access as an admin, but are not a release manager", "warning")
+ return True
+ return False
+
async def form_data(self) -> dict[str, Any]:
if self.__form_data is None:
self.__form_data = await form.quart_request()
````
### Open questions
- What is the status of issues #656 and #801? @dave2wave's last comment suggests this work may be handled there instead.
- What data model should store designated release managers? A new SQL table (CommitteeReleaseManager) or a JSON field on Committee?
- Should the 'designated committees where all PMC members and committers are allowed permissions' (variant 2 from @dave2wave) be a boolean flag on Committee, or a separate configuration?
- Which specific endpoints should have the release manager permission applied? Issue references #242 skeleton but we don't have access to that.
- How does the db.session() query for designated release managers work? The data access pattern (data.committee_release_managers) needs to be implemented in atr/db/.
### Files examined
- `atr/blueprints/common.py`
- `atr/user.py`
- `atr/blueprints/api.py`
- `atr/web.py`
- `atr/blueprints/get.py`
- `atr/blueprints/post.py`
- `atr/ldap.py`
- `atr/models/api.py`
---
*Draft from a triage agent. A human reviewer should validate before merging any change. The agent did not run tests or verify diffs apply.*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tooling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tooling.apache.org
For additional commands, e-mail: dev-help@tooling.apache.org