Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for authz lookup #1196

Merged
merged 32 commits into from
Jan 9, 2025

Conversation

JoshuaSBrown
Copy link
Collaborator

@JoshuaSBrown JoshuaSBrown commented Dec 30, 2024

PR Description

Tasks

  • - A description of the PR has been provided, and a diagram included if it is a new feature.
  • - Formatter has been run
  • - CHANGELOG comment has been added
  • - Labels have been assigned to the pr
  • - A reviwer has been added
  • - A user has been assigned to work on the pr
  • - If new feature a unit test has been added

Summary by Sourcery

Implement authorization checks for data records and projects.

New Features:

  • Added support for authorization lookups based on user, project, and record ownership.
  • Introduced a new authorization strategy for GridFTP actions, including read, write, create, delete, change directory, and lookup.
  • Implemented a new Repo class to manage repository paths and determine path types (user, project, record, etc.).

Tests:

  • Added unit tests for the new authorization functions and the Repo class.

Copy link

sourcery-ai bot commented Dec 30, 2024

Reviewer's Guide by Sourcery

This pull request implements authorization checks for GridFTP operations based on user, project, and record permissions. It introduces a new authorization lookup mechanism and adds corresponding unit tests.

Sequence diagram for GridFTP authorization flow

sequenceDiagram
    actor Client
    participant Router as AuthZ Router
    participant Repo as Repo Class
    participant AuthZ as AuthZ Module

    Client->>Router: GET /gridftp
    Note over Router: Validate request params
    Router->>Router: Get client info
    Router->>Repo: new Repo(repo)
    Router->>Repo: pathType(file)
    Repo-->>Router: PathType result
    alt PathType is UNKNOWN
        Router-->>Client: Permission Denied
    else Valid PathType
        Router->>AuthZ: Check authz_strategy
        AuthZ->>AuthZ: Execute strategy function
        alt Authorization Successful
            AuthZ-->>Router: Success
            Router-->>Client: Authorized
        else Authorization Failed
            AuthZ-->>Router: Throw Permission Denied
            Router-->>Client: Permission Denied
        end
    end
Loading

Class diagram for the new Repo class and PathType enum

classDiagram
    class Repo {
        -error: number
        -err_msg: string
        -exists: boolean
        -repo_id: string
        -repo_key: string
        +constructor(a_key: string)
        +exists(): boolean
        +key(): string
        +id(): string
        +error(): number
        +errorMessage(): string
        +pathType(a_path: string): PathType
    }
    class PathType {
        <<enumeration>>
        USER_PATH
        USER_RECORD_PATH
        PROJECT_PATH
        PROJECT_RECORD_PATH
        REPO_BASE_PATH
        REPO_ROOT_PATH
        REPO_PATH
        UNKNOWN
    }
    note for Repo "Handles repository path validation
and type classification"
Loading

File-Level Changes

Change Details Files
Implemented authorization checks based on user, project, and record permissions.
  • Added functions to check record-level permissions.
  • Integrated authorization checks into GridFTP routes.
  • Added tests for various authorization scenarios, including admin access, owner access, project admin access, and access denial for unauthorized users.
  • Implemented a new authz strategy to handle different path types and actions based on permissions.
  • Added support for public read access to records.
  • Implemented path consistency checks to ensure valid record access.
  • Added functions to handle create and read record permissions based on the provided path and client permissions.
core/database/foxx/api/authz.js
core/database/foxx/api/authz_router.js
core/database/foxx/tests/authz.test.js
Added unit tests for the new authorization functionality.
  • Added tests to cover different access scenarios.
  • Updated CMakeLists.txt to include the new unit tests in the test suite.
core/database/foxx/tests/authz.test.js
core/database/CMakeLists.txt
Introduced the Repo class and PathType enum for path handling.
  • Created the Repo class to manage repository information and path types.
  • Defined the PathType enum to categorize different path types within a repository.
  • Integrated the Repo class and PathType enum into the authorization module.
  • Added unit tests for the Repo class.
  • Updated CMakeLists.txt to include the new unit tests in the test suite.
  • Updated authz and authz_router modules to use the new Repo class and PathType enum for path handling and authorization logic.
core/database/foxx/api/repo.js
core/database/foxx/tests/repo.test.js
core/database/foxx/api/authz.js
core/database/foxx/api/authz_router.js
core/database/CMakeLists.txt

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @JoshuaSBrown - I've reviewed your changes - here's some feedback:

Overall Comments:

  • There are some duplicated comments in the code, e.g. 'Will split a posix path into an array' appears twice. Consider cleaning these up for better maintainability.
Here's what I looked at during the review
  • 🟡 General issues: 2 issues found
  • 🟢 Security: all looks good
  • 🟡 Testing: 1 issue found
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

core/database/foxx/api/authz.js Outdated Show resolved Hide resolved
core/database/foxx/api/authz_router.js Outdated Show resolved Hide resolved
core/database/foxx/tests/authz.test.js Outdated Show resolved Hide resolved
core/database/foxx/api/authz.js Outdated Show resolved Hide resolved
core/database/foxx/api/authz.js Outdated Show resolved Hide resolved
core/database/foxx/tests/repo.test.js Outdated Show resolved Hide resolved
@JoshuaSBrown JoshuaSBrown self-assigned this Dec 30, 2024
JoshuaSBrown and others added 7 commits December 30, 2024 16:12
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
@JoshuaSBrown
Copy link
Collaborator Author

@sourcery-ai review

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @JoshuaSBrown - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 2 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

core/database/foxx/api/authz.js Outdated Show resolved Hide resolved
core/database/foxx/api/authz_router.js Outdated Show resolved Hide resolved
@JoshuaSBrown JoshuaSBrown merged commit 7555914 into 1180-refactor-authz Jan 9, 2025
13 of 14 checks passed
@JoshuaSBrown JoshuaSBrown deleted the add-support-for-authz-lookup branch January 9, 2025 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant