Skip to content

Normalize permission model#1207

Open
jhoward-lm wants to merge 36 commits intoDependencyTrack:mainfrom
jhoward-lm:normalize-permission-model
Open

Normalize permission model#1207
jhoward-lm wants to merge 36 commits intoDependencyTrack:mainfrom
jhoward-lm:normalize-permission-model

Conversation

@jhoward-lm
Copy link
Copy Markdown
Contributor

@jhoward-lm jhoward-lm commented May 16, 2025

Description

This PR is an implementation of the permission model normalization discussed in DependencyTrack/hyades#1757 and proposed by DependencyTrack/hyades#1783.

Addressed Issue

Closes DependencyTrack/hyades#1757

Additional Details

Checklist

  • I have read and understand the contributing guidelines
  • This PR fixes a defect, and I have provided tests to verify that the fix is effective
  • This PR implements an enhancement, and I have provided tests to verify that it works as intended
  • This PR introduces changes to the database model, and I have updated the migration changelog accordingly
  • This PR introduces new or alters existing behavior, and I have updated the documentation accordingly

Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
@jhoward-lm jhoward-lm force-pushed the normalize-permission-model branch from cf8ac6f to 23057e5 Compare June 20, 2025 21:35
…erver into normalize-permission-model

Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
@jhoward-lm jhoward-lm marked this pull request as ready for review July 2, 2025 15:03
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
@codacy-production
Copy link
Copy Markdown

codacy-production bot commented Jul 7, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.00% (target: -1.00%) 76.00% (target: 70.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (39a308d) 27850 22860 82.08%
Head commit (22923ba) 27848 (-2) 22857 (-3) 82.08% (+0.00%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#1207) 125 95 76.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

@@ -26,96 +26,78 @@
*/
public enum Permissions {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note so we don't forget:

  • Before this can be merged, we need a DB migration that reassigns any existing old permissions to their corresponding new counterparts.
  • Changes in frontend would also be required.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@nscuro I added a change set for the migration. Not sure if it's what you were looking for, so any suggestions are welcome. Will work on the frontend tomorrow

Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
…to role

Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
@jhoward-lm
Copy link
Copy Markdown
Contributor Author

jhoward-lm commented Jul 11, 2025

@nscuro I haven't pushed this, but initially thought about adding database-level enforcement using:

  • Permissions defined as PostgreSQL enums, one for project-scoped and one for system-scoped
  • A domain representing a combination of these two enums to be used as the PERMISSION table NAME column datatype
  • A view of permission names and scopes for ease of lookup
    • or could be one view for project-scoped and another for system-scoped, e.g. PROJECT_PERMISSIONS and SYSTEM_PERMISSIONS (optionally with a V_ prefix, I've seen that convention from time to time)

Either the addition of the SCOPE column on the PERMISSION table or the view might be unnecessary, but here is the change set including both. WDYT?

changeSet
    <changeSet id="v5.6.0-32.3" author="jhoward-lm">
        <sql splitStatements="true">
            CREATE TYPE permission_scope AS ENUM (
              'PROJECT',
              'SYSTEM'
            );

            CREATE TYPE project_permission AS ENUM (
              'BADGES_READ',
              'BOM_READ',
              'BOM_CREATE',
              'FINDING_READ',
              'FINDING_UPDATE',
              'FINDING_CREATE',
              'POLICY_VIOLATION_READ',
              'POLICY_VIOLATION_UPDATE',
              'POLICY_VIOLATION_CREATE',
              'PROJECT_READ',
              'PROJECT_UPDATE',
              'PROJECT_DELETE'
            );

            CREATE TYPE system_permission AS ENUM (
              'ACCESS_MANAGEMENT',
              'NOTIFICATION_RULE',
              'POLICY',
              'PORTFOLIO_ACCESS_CONTROL_BYPASS',
              'PORTFOLIO',
              'SYSTEM_CONFIGURATION',
              'TAG',
              'VULNERABILITY'
            );

            CREATE CAST (VARCHAR AS project_permission) WITH INOUT AS IMPLICIT;
            CREATE CAST (VARCHAR AS system_permission) WITH INOUT AS IMPLICIT;

            CREATE DOMAIN permission AS TEXT NOT NULL
            CHECK (
              VALUE = ANY(CAST(enum_range(NULL::project_permission) AS TEXT[])) OR
              VALUE = ANY(CAST(enum_range(NULL::system_permission) AS TEXT[]))
            );
        </sql>

        <addColumn tableName="PERMISSION">
            <column name="SCOPE" type="permission_scope" />
        </addColumn>

        <sql splitStatements="false">
            UPDATE "PERMISSION" SET "SCOPE" = CASE
              WHEN "NAME" = ANY(CAST(enum_range(NULL::project_permission) AS TEXT[])) THEN 'PROJECT'::permission_scope
              WHEN "NAME" = ANY(CAST(enum_range(NULL::system_permission) AS TEXT[])) THEN 'SYSTEM'::permission_scope
              ELSE "SCOPE"
            END;
        </sql>

        <addNotNullConstraint tableName="PERMISSION" columnName="SCOPE" validate="true" />

        <createView viewName="PERMISSIONS_SCOPED" replaceIfExists="true">
            SELECT 'PROJECT'::permission_scope AS "TYPE", enum_val::TEXT AS "NAME"
              FROM unnest(enum_range(NULL::project_permission)) AS enum_val
             UNION
            SELECT 'SYSTEM'::permission_scope AS "TYPE", enum_val::TEXT AS "NAME"
              FROM unnest(enum_range(NULL::system_permission)) AS enum_val
        </createView>

        <modifyDataType tableName="PERMISSION" columnName="NAME" newDataType="permission" />
    </changeSet>

If a new permission needs to be added or renamed in the future, though, the contributor would need to know to add a change set containing something like

<sql>
    ALTER TYPE project_permission
    ADD VALUE 'METRICS_READ'
    AFTER 'FINDING_CREATE';

    ALTER TYPE system_permission
    RENAME VALUE 'PORTFOLIO'
    TO 'PORTFOLIO_MANAGEMENT';
</sql>

jhoward-lm and others added 11 commits July 11, 2025 18:27
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: jhoward-lm <140011346+jhoward-lm@users.noreply.github.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
…erver into normalize-permission-model

Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
test:
  - enable authorization for tests in NotificationRuleResourceTest
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Copy link
Copy Markdown
Member

@nscuro nscuro left a comment

Choose a reason for hiding this comment

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

Some initial feedback. I still need to have a close look at all the changed permission checks and verify they behave as expected.

I am also thinking to cut a release, even if it's just a release candidate, before merging this since it's rather intrusive.

jhoward-lm and others added 5 commits July 24, 2025 13:01
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: nscuro <nscuro@protonmail.com>
@nscuro
Copy link
Copy Markdown
Member

nscuro commented Jul 25, 2025

@jhoward-lm I raised jhoward-lm#16 to resolve the merge conflicts.

@nscuro nscuro added the enhancement New feature or request label Jul 25, 2025
@jhoward-lm
Copy link
Copy Markdown
Contributor Author

@jhoward-lm I raised jhoward-lm#16 to resolve the merge conflicts.

@nscuro Thanks! I merged it

Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Signed-off-by: Jonathan Howard <jonathan.w.howard@lmco.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Normalize permission model

2 participants