Skip to content

fix(history): authorize file history through the file's parent directory - #42

Open
opendev9 wants to merge 1 commit into
devfrom
fix/file-history-share-20261021
Open

opendev9 wants to merge 1 commit into
devfrom
fix/file-history-share-20261021

Conversation

@opendev9

Copy link
Copy Markdown

Problem

FileHistoryView (/api/v2.1/repos/{id}/file/history/) and NewFileHistoryView
(/api/v2.1/repos/{id}/file/new_history/) both authorized the caller with:

check_folder_permission(request, repo_id, '/')

A share-only account has no library permission and is qualified on the shared
directory instead (the native grant seam added in #40). Asking about the library
root therefore answers None and returns 403 — for exactly the files the account
can already list, read and download. The file detail view (file.py:126) has
always asked about the parent directory; the history views were the outliers.

Fix

Ask about the file's parent directory, matching the file detail view:

parent_dir = posixpath.dirname(path) or '/'
if not check_folder_permission(request, repo_id, parent_dir):

Both views changed; posixpath was already imported.

Scope and risk

  • Library members: unaffected. The native check answers from the library
    grant and ignores the path, so '/' and '/a/b' produce the same verdict.
  • Share-only accounts: newly get history for files whose parent directory
    qualifies them — the same files they can already read. Nothing outside a
    qualified directory is authorized.
  • Renames: the old path from rev_renamed_old_path is still checked against
    its own parent, so a path that is no longer under a share stays refused.
  • The check still runs after the file-existence lookup, so the 404/403 ordering
    is unchanged from the previous revision.

Testing

python3 -m py_compile only — the Seahub test suite needs Django, which is not
installed on this machine. Suggested follow-up coverage in tests/api/test_files.py:

  • share-only account + file under the share root → 200 on both endpoints;
  • share-only account + file outside the share root → 403;
  • library member unchanged → 200/403 exactly as before.

Related

Follow-up to #40 (merged), which gave the native permission seam the shared-root
r/rw levels these views now rely on.

Both history views gate on check_folder_permission(repo_id, '/'), so a share-only
account -- no library permission, qualified on the shared directory -- is refused
the history of exactly the files it can read and download.

Ask about the parent directory instead, the convention the file detail view
already uses (file.py:126). Library members are unaffected: the native check
answers from the library grant and ignores the path. A rename's old parent path
can only affect a share-only account, which was refused before anyway.

Compile-checked only; the Hub test suite needs Django, which is not installed
on this machine.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant