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.

Sign up for GitHub

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

Introduce Role-Based Authentication for Repository Management #1060

Merged
merged 12 commits into from
Dec 24, 2024
+1,408 −1,282
Merged

Introduce Role-Based Authentication for Repository Management #1060

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
559ce4d
[WIP] Use the highest authority permission
minwoox Nov 14, 2024
0ad7b35
lint
minwoox Nov 14, 2024
e8249bc
WIP
minwoox Nov 14, 2024
1ffab05
Fix UI
minwoox Nov 14, 2024
35459a1
Add test
minwoox Nov 18, 2024
c06e107
WIP
minwoox Nov 21, 2024
8df1aa1
Add missing tests
minwoox Nov 21, 2024
f638fd4
Fix UI
minwoox Nov 21, 2024
85cca89
Fix comment
minwoox Nov 22, 2024
ef403a9
Merge branch 'main' into add_repo_admin
minwoox Dec 16, 2024
75da118
Address comments from @ikhoon
minwoox Dec 17, 2024
64a00ab
Address the comment from @jrhee17
minwoox Dec 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Address the comment from @jrhee17
@minwoox
minwoox committed Dec 23, 2024
commit 64a00ab3bfc0edc3f11c7893be3e90abaec717aa
19 changes: 11 additions & 8 deletions server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java
View file
Original file line number Diff line number Diff line change
@@ -903,28 +903,31 @@ private CompletableFuture<RepositoryRole> findRepositoryRole0(String projectName
@Nullable
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional) What do you think of adding a RepositoryRole.NONE? It doesn't seem intuitive that a method returning an enum is nullable.

Suggested change
@Nullable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One reason I avoided using RepositoryRole.NONE is that it shouldn't be used when assigning a role to a member or a token.
If you're not strongly opposed, let me return null. 😉

private static RepositoryRole repositoryRole(Roles roles, @Nullable RepositoryRole repositoryRole,
Copy link
Contributor

Choose a reason for hiding this comment

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

optional) the logic in this class was not easy to follow at first glance. To me, the following flow makes more sense:

    @Nullable
    private static RepositoryRole repositoryRole(Roles roles, @Nullable RepositoryRole repositoryRole,
                                                 ProjectRole projectRole) {
        final RepositoryRole projectRepositoryRole;
        if (projectRole == ProjectRole.MEMBER) {
            projectRepositoryRole = roles.projectRoles().member();
        } else {
            projectRepositoryRole = roles.projectRoles().guest();
        }
        if (repositoryRole == RepositoryRole.ADMIN || projectRepositoryRole == RepositoryRole.ADMIN) {
            return RepositoryRole.ADMIN;
        }
        if (repositoryRole == RepositoryRole.WRITE || projectRepositoryRole == RepositoryRole.WRITE) {
            return RepositoryRole.ADMIN;
        }
        if (repositoryRole == RepositoryRole.READ || projectRepositoryRole == RepositoryRole.READ) {
            return RepositoryRole.READ;
        }
        return null;
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion. Applied. 👍

ProjectRole projectRole) {
if (repositoryRole == RepositoryRole.ADMIN || projectRole == ProjectRole.OWNER) {
if (projectRole == ProjectRole.OWNER) {
return RepositoryRole.ADMIN;
}

final RepositoryRole memberOrGuestRole;
if (projectRole == ProjectRole.MEMBER) {
memberOrGuestRole = roles.projectRoles().member();
} else {
assert projectRole == ProjectRole.GUEST;
memberOrGuestRole = roles.projectRoles().guest();
}

if (repositoryRole == null) {
return memberOrGuestRole;
}
if (memberOrGuestRole == null) {
return repositoryRole;
if (repositoryRole == RepositoryRole.ADMIN || memberOrGuestRole == RepositoryRole.ADMIN) {
return RepositoryRole.ADMIN;
}
if (memberOrGuestRole == RepositoryRole.WRITE || repositoryRole == RepositoryRole.WRITE) {

if (repositoryRole == RepositoryRole.WRITE || memberOrGuestRole == RepositoryRole.WRITE) {
return RepositoryRole.WRITE;
}

return RepositoryRole.READ;
if (repositoryRole == RepositoryRole.READ || memberOrGuestRole == RepositoryRole.READ) {
return RepositoryRole.READ;
}

return null;
}

/**