Skip to content

Commit

Permalink
Introduce Role-Based Authentication for Repository Management (#1060)
Browse files Browse the repository at this point in the history
Motivation:
To address the migration of mirroring configurations from projects to repositories and to simplify access management,
we decided to:
- Hide the meta repository.
- Replace permission-based authentication with a role-based system using `RepositoryRole` (`READ`, `WRITE`, `ADMIN`). The `ADMIN` role has access to the configurations.

This change resolves the issue where users with `WRITE` permission for the meta repository were only allowed to creating mirroring configurations.

With `RepositoryRole`, access becomes more structured and extensible for future enhancements (e.g., introducing custom roles). While permission-based authentication is replaced here, this does not preclude the future coexistence of role-based and permission-based systems.

Modifications:
- Added `RepositoryRole` with hierarchical roles (`READ`, `WRITE`, `ADMIN`).
- Replaced `RequiresPermission` annotations with `RequiresRepositoryRole`.
- Renamed `RequiresRole` to `RequiresProjectRole`.
  - Updated it to accept a single `ProjectRole`, utilizing the hierarchical model to avoid redundant role specifications.
- Removed APIs for managing permissions, replacing them with role management APIs.

Result:
- Role-based access simplifies management and aligns with repository-specific mirroring configurations.
- (Breaking) APIs for managing permissions are removed.

To-do:
- Update the documentation to reflect the new systme, including updated screenshots

Migration plan:
- Deploy [PR](#1061), which supports deserialization of both legacy and new metadata format.
- Deploy intermediate commit supporting both permission and role APIs, ensuring metadata is stored in the new format.
  - The commit also migrate the legacy format to new format.
- Deploy this commit, which exclusively supports role-based APIs.
  • Loading branch information
minwoox authored Dec 24, 2024
1 parent 7d75b79 commit a813dcd
Show file tree
Hide file tree
Showing 58 changed files with 1,408 additions and 1,282 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -906,8 +906,8 @@ private <T> CompletableFuture<T> watch(Revision lastKnownRevision, long timeoutM
}

private static void validateProjectName(String projectName) {
// We don't know if the token has the permission to access internal projects.
// The server will reject the request if the token does not have the permission.
// We don't know if the token has the role to access internal projects.
// The server will reject the request if the token does not have the required role.
Util.validateProjectName(projectName, "projectName", true);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ void shouldAllowMembersToAccessInternalProjects() throws Exception {
.execute().status())
.isEqualTo(HttpStatus.FORBIDDEN);

// Grant the user permission to access the internal project.
// Grant the user role to access the internal project.
final AggregatedHttpResponse res =
adminWebClient.prepare()
.post("/api/v1/metadata/@xds/members")
Expand All @@ -154,7 +154,7 @@ void shouldAllowMembersToAccessInternalProjects() throws Exception {

// @xds project should be visible to member users.
assertThat(userClient.listProjects().join()).containsOnly("foo", "@xds");
// Read and write permission should be granted as well.
// Read and write should be granted as well.
userRepo.commit("Update test.txt", Change.ofTextUpsert("/text.txt", "bar"))
.push()
.join();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,8 @@
import com.linecorp.centraldogma.server.internal.api.TokenService;
import com.linecorp.centraldogma.server.internal.api.WatchService;
import com.linecorp.centraldogma.server.internal.api.auth.ApplicationTokenAuthorizer;
import com.linecorp.centraldogma.server.internal.api.auth.RequiresPermissionDecorator.RequiresReadPermissionDecoratorFactory;
import com.linecorp.centraldogma.server.internal.api.auth.RequiresPermissionDecorator.RequiresWritePermissionDecoratorFactory;
import com.linecorp.centraldogma.server.internal.api.auth.RequiresRoleDecorator.RequiresRoleDecoratorFactory;
import com.linecorp.centraldogma.server.internal.api.auth.RequiresProjectRoleDecorator.RequiresProjectRoleDecoratorFactory;
import com.linecorp.centraldogma.server.internal.api.auth.RequiresRepositoryRoleDecorator.RequiresRepositoryRoleDecoratorFactory;
import com.linecorp.centraldogma.server.internal.api.converter.HttpApiRequestConverter;
import com.linecorp.centraldogma.server.internal.mirror.DefaultMirroringServicePlugin;
import com.linecorp.centraldogma.server.internal.mirror.MirrorRunner;
Expand Down Expand Up @@ -835,9 +834,8 @@ private void configureHttpApi(ServerBuilder sb,
// See JacksonRequestConverterFunctionTest
new JacksonRequestConverterFunction(new ObjectMapper()),
new HttpApiRequestConverter(projectApiManager),
new RequiresReadPermissionDecoratorFactory(mds),
new RequiresWritePermissionDecoratorFactory(mds),
new RequiresRoleDecoratorFactory(mds)
new RequiresRepositoryRoleDecoratorFactory(mds),
new RequiresProjectRoleDecoratorFactory(mds)
);
sb.dependencyInjector(dependencyInjector, false)
// TODO(ikhoon): Consider exposing ReflectiveDependencyInjector as a public API via
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import com.linecorp.centraldogma.common.Markup;
import com.linecorp.centraldogma.common.Query;
import com.linecorp.centraldogma.common.QueryType;
import com.linecorp.centraldogma.common.RepositoryRole;
import com.linecorp.centraldogma.common.Revision;
import com.linecorp.centraldogma.internal.Jackson;
import com.linecorp.centraldogma.server.command.Command;
Expand All @@ -59,16 +60,15 @@
import com.linecorp.centraldogma.server.internal.admin.dto.RevisionDto;
import com.linecorp.centraldogma.server.internal.admin.util.RestfulJsonResponseConverter;
import com.linecorp.centraldogma.server.internal.api.AbstractService;
import com.linecorp.centraldogma.server.internal.api.auth.RequiresReadPermission;
import com.linecorp.centraldogma.server.internal.api.auth.RequiresWritePermission;
import com.linecorp.centraldogma.server.internal.api.auth.RequiresRepositoryRole;
import com.linecorp.centraldogma.server.internal.storage.project.ProjectApiManager;
import com.linecorp.centraldogma.server.metadata.User;
import com.linecorp.centraldogma.server.storage.repository.Repository;

/**
* Annotated service object for managing repositories.
*/
@RequiresReadPermission
@RequiresRepositoryRole(RepositoryRole.READ)
@ResponseConverter(RestfulJsonResponseConverter.class)
public class RepositoryService extends AbstractService {

Expand Down Expand Up @@ -122,7 +122,7 @@ public CompletionStage<EntryDto> getFile(@Param String projectName,
@Post
@Put
@Path("/projects/{projectName}/repositories/{repoName}/files/revisions/{revision}")
@RequiresWritePermission
@RequiresRepositoryRole(RepositoryRole.WRITE)
public CompletionStage<Object> addOrEditFile(@Param String projectName,
@Param String repoName,
@Param String revision,
Expand All @@ -146,7 +146,7 @@ public CompletionStage<Object> addOrEditFile(@Param String projectName,
*/
@Post("regex:/projects/(?<projectName>[^/]+)/repositories/(?<repoName>[^/]+)" +
"/delete/revisions/(?<revision>[^/]+)(?<path>/.*$)")
@RequiresWritePermission
@RequiresRepositoryRole(RepositoryRole.WRITE)
public HttpResponse deleteFile(@Param String projectName,
@Param String repoName,
@Param String revision,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import com.linecorp.centraldogma.common.Markup;
import com.linecorp.centraldogma.common.MergeQuery;
import com.linecorp.centraldogma.common.Query;
import com.linecorp.centraldogma.common.RepositoryRole;
import com.linecorp.centraldogma.common.Revision;
import com.linecorp.centraldogma.common.RevisionRange;
import com.linecorp.centraldogma.common.ShuttingDownException;
Expand All @@ -79,8 +80,7 @@
import com.linecorp.centraldogma.server.command.CommandExecutor;
import com.linecorp.centraldogma.server.command.CommitResult;
import com.linecorp.centraldogma.server.internal.admin.auth.AuthUtil;
import com.linecorp.centraldogma.server.internal.api.auth.RequiresReadPermission;
import com.linecorp.centraldogma.server.internal.api.auth.RequiresWritePermission;
import com.linecorp.centraldogma.server.internal.api.auth.RequiresRepositoryRole;
import com.linecorp.centraldogma.server.internal.api.converter.ChangesRequestConverter;
import com.linecorp.centraldogma.server.internal.api.converter.CommitMessageRequestConverter;
import com.linecorp.centraldogma.server.internal.api.converter.MergeQueryRequestConverter;
Expand All @@ -99,7 +99,7 @@
* Annotated service object for managing and watching contents.
*/
@ProducesJson
@RequiresReadPermission
@RequiresRepositoryRole(RepositoryRole.READ)
@RequestConverter(CommitMessageRequestConverter.class)
public class ContentServiceV1 extends AbstractService {

Expand Down Expand Up @@ -189,7 +189,7 @@ private static String normalizePath(String path) {
*/
@Post("/projects/{projectName}/repos/{repoName}/contents")
@ConsumesJson
@RequiresWritePermission
@RequiresRepositoryRole(RepositoryRole.WRITE)
public CompletableFuture<PushResultDto> push(
ServiceRequestContext ctx,
@Param @Default("-1") String revision,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@
import com.linecorp.centraldogma.common.Author;
import com.linecorp.centraldogma.common.Change;
import com.linecorp.centraldogma.common.Markup;
import com.linecorp.centraldogma.common.RepositoryRole;
import com.linecorp.centraldogma.common.Revision;
import com.linecorp.centraldogma.internal.api.v1.PushResultDto;
import com.linecorp.centraldogma.server.command.Command;
import com.linecorp.centraldogma.server.command.CommandExecutor;
import com.linecorp.centraldogma.server.command.CommitResult;
import com.linecorp.centraldogma.server.credential.Credential;
import com.linecorp.centraldogma.server.internal.api.auth.RequiresReadPermission;
import com.linecorp.centraldogma.server.internal.api.auth.RequiresWritePermission;
import com.linecorp.centraldogma.server.internal.api.auth.RequiresRepositoryRole;
import com.linecorp.centraldogma.server.internal.storage.project.ProjectApiManager;
import com.linecorp.centraldogma.server.metadata.User;
import com.linecorp.centraldogma.server.storage.project.Project;
Expand All @@ -65,7 +65,7 @@ public CredentialServiceV1(ProjectApiManager projectApiManager, CommandExecutor
*
* <p>Returns the list of the credentials in the project.
*/
@RequiresReadPermission(repository = Project.REPO_META)
@RequiresRepositoryRole(value = RepositoryRole.READ, repository = Project.REPO_META)
@Get("/projects/{projectName}/credentials")
public CompletableFuture<List<Credential>> listCredentials(User loginUser,
@Param String projectName) {
Expand All @@ -86,7 +86,7 @@ public CompletableFuture<List<Credential>> listCredentials(User loginUser,
*
* <p>Returns the credential for the ID in the project.
*/
@RequiresReadPermission(repository = Project.REPO_META)
@RequiresRepositoryRole(value = RepositoryRole.READ, repository = Project.REPO_META)
@Get("/projects/{projectName}/credentials/{id}")
public CompletableFuture<Credential> getCredentialById(User loginUser,
@Param String projectName, @Param String id) {
Expand All @@ -102,10 +102,10 @@ public CompletableFuture<Credential> getCredentialById(User loginUser,
*
* <p>Creates a new credential.
*/
@RequiresWritePermission(repository = Project.REPO_META)
@Post("/projects/{projectName}/credentials")
@ConsumesJson
@StatusCode(201)
@Post("/projects/{projectName}/credentials")
@RequiresRepositoryRole(value = RepositoryRole.WRITE, repository = Project.REPO_META)
public CompletableFuture<PushResultDto> createCredential(@Param String projectName,
Credential credential, Author author, User user) {
return createOrUpdate(projectName, credential, author, user, false);
Expand All @@ -116,9 +116,9 @@ public CompletableFuture<PushResultDto> createCredential(@Param String projectNa
*
* <p>Update the existing credential.
*/
@RequiresWritePermission(repository = Project.REPO_META)
@Put("/projects/{projectName}/credentials/{id}")
@ConsumesJson
@Put("/projects/{projectName}/credentials/{id}")
@RequiresRepositoryRole(value = RepositoryRole.WRITE, repository = Project.REPO_META)
public CompletableFuture<PushResultDto> updateCredential(@Param String projectName, @Param String id,
Credential credential, Author author, User user) {
checkArgument(id.equals(credential.id()), "The credential ID (%s) can't be updated", id);
Expand All @@ -130,8 +130,8 @@ public CompletableFuture<PushResultDto> updateCredential(@Param String projectNa
*
* <p>Delete the existing credential.
*/
@RequiresWritePermission(repository = Project.REPO_META)
@Delete("/projects/{projectName}/credentials/{id}")
@RequiresRepositoryRole(value = RepositoryRole.WRITE, repository = Project.REPO_META)
public CompletableFuture<Void> deleteCredential(@Param String projectName,
@Param String id, Author author, User user) {
final MetaRepository metaRepository = metaRepo(projectName, user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@
import com.linecorp.armeria.server.annotation.Param;
import com.linecorp.armeria.server.annotation.Post;
import com.linecorp.armeria.server.annotation.RequestConverter;
import com.linecorp.centraldogma.server.internal.api.auth.RequiresReadPermission;
import com.linecorp.centraldogma.common.RepositoryRole;
import com.linecorp.centraldogma.server.internal.api.auth.RequiresRepositoryRole;
import com.linecorp.centraldogma.server.internal.api.converter.HttpApiRequestConverter;
import com.linecorp.centraldogma.server.internal.storage.project.ProjectApiManager;
import com.linecorp.centraldogma.server.metadata.User;
Expand All @@ -60,7 +61,7 @@
* A service that provides Git HTTP protocol.
*/
@RequestConverter(HttpApiRequestConverter.class)
@RequiresReadPermission
@RequiresRepositoryRole(RepositoryRole.READ)
public final class GitHttpService {

private static final Logger logger = LoggerFactory.getLogger(GitHttpService.class);
Expand Down
Loading

0 comments on commit a813dcd

Please sign in to comment.