diff --git a/pygitweb/auth.schema.json b/pygitweb/auth.schema.json index 7f99ef5..cf91728 100644 --- a/pygitweb/auth.schema.json +++ b/pygitweb/auth.schema.json @@ -60,7 +60,7 @@ "additionalProperties": { "type": "array", "items": { "type": "string" }, - "description": "Scoped keys: pgw.read.{scope}, pgw.write.{scope}, pgw.write_branch.{project/branch}, pgw.boards.{scope}, pgw.tasks.{scope}, pgw.comments.{scope}, pgw.hooks.{scope}, pgw.mr.create.{scope}, pgw.mr.merge.{scope} (fnmatch). Principal * matches any authenticated user." + "description": "Scoped keys: pgw.read.{scope}, pgw.write.{scope}, pgw.write_branch.{project/branch} (refs/heads only), pgw.write_ref.{project/path-after-refs}, pgw.boards.{scope}, pgw.tasks.{scope}, pgw.comments.{scope}, pgw.hooks.{scope}, pgw.mr.create.{scope}, pgw.mr.merge.{scope} (fnmatch). Principal * matches any authenticated user." } }, "local_users": { diff --git a/pygitweb/src/pygitweb/auth.py b/pygitweb/src/pygitweb/auth.py index 2a021d6..c44a050 100644 --- a/pygitweb/src/pygitweb/auth.py +++ b/pygitweb/src/pygitweb/auth.py @@ -45,7 +45,8 @@ from pygitweb.password_hash import hash_password, verify_password from pygitweb.permissions import ( Permission, PermissionPrincipal, - branch_permission_subject, + ref_permission_subject, + ref_write_grant, ) from pygitweb.projects import git_get_project_owner from pygitweb.sessions import create_session, get_session, revoke_session @@ -189,20 +190,21 @@ def can_write_project(project: str, token: str | None) -> bool: def can_push_ref(project: str, principal: PermissionPrincipal, ref_name: str) -> bool: - """Branch push check. Project write is a separate gate. + """Per-ref push check. Project write is the fallback. - Non-branch refs are allowed. When any ``pgw.write_branch`` grant matches the - branch, only those principals may update it. Otherwise project write (including - ownership) is enough. + ``pgw.write_branch`` matches ``refs/heads`` only. ``pgw.write_ref`` matches any + ref under ``refs/``. A name outside ``refs/`` is rejected. When no specific + grant matches, project write (including ownership) is enough. """ if not settings.AUTH: return True - subject = branch_permission_subject(project, ref_name) - if subject is None: - return True + if ref_permission_subject(project, ref_name) is None: + return False permissions = auth_config.oauth_permissions - if permissions.matching_scoped_principals(Permission.WRITE_BRANCH, subject): - return permissions.has_permission(principal.identity, Permission.WRITE_BRANCH, subject) + grant = ref_write_grant(permissions, project, ref_name) + if grant is not None: + permission, subject = grant + return permissions.has_permission(principal.identity, permission, subject) return principal_can_write_project(project, principal) diff --git a/pygitweb/src/pygitweb/permissions.py b/pygitweb/src/pygitweb/permissions.py index 36dd6a1..1694984 100644 --- a/pygitweb/src/pygitweb/permissions.py +++ b/pygitweb/src/pygitweb/permissions.py @@ -7,7 +7,9 @@ every permission. Scoped keys use a project segment that supports ``fnmatch`` globs (``*``, ``?``, ``[seq]``), e.g. ``pgw.tasks.foo/*``, ``pgw.read.*``, or ``pgw.write.myproj``. -``pgw.write_branch`` scopes are ``project/branch`` (for example ``demo/main``). +``pgw.write_branch`` scopes are ``project/branch`` and apply only to ``refs/heads``. +``pgw.write_ref`` scopes are ``project`` plus the path after ``refs/`` +(for example ``demo/heads/main``, ``demo/tags/v1.*``, ``demo/notes/*``). Principal ``*`` in a grant list matches any authenticated principal. """ @@ -29,9 +31,10 @@ PERMISSION_ADD_PROJECTS = "pgw.addprojects" PERMISSION_SETTINGS = "pgw.settings" _SCOPED_PREFIX = re.compile( - r"^pgw\.(boards|tasks|comments|hooks|mr\.create|mr\.merge|read|write|write_branch)\.(.+)$", + r"^pgw\.(boards|tasks|comments|hooks|mr\.create|mr\.merge|read|write_branch|write_ref|write)\.(.+)$", ) +_REFS_PREFIX = "refs/" _BRANCH_REF_PREFIX = "refs/heads/" @@ -41,6 +44,7 @@ class Permission(StrEnum): READ = "read" WRITE = "write" WRITE_BRANCH = "write_branch" + WRITE_REF = "write_ref" BOARDS = "boards" TASKS = "tasks" COMMENTS = "comments" @@ -64,6 +68,7 @@ class Permission(StrEnum): Permission.READ, Permission.WRITE, Permission.WRITE_BRANCH, + Permission.WRITE_REF, Permission.BOARDS, Permission.TASKS, Permission.COMMENTS, @@ -136,18 +141,45 @@ def branch_permission_subject(project: str, ref_name: str) -> str | None: return f"{project}/{short_name}" -def has_branch_write_permission( +def ref_permission_subject(project: str, ref_name: str) -> str | None: + if not ref_name.startswith(_REFS_PREFIX): + return None + rest = ref_name.removeprefix(_REFS_PREFIX) + if not rest: + return None + return f"{project}/{rest}" + + +def ref_write_grant( + permissions: PermissionsMap, + project: str, + ref_name: str, +) -> tuple[Permission, str] | None: + """Specific ``write_branch`` or ``write_ref`` rule for this ref, if one matches. + + ``pgw.write_branch`` is checked first and only for ``refs/heads/*``. + """ + legacy = branch_permission_subject(project, ref_name) + if legacy is not None and permissions.matching_scoped_principals(Permission.WRITE_BRANCH, legacy): + return Permission.WRITE_BRANCH, legacy + subject = ref_permission_subject(project, ref_name) + if subject is not None and permissions.matching_scoped_principals(Permission.WRITE_REF, subject): + return Permission.WRITE_REF, subject + return None + + +def has_ref_write_permission( permissions: PermissionsMap, principal: str | None, project: str, ref_name: str, ) -> bool: - subject = branch_permission_subject(project, ref_name) - if subject is None: - return True - branch_specific = permissions.matching_scoped_principals(Permission.WRITE_BRANCH, subject) - if branch_specific: - return permissions.has_permission(principal, Permission.WRITE_BRANCH, subject) + if ref_permission_subject(project, ref_name) is None: + return False + grant = ref_write_grant(permissions, project, ref_name) + if grant is not None: + permission, subject = grant + return permissions.has_permission(principal, permission, subject) return permissions.has_permission(principal, Permission.WRITE, project) diff --git a/pygitweb/src/pygitweb/permissions_test.py b/pygitweb/src/pygitweb/permissions_test.py index 74b0f5b..54c95d8 100644 --- a/pygitweb/src/pygitweb/permissions_test.py +++ b/pygitweb/src/pygitweb/permissions_test.py @@ -6,10 +6,11 @@ from pygitweb.permissions import ( Permission, PermissionsMap, branch_permission_subject, - has_branch_write_permission, + has_ref_write_permission, parse_permission_key, permission_key, project_matches_scope, + ref_permission_subject, short_branch_name_from_ref, ) @@ -62,6 +63,10 @@ def test_parse_branch_scoped_keys() -> None: assert write is not None assert write.permission is Permission.WRITE_BRANCH assert write.project_scope == "dev/alice.*" + ref = parse_permission_key("pgw.write_ref.demo/tags/v*") + assert ref is not None + assert ref.permission is Permission.WRITE_REF + assert ref.project_scope == "demo/tags/v*" assert parse_permission_key("pgw.read_branch.team/app-main") is None @@ -77,6 +82,10 @@ def test_branch_permission_subject() -> None: assert short_branch_name_from_ref("refs/tags/v1.0.0") is None assert branch_permission_subject("demo", "refs/heads/main") == "demo/main" assert branch_permission_subject("demo", "refs/tags/v1.0.0") is None + assert ref_permission_subject("demo", "refs/heads/main") == "demo/heads/main" + assert ref_permission_subject("demo", "refs/tags/v1.0.0") == "demo/tags/v1.0.0" + assert ref_permission_subject("demo", "refs/notes/commits") == "demo/notes/commits" + assert ref_permission_subject("demo", "HEAD") is None def test_superuser_grants_all() -> None: @@ -159,20 +168,51 @@ def test_owner_grants_read_and_write() -> None: assert not can_read_project("secret", None) -def test_has_branch_write_permission_falls_back_to_project_scope() -> None: +def test_has_ref_write_permission_falls_back_to_project_scope() -> None: m = PermissionsMap.model_validate({ "*": [], "pgw.write.demo": ["writer@example.com"], }) - assert has_branch_write_permission(m, "writer@example.com", "demo", "refs/heads/main") - assert not has_branch_write_permission(m, "other@example.com", "demo", "refs/heads/main") + assert has_ref_write_permission(m, "writer@example.com", "demo", "refs/heads/main") + assert has_ref_write_permission(m, "writer@example.com", "demo", "refs/tags/v1") + assert has_ref_write_permission(m, "writer@example.com", "demo", "refs/notes/commits") + assert not has_ref_write_permission(m, "other@example.com", "demo", "refs/heads/main") + assert not has_ref_write_permission(m, "writer@example.com", "demo", "HEAD") -def test_has_branch_write_permission_enforces_branch_specific_rules() -> None: +def test_has_ref_write_permission_enforces_branch_specific_rules() -> None: m = PermissionsMap.model_validate({ "*": [], "pgw.write.demo": ["writer@example.com"], "pgw.write_branch.demo/main": ["branch-writer@example.com"], }) - assert has_branch_write_permission(m, "branch-writer@example.com", "demo", "refs/heads/main") - assert not has_branch_write_permission(m, "writer@example.com", "demo", "refs/heads/main") + assert has_ref_write_permission(m, "branch-writer@example.com", "demo", "refs/heads/main") + assert not has_ref_write_permission(m, "writer@example.com", "demo", "refs/heads/main") + assert has_ref_write_permission(m, "writer@example.com", "demo", "refs/tags/v1") + + +def test_has_ref_write_permission_enforces_ref_rules() -> None: + m = PermissionsMap.model_validate({ + "*": [], + "pgw.write.demo": ["writer@example.com"], + "pgw.write_ref.demo/tags/v*": ["release@example.com"], + "pgw.write_ref.demo/notes/*": ["alice@example.com"], + "pgw.write_ref.demo/heads/main": ["ref-writer@example.com"], + }) + assert has_ref_write_permission(m, "release@example.com", "demo", "refs/tags/v1.2") + assert not has_ref_write_permission(m, "writer@example.com", "demo", "refs/tags/v1.2") + assert has_ref_write_permission(m, "writer@example.com", "demo", "refs/tags/nightly") + assert has_ref_write_permission(m, "alice@example.com", "demo", "refs/notes/commits") + assert not has_ref_write_permission(m, "writer@example.com", "demo", "refs/notes/commits") + assert has_ref_write_permission(m, "ref-writer@example.com", "demo", "refs/heads/main") + assert not has_ref_write_permission(m, "writer@example.com", "demo", "refs/heads/main") + + +def test_write_branch_overrides_write_ref_for_heads() -> None: + m = PermissionsMap.model_validate({ + "*": [], + "pgw.write_branch.demo/main": ["branch-writer@example.com"], + "pgw.write_ref.demo/heads/main": ["ref-writer@example.com"], + }) + assert has_ref_write_permission(m, "branch-writer@example.com", "demo", "refs/heads/main") + assert not has_ref_write_permission(m, "ref-writer@example.com", "demo", "refs/heads/main") diff --git a/pygitweb/src/pygitweb/smart_http_test.py b/pygitweb/src/pygitweb/smart_http_test.py index 80a0ff8..4d1c341 100644 --- a/pygitweb/src/pygitweb/smart_http_test.py +++ b/pygitweb/src/pygitweb/smart_http_test.py @@ -486,6 +486,40 @@ class TestSmartHttpRoutes: ) assert oauth_push.status_code == 200 + def test_ref_push_permission_on_receive_pack(self, http_repo_env: dict[str, str]) -> None: + restricted = PermissionsMap.model_validate({ + "*": [], + "pgw.read.*": ["*"], + "pgw.write.*": ["*"], + "pgw.write_ref.demo/tags/v*": ["release@example.com"], + "pgw.write_ref.demo/notes/*": ["release@example.com"], + }) + client = TestClient(app) + with ( + _auth_enabled(_user_with_keys().model_copy(update={"oauth_permissions": restricted})), + patch("pygitweb.smart_http._run_git_service", return_value=b"") as run, + ): + tag = client.post( + f"/http/{http_repo_env['project']}/git-receive-pack", + content=_push_commands("refs/tags/v1"), + auth=("alice", _WRITE_KEY), + ) + notes = client.post( + f"/http/{http_repo_env['project']}/git-receive-pack", + content=_push_commands("refs/notes/commits"), + auth=("alice", _WRITE_KEY), + ) + other_tag = client.post( + f"/http/{http_repo_env['project']}/git-receive-pack", + content=_push_commands("refs/tags/nightly"), + auth=("alice", _WRITE_KEY), + ) + assert tag.status_code == 403 + assert "refs/tags/v1" in tag.json()["detail"] + assert notes.status_code == 403 + assert other_tag.status_code == 200 + assert run.call_count == 1 + def test_clone_and_push_over_http(self, http_repo_env: dict[str, str]) -> None: with _http_server() as port: clone_dir = Path(http_repo_env["root"]) / "clone"