diff --git a/.gitignore b/.gitignore index 013b84705..033420cdd 100644 --- a/.gitignore +++ b/.gitignore @@ -143,3 +143,6 @@ typings/ # Codex AGENTS.md + +# Claude +.claude/settings.json diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/api/v1/CreateRepositoryRequest.java b/common/src/main/java/com/linecorp/centraldogma/internal/api/v1/CreateRepositoryRequest.java index efdc956ed..2cb74ca37 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/api/v1/CreateRepositoryRequest.java +++ b/common/src/main/java/com/linecorp/centraldogma/internal/api/v1/CreateRepositoryRequest.java @@ -32,12 +32,15 @@ public class CreateRepositoryRequest { private final String name; private final boolean encrypt; + private final boolean isPublic; @JsonCreator public CreateRepositoryRequest(@JsonProperty("name") String name, - @JsonProperty("encrypt") @Nullable Boolean encrypt) { + @JsonProperty("encrypt") @Nullable Boolean encrypt, + @JsonProperty("isPublic") @Nullable Boolean isPublic) { this.name = validateRepositoryName(name, "name"); this.encrypt = firstNonNull(encrypt, false); + this.isPublic = firstNonNull(isPublic, false); } @JsonProperty @@ -50,11 +53,17 @@ public boolean encrypt() { return encrypt; } + @JsonProperty("isPublic") + public boolean isPublic() { + return isPublic; + } + @Override public String toString() { return MoreObjects.toStringHelper(this) .add("name", name()) .add("encrypt", encrypt) + .add("isPublic", isPublic) .toString(); } } diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/MetadataApiService.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/MetadataApiService.java index d3715cc40..145e5534d 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/MetadataApiService.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/MetadataApiService.java @@ -36,6 +36,7 @@ import com.linecorp.armeria.server.annotation.Patch; import com.linecorp.armeria.server.annotation.Post; import com.linecorp.armeria.server.annotation.ProducesJson; +import com.linecorp.armeria.server.annotation.Put; import com.linecorp.centraldogma.common.Author; import com.linecorp.centraldogma.common.ProjectRole; import com.linecorp.centraldogma.common.RepositoryRole; @@ -139,7 +140,7 @@ public CompletableFuture updateRepositoryProjectRoles( JsonNode payload, Author author) throws JsonProcessingException { final JsonNode guest = payload.get("guest"); - if (guest.isTextual()) { + if (guest != null && guest.isTextual()) { // TODO(ikhoon): Move this validation to the constructor of ProjectRoles once GUEST WRITE role is // migrated to GUEST READ. final String role = guest.asText(); @@ -151,6 +152,30 @@ public CompletableFuture updateRepositoryProjectRoles( return mds.updateRepositoryProjectRoles(author, projectName, repoName, projectRoles); } + /** + * PUT /metadata/{projectName}/settings + * + *

Updates the settings of the specified {@code projectName}. A field which is not specified + * is left unchanged. The body of the request will be: + * 

{@code
+     * {
+     *   "allowPublicRepositories": false
+     * }
+     * }
+ */ + @RequiresProjectRole(ProjectRole.OWNER) + @Put("/metadata/{projectName}/settings") + public CompletableFuture updateProjectSettings( + @Param String projectName, + UpdateProjectSettingsRequest request, + Author author) { + final Boolean allowPublicRepositories = request.allowPublicRepositories(); + if (allowPublicRepositories == null) { + throw new IllegalArgumentException("No settings are specified."); + } + return mds.updateAllowPublicRepositories(author, projectName, allowPublicRepositories); + } + /** * POST /metadata/{projectName}/repos/{repoName}/roles/users * diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceUtil.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceUtil.java index da9555f7b..bfb072473 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceUtil.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceUtil.java @@ -30,6 +30,7 @@ import com.linecorp.centraldogma.server.command.Command; import com.linecorp.centraldogma.server.command.CommandExecutor; import com.linecorp.centraldogma.server.metadata.MetadataService; +import com.linecorp.centraldogma.server.metadata.ProjectRoles; import com.linecorp.centraldogma.server.metadata.RepositoryMetadata; import com.linecorp.centraldogma.server.metadata.Roles; import com.linecorp.centraldogma.server.metadata.UserAndTimestamp; @@ -43,6 +44,14 @@ public static CompletableFuture createRepository( CommandExecutor commandExecutor, MetadataService mds, Author author, String projectName, String repoName, boolean encrypt, @Nullable EncryptionStorageManager encryptionStorageManager) { + return createRepository(commandExecutor, mds, author, projectName, repoName, + DEFAULT_PROJECT_ROLES, encrypt, encryptionStorageManager); + } + + public static CompletableFuture createRepository( + CommandExecutor commandExecutor, MetadataService mds, + Author author, String projectName, String repoName, ProjectRoles projectRoles, boolean encrypt, + @Nullable EncryptionStorageManager encryptionStorageManager) { final Map users; final Map appIds; if (author.isAppIdentity()) { @@ -54,7 +63,7 @@ public static CompletableFuture createRepository( appIds = ImmutableMap.of(); } - final Roles roles = new Roles(DEFAULT_PROJECT_ROLES, users, null, appIds); + final Roles roles = new Roles(projectRoles, users, null, appIds); final RepositoryMetadata repositoryMetadata = RepositoryMetadata.of(repoName, roles, UserAndTimestamp.of(author)); diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java index 152b784c0..0da37c54c 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java @@ -21,6 +21,8 @@ import static com.linecorp.centraldogma.server.internal.api.DtoConverter.newRepositoryDto; import static com.linecorp.centraldogma.server.internal.api.HttpApiUtil.checkUnremoveArgument; import static com.linecorp.centraldogma.server.internal.api.HttpApiUtil.returnOrThrow; +import static com.linecorp.centraldogma.server.metadata.RepositoryMetadata.DEFAULT_PROJECT_ROLES; +import static com.linecorp.centraldogma.server.metadata.RepositoryMetadata.PUBLIC_PROJECT_ROLES; import static java.util.Objects.requireNonNull; import java.util.List; @@ -66,6 +68,7 @@ import com.linecorp.centraldogma.server.internal.api.converter.CreateApiResponseConverter; import com.linecorp.centraldogma.server.metadata.MetadataService; import com.linecorp.centraldogma.server.metadata.ProjectMetadata; +import com.linecorp.centraldogma.server.metadata.ProjectRoles; import com.linecorp.centraldogma.server.metadata.RepositoryMetadata; import com.linecorp.centraldogma.server.metadata.User; import com.linecorp.centraldogma.server.storage.encryption.EncryptionStorageManager; @@ -198,12 +201,25 @@ public CompletableFuture createRepository(ServiceRequestContext c "Encryption is not enabled in the server."); } + if (request.isPublic()) { + // Reject before the storage repository is created so a rejected public creation does not + // leave an orphaned repository in the common case. MetadataService.addRepo re-validates + // atomically. + final ProjectMetadata metadata = project.metadata(); + if (metadata == null || !metadata.allowPublicRepositories()) { + return HttpApiUtil.throwResponse(ctx, HttpStatus.BAD_REQUEST, + "Public repositories are not allowed in the project: %s", + project.name()); + } + } + final boolean encrypt = request.encrypt() || isEncryptedProject(project); + final ProjectRoles projectRoles = request.isPublic() ? PUBLIC_PROJECT_ROLES : DEFAULT_PROJECT_ROLES; final CommandExecutor commandExecutor = executor(); final CompletableFuture future = RepositoryServiceUtil.createRepository(commandExecutor, mds, author, project.name(), repoName, - encrypt, encryptionStorageManager); + projectRoles, encrypt, encryptionStorageManager); return future.handle(returnOrThrow(() -> { final Repository repository = project.repos().get(repoName); return newRepositoryDto(repository, repositoryStatus(repository)); diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/UpdateProjectSettingsRequest.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/UpdateProjectSettingsRequest.java new file mode 100644 index 000000000..c96d913d5 --- /dev/null +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/UpdateProjectSettingsRequest.java @@ -0,0 +1,72 @@ +/* + * Copyright 2026 LY Corporation + * + * LY Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package com.linecorp.centraldogma.server.internal.api; + +import java.util.Objects; + +import org.jspecify.annotations.Nullable; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.MoreObjects; + +/** + * A request to update the settings of a project. A {@code null} field is left unchanged. + */ +final class UpdateProjectSettingsRequest { + + @Nullable + private final Boolean allowPublicRepositories; + + @JsonCreator + UpdateProjectSettingsRequest( + @JsonProperty("allowPublicRepositories") @Nullable Boolean allowPublicRepositories) { + this.allowPublicRepositories = allowPublicRepositories; + } + + /** + * Returns whether the repositories of the project can be made public, or {@code null} if unchanged. + */ + @Nullable + @JsonProperty + public Boolean allowPublicRepositories() { + return allowPublicRepositories; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof UpdateProjectSettingsRequest)) { + return false; + } + final UpdateProjectSettingsRequest that = (UpdateProjectSettingsRequest) o; + return Objects.equals(allowPublicRepositories, that.allowPublicRepositories); + } + + @Override + public int hashCode() { + return Objects.hashCode(allowPublicRepositories); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("allowPublicRepositories", allowPublicRepositories()) + .toString(); + } +} diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/sysadmin/AppIdentityRegistryService.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/sysadmin/AppIdentityRegistryService.java index 0131e6dc9..5661339db 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/sysadmin/AppIdentityRegistryService.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/sysadmin/AppIdentityRegistryService.java @@ -118,6 +118,7 @@ public CompletableFuture> createAppIdentity( @Param AppIdentityType type, @Param @Nullable String secret, @Param @Nullable String certificateId, + @Param @Default("false") boolean allowGuestAccess, Author author, User loginUser) { if (!mtlsEnabled && type == AppIdentityType.CERTIFICATE) { throw new IllegalArgumentException( @@ -137,16 +138,12 @@ public CompletableFuture> createAppIdentity( if (type == AppIdentityType.TOKEN) { checkArgument(certificateId == null, "TOKEN type cannot have a certificateId: %s", certificateId); - if (secret != null) { - future = mds.createToken(author, appId, secret, isSystemAdmin); - } else { - future = mds.createToken(author, appId, isSystemAdmin); - } + future = mds.createToken(author, appId, secret, isSystemAdmin, allowGuestAccess); } else { checkArgument(certificateId != null, "CERTIFICATE type must have a certificateId."); checkArgument(secret == null, "CERTIFICATE type cannot have a secret: %s", secret); - future = mds.createCertificate(author, appId, certificateId, isSystemAdmin); + future = mds.createCertificate(author, appId, certificateId, isSystemAdmin, allowGuestAccess); } return future.thenCompose(unused -> fetchAppIdentity(appId)) .thenApply(appIdentity -> { @@ -329,17 +326,19 @@ public Collection listTokens(User loginUser) { *

Returns a newly-generated token belonging to the current login user. * * @deprecated Use {@link #createAppIdentity( - * String, boolean, AppIdentityType, String, String, Author, User)}. + * String, boolean, AppIdentityType, String, String, boolean, Author, User)}. */ @Post("/tokens") @StatusCode(201) @ResponseConverter(CreateApiResponseConverter.class) @Deprecated - public CompletableFuture> createToken(@Param String appId, - @Param @Default("false") boolean isSystemAdmin, - @Param @Nullable String secret, - Author author, User loginUser) { - return createAppIdentity(appId, isSystemAdmin, AppIdentityType.TOKEN, secret, null, + public CompletableFuture> createToken( + @Param String appId, + @Param @Default("false") boolean isSystemAdmin, + @Param @Nullable String secret, + @Param @Default("false") boolean allowGuestAccess, + Author author, User loginUser) { + return createAppIdentity(appId, isSystemAdmin, AppIdentityType.TOKEN, secret, null, allowGuestAccess, author, loginUser) .thenApply(responseEntity -> { final AppIdentity app = responseEntity.content(); diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/project/DefaultProject.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/project/DefaultProject.java index 2d2019645..eb148ead8 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/project/DefaultProject.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/project/DefaultProject.java @@ -237,6 +237,7 @@ private void initializeMetadata(long creationTimeMillis, Author author) { members, null, appIds, + null, userAndTimestamp, null); final CommitResult result = dogmaRepo.commit(headRev, creationTimeMillis, Author.SYSTEM, diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/AppIdentityService.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/AppIdentityService.java index db742735c..ff59bc0d7 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/metadata/AppIdentityService.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/AppIdentityService.java @@ -33,6 +33,8 @@ import java.util.UUID; import java.util.concurrent.CompletableFuture; +import org.jspecify.annotations.Nullable; + import com.fasterxml.jackson.core.JsonPointer; import com.fasterxml.jackson.databind.JsonNode; import com.google.common.collect.ImmutableMap; @@ -69,26 +71,17 @@ AppIdentityRegistry getAppIdentityRegistry() { return projectInitializer.appIdentityRegistry(); } - CompletableFuture createToken(Author author, String appId) { - return createToken(author, appId, false); - } - - CompletableFuture createToken(Author author, String appId, boolean isSystemAdmin) { - return createToken(author, appId, SECRET_PREFIX + UUID.randomUUID(), isSystemAdmin); - } - - CompletableFuture createToken(Author author, String appId, String secret) { - return createToken(author, appId, secret, false); - } - - CompletableFuture createToken(Author author, String appId, String secret, - boolean isSystemAdmin) { + CompletableFuture createToken(Author author, String appId, @Nullable String secret, + boolean isSystemAdmin, boolean allowGuestAccess) { requireNonNull(author, "author"); requireNonNull(appId, "appId"); - requireNonNull(secret, "secret"); + if (secret == null) { + secret = SECRET_PREFIX + UUID.randomUUID(); + } validateSecret(secret); - final Token newToken = new Token(appId, secret, isSystemAdmin, isSystemAdmin, + // A system admin app identity can access any repository, so guest access is implied. + final Token newToken = new Token(appId, secret, isSystemAdmin, isSystemAdmin || allowGuestAccess, UserAndTimestamp.of(author)); final AppIdentityRegistryTransformer transformer = new AppIdentityRegistryTransformer( (headRevision, tokens) -> { @@ -280,14 +273,15 @@ private static void throwIfInvalidType(String appId, AppIdentity appIdentity, } CompletableFuture createCertificate(Author author, String appId, String certificateId, - boolean isSystemAdmin) { + boolean isSystemAdmin, boolean allowGuestAccess) { requireNonNull(author, "author"); requireNonNull(appId, "appId"); checkArgument(!isNullOrEmpty(certificateId), "certificateId must not be null or empty"); - // Does not allow guest access for non admin certificate. + // A system admin app identity can access any repository, so guest access is implied. final CertificateAppIdentity certificate = - new CertificateAppIdentity(appId, certificateId, isSystemAdmin, isSystemAdmin, + new CertificateAppIdentity(appId, certificateId, isSystemAdmin, + isSystemAdmin || allowGuestAccess, UserAndTimestamp.of(author)); final JsonPointer appIdPath = JsonPointer.compile("/appIds" + encodeSegment(certificate.appId())); final JsonPointer certificateIdPath = diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java index e1a108be2..b0f2d905b 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java @@ -16,12 +16,14 @@ package com.linecorp.centraldogma.server.metadata; +import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.linecorp.centraldogma.server.internal.storage.project.ProjectApiManager.listProjectsWithoutInternal; import static com.linecorp.centraldogma.server.metadata.RepositoryMetadata.DEFAULT_PROJECT_ROLES; import static java.util.Objects.requireNonNull; import java.util.Collection; +import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Set; @@ -186,6 +188,7 @@ public CompletableFuture removeProject(Author author, String projectNa projectMetadata.members(), null, projectMetadata.appIds(), + projectMetadata.allowPublicRepositories(), projectMetadata.creation(), UserAndTimestamp.of(author)); }); @@ -210,6 +213,7 @@ public CompletableFuture restoreProject(Author author, String projectN projectMetadata.members(), null, projectMetadata.appIds(), + projectMetadata.allowPublicRepositories(), projectMetadata.creation(), null); }); @@ -259,6 +263,7 @@ public CompletableFuture addMember(Author author, String projectName, newMembers, null, projectMetadata.appIds(), + projectMetadata.allowPublicRepositories(), projectMetadata.creation(), projectMetadata.removal()); }); @@ -290,6 +295,7 @@ public CompletableFuture removeMember(Author author, String projectNam newMembers, null, projectMetadata.appIds(), + projectMetadata.allowPublicRepositories(), projectMetadata.creation(), projectMetadata.removal()); }); @@ -355,6 +361,7 @@ public CompletableFuture updateMemberRole(Author author, String projec newMembers, null, projectMetadata.appIds(), + projectMetadata.allowPublicRepositories(), projectMetadata.creation(), projectMetadata.removal()); }); @@ -412,6 +419,11 @@ public CompletableFuture addRepo(Author author, String projectName, if (projectMetadata.repos().containsKey(repoName)) { throw RepositoryExistsException.of(projectName, repoName); } + if (repositoryMetadata.roles().projectRoles().guest() != null && + !projectMetadata.allowPublicRepositories()) { + // Raced with disallowing public repositories. + throw new ChangeConflictException(publicRepositoriesNotAllowed(projectName)); + } final ImmutableMap newRepos = ImmutableMap.builderWithExpectedSize( projectMetadata.repos().size() + 1) @@ -423,10 +435,20 @@ public CompletableFuture addRepo(Author author, String projectName, projectMetadata.members(), null, projectMetadata.appIds(), + projectMetadata.allowPublicRepositories(), projectMetadata.creation(), projectMetadata.removal()); }); - return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, transformer); + if (repositoryMetadata.roles().projectRoles().guest() == null) { + return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, transformer); + } + return fetchMetadata(projectName).thenCompose(projectMetadata -> { + // Give a specific 400 for the common case; the transformer above atomically re-validates. + if (!projectMetadata.allowPublicRepositories()) { + throw new IllegalArgumentException(publicRepositoriesNotAllowed(projectName)); + } + return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, transformer); + }); } /** @@ -481,6 +503,7 @@ public CompletableFuture purgeRepo(Author author, String projectName, projectMetadata.members(), null, projectMetadata.appIds(), + projectMetadata.allowPublicRepositories(), projectMetadata.creation(), projectMetadata.removal()); }); @@ -531,17 +554,111 @@ public CompletableFuture updateRepositoryProjectRoles(Author author, final String commitSummary = "Update the project roles of the '" + repoName + "' in the project '" + projectName + '\''; - final RepositoryMetadataTransformer transformer = new RepositoryMetadataTransformer( - repoName, (headRevision, repositoryMetadata) -> { - final Roles newRoles = new Roles(projectRoles, repositoryMetadata.roles().users(), null, - repositoryMetadata.roles().appIds()); - return new RepositoryMetadata(repositoryMetadata.name(), - newRoles, - repositoryMetadata.creation(), - repositoryMetadata.removal(), - repositoryMetadata.status()); + final ProjectMetadataTransformer transformer = new ProjectMetadataTransformer( + (headRevision, projectMetadata) -> { + final RepositoryMetadata repositoryMetadata = projectMetadata.repo(repoName); + // Only a private-to-public transition requires the project to allow public + // repositories; an already-public repository can still update its member role. + if (projectRoles.guest() != null && + repositoryMetadata.roles().projectRoles().guest() == null && + !projectMetadata.allowPublicRepositories()) { + // Raced with disallowing public repositories. + throw new ChangeConflictException(publicRepositoriesNotAllowed(projectName)); + } + final Roles newRoles = new Roles(projectRoles, repositoryMetadata.roles().users(), null, + repositoryMetadata.roles().appIds()); + final RepositoryMetadata newRepositoryMetadata = + new RepositoryMetadata(repositoryMetadata.name(), + newRoles, + repositoryMetadata.creation(), + repositoryMetadata.removal(), + repositoryMetadata.status()); + return RepositoryMetadataTransformer.newProjectMetadata(projectMetadata, + newRepositoryMetadata); + }); + if (projectRoles.guest() == null) { + return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, transformer); + } + return fetchMetadata(projectName).thenCompose(projectMetadata -> { + // Raise RepositoryNotFoundException for a missing repository. + final RepositoryMetadata repositoryMetadata = projectMetadata.repo(repoName); + // Give a specific 400 for the common case; the transformer above atomically re-validates. + if (repositoryMetadata.roles().projectRoles().guest() == null && + !projectMetadata.allowPublicRepositories()) { + throw new IllegalArgumentException(publicRepositoriesNotAllowed(projectName)); + } + return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, transformer); }); - return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, transformer); + } + + /** + * Updates whether the repositories of the specified {@code projectName} can be made public. + * Disallowing is rejected while the project still has a public repository. + */ + public CompletableFuture updateAllowPublicRepositories(Author author, String projectName, + boolean allowPublicRepositories) { + requireNonNull(author, "author"); + requireNonNull(projectName, "projectName"); + + final String commitSummary = + (allowPublicRepositories ? "Allow" : "Disallow") + + " public repositories in the project '" + projectName + '\''; + final ProjectMetadataTransformer transformer = new ProjectMetadataTransformer( + (headRevision, projectMetadata) -> { + if (projectMetadata.allowPublicRepositories() == allowPublicRepositories) { + // Swallowed by the push layer, which returns the current head revision. + throw new RedundantChangeException( + headRevision, + "allowPublicRepositories is already " + allowPublicRepositories + + " in the project: " + projectName); + } + if (!allowPublicRepositories) { + final List publicRepos = publicRepositories(projectMetadata); + if (!publicRepos.isEmpty()) { + // Raced with making a repository public. + throw new ChangeConflictException( + cannotDisallowPublicRepositories(projectName, publicRepos)); + } + } + return new ProjectMetadata(projectMetadata.name(), + projectMetadata.repos(), + projectMetadata.members(), + null, + projectMetadata.appIds(), + allowPublicRepositories, + projectMetadata.creation(), + projectMetadata.removal()); + }); + if (allowPublicRepositories) { + return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, transformer); + } + return fetchMetadata(projectName).thenCompose(projectMetadata -> { + // Give a specific 400 for the common case; the transformer above atomically re-validates. + final List publicRepos = publicRepositories(projectMetadata); + if (!publicRepos.isEmpty()) { + throw new IllegalArgumentException( + cannotDisallowPublicRepositories(projectName, publicRepos)); + } + return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, transformer); + }); + } + + private static List publicRepositories(ProjectMetadata metadata) { + // Removed repositories count as well; otherwise restoring one could resurrect a guest role + // into a project which disallows public repositories. + return metadata.repos().values().stream() + .filter(repo -> repo.roles().projectRoles().guest() != null) + .map(repo -> repo.removal() != null ? repo.name() + " (removed)" : repo.name()) + .collect(toImmutableList()); + } + + private static String publicRepositoriesNotAllowed(String projectName) { + return "Public repositories are not allowed in the project: " + projectName; + } + + private static String cannotDisallowPublicRepositories(String projectName, List publicRepos) { + return "Cannot disallow public repositories in the project '" + projectName + + "'. Make the following repositories private first: " + publicRepos; } /** @@ -575,6 +692,7 @@ public CompletableFuture addAppIdentity(Author author, String projectN projectMetadata.members(), null, newAppIds, + projectMetadata.allowPublicRepositories(), projectMetadata.creation(), projectMetadata.removal()); }); @@ -623,6 +741,7 @@ CompletableFuture removeAppIdentityFromProject(Author author, String p projectMetadata.members(), null, newAppIds, + projectMetadata.allowPublicRepositories(), projectMetadata.creation(), projectMetadata.removal()); }); @@ -687,6 +806,7 @@ public CompletableFuture updateAppIdentityRole( projectMetadata.members(), null, newAppIds, + projectMetadata.allowPublicRepositories(), projectMetadata.creation(), projectMetadata.removal()); }); @@ -978,41 +1098,6 @@ public static RepositoryRole findRepositoryRole(ProjectMetadata metadata, String return repositoryRole(roles, repositoryRole, projectRole); } - /** - * Finds {@link RepositoryRole} of the specified {@link AppIdentity} from the specified - * {@code repoName} in the specified {@code projectName}. If the {@code appIdentity} is not found, - * it will return {@code null}. - */ - public CompletableFuture findRepositoryRole(String projectName, String repoName, - AppIdentity appIdentity) { - requireNonNull(projectName, "projectName"); - requireNonNull(repoName, "repoName"); - requireNonNull(appIdentity, "appIdentity"); - - return getProject(projectName).thenApply(metadata -> { - final RepositoryMetadata repositoryMetadata = metadata.repo(repoName); - final Roles roles = repositoryMetadata.roles(); - final String appId = appIdentity.appId(); - final RepositoryRole repositoryRole = roles.appIds().get(appId); - - final AppIdentityRegistration projectAppIdentityRegistration = metadata.appIds().get(appId); - final ProjectRole projectRole; - if (projectAppIdentityRegistration != null) { - projectRole = projectAppIdentityRegistration.role(); - } else { - // System admin app identities were checked before this method. - assert !appIdentity.isSystemAdmin(); - if (repositoryRole != null || appIdentity.allowGuestAccess()) { - projectRole = ProjectRole.GUEST; - } else { - // The app identity is not allowed with the GUEST permission. - return null; - } - } - return repositoryRole(roles, repositoryRole, projectRole); - }); - } - @Nullable private static RepositoryRole repositoryRole(Roles roles, @Nullable RepositoryRole repositoryRole, ProjectRole projectRole) { @@ -1085,7 +1170,7 @@ public AppIdentityRegistry getAppIdentityRegistry() { * will be automatically generated. */ public CompletableFuture createToken(Author author, String appId) { - return appIdentityService.createToken(author, appId); + return createToken(author, appId, null, false, false); } /** @@ -1093,14 +1178,14 @@ public CompletableFuture createToken(Author author, String appId) { * secret. */ public CompletableFuture createToken(Author author, String appId, boolean isSystemAdmin) { - return appIdentityService.createToken(author, appId, isSystemAdmin); + return createToken(author, appId, null, isSystemAdmin, false); } /** * Creates a new user-level {@link Token} with the specified {@code appId} and {@code secret}. */ public CompletableFuture createToken(Author author, String appId, String secret) { - return appIdentityService.createToken(author, appId, secret); + return createToken(author, appId, requireNonNull(secret, "secret"), false, false); } /** @@ -1108,7 +1193,17 @@ public CompletableFuture createToken(Author author, String appId, Stri */ public CompletableFuture createToken(Author author, String appId, String secret, boolean isSystemAdmin) { - return appIdentityService.createToken(author, appId, secret, isSystemAdmin); + return createToken(author, appId, requireNonNull(secret, "secret"), isSystemAdmin, false); + } + + /** + * Creates a new {@link Token} with the specified {@code appId}, {@code secret}, {@code isSystemAdmin} + * and {@code allowGuestAccess}. If {@code secret} is {@code null}, it will be automatically generated. + * A system admin {@link Token} always allows guest access. + */ + public CompletableFuture createToken(Author author, String appId, @Nullable String secret, + boolean isSystemAdmin, boolean allowGuestAccess) { + return appIdentityService.createToken(author, appId, secret, isSystemAdmin, allowGuestAccess); } /** @@ -1249,6 +1344,7 @@ public CompletableFuture updateRepositoryStatus( projectMetadata.members(), null, projectMetadata.appIds(), + projectMetadata.allowPublicRepositories(), projectMetadata.creation(), projectMetadata.removal()); }); @@ -1281,11 +1377,22 @@ private static void throwIfRedundant(RepositoryStatus repositoryStatus, Revision /** * Creates a new app identity {@link CertificateAppIdentity} with the specified {@code appId} and - * {@code certificateId}. + * {@code certificateId}. A system admin {@link CertificateAppIdentity} always allows guest access. */ public CompletableFuture createCertificate(Author author, String appId, String certificateId, boolean isSystemAdmin) { - return appIdentityService.createCertificate(author, appId, certificateId, isSystemAdmin); + return createCertificate(author, appId, certificateId, isSystemAdmin, false); + } + + /** + * Creates a new app identity {@link CertificateAppIdentity} with the specified {@code appId}, + * {@code certificateId}, {@code isSystemAdmin} and {@code allowGuestAccess}. + * A system admin {@link CertificateAppIdentity} always allows guest access. + */ + public CompletableFuture createCertificate(Author author, String appId, String certificateId, + boolean isSystemAdmin, boolean allowGuestAccess) { + return appIdentityService.createCertificate(author, appId, certificateId, isSystemAdmin, + allowGuestAccess); } /** diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/ProjectMetadata.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/ProjectMetadata.java index 289f0d129..d9e51fbe4 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/metadata/ProjectMetadata.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/ProjectMetadata.java @@ -16,6 +16,7 @@ package com.linecorp.centraldogma.server.metadata; +import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.collect.ImmutableMap.toImmutableMap; import static java.util.Objects.requireNonNull; @@ -50,6 +51,7 @@ public class ProjectMetadata implements Identifiable, HasWeight { ImmutableMap.of(), null, ImmutableMap.of(), + false, new UserAndTimestamp(User.SYSTEM.id()), null); @@ -73,6 +75,11 @@ public class ProjectMetadata implements Identifiable, HasWeight { */ private final Map appIds; + /** + * Whether the repositories in this project can be made public. + */ + private final boolean allowPublicRepositories; + /** * Specifies when this project is created by whom. */ @@ -93,6 +100,8 @@ public ProjectMetadata(@JsonProperty("name") String name, @JsonProperty("members") Map members, @JsonProperty("tokens") @Nullable Map tokens, @JsonProperty("appIds") @Nullable Map appIds, + @JsonProperty("allowPublicRepositories") + @Nullable Boolean allowPublicRepositories, @JsonProperty("creation") UserAndTimestamp creation, @JsonProperty("removal") @Nullable UserAndTimestamp removal) { this.name = requireNonNull(name, "name"); @@ -108,6 +117,7 @@ public ProjectMetadata(@JsonProperty("name") String name, this.appIds = ImmutableMap.copyOf(tokens); } + this.allowPublicRepositories = firstNonNull(allowPublicRepositories, true); this.creation = requireNonNull(creation, "creation"); this.removal = removal; } @@ -149,6 +159,14 @@ public Map appIds() { return appIds; } + /** + * Returns whether the repositories in this project can be made public. + */ + @JsonProperty + public boolean allowPublicRepositories() { + return allowPublicRepositories; + } + /** * Returns who created this project when. */ @@ -244,22 +262,25 @@ public boolean equals(Object o) { repos.equals(that.repos) && members.equals(that.members) && appIds.equals(that.appIds) && + allowPublicRepositories == that.allowPublicRepositories && creation.equals(that.creation) && Objects.equals(removal, that.removal); } @Override public int hashCode() { - return Objects.hash(name, repos, members, appIds, creation, removal); + return Objects.hash(name, repos, members, appIds, allowPublicRepositories, creation, removal); } @Override public String toString() { return MoreObjects.toStringHelper(this) + .omitNullValues() .add("name", name()) .add("repos", repos()) .add("members", members()) .add("appIds", appIds()) + .add("allowPublicRepositories", allowPublicRepositories()) .add("creation", creation()) .add("removal", removal()) .toString(); @@ -280,6 +301,7 @@ public ProjectMetadata withoutDogmaRepo() { members(), null, appIds(), + allowPublicRepositories(), creation(), removal()); } diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadata.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadata.java index 64ae50847..19140bfc7 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadata.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadata.java @@ -46,6 +46,12 @@ public final class RepositoryMetadata implements Identifiable, HasWeight { public static final ProjectRoles DEFAULT_PROJECT_ROLES = ProjectRoles.of(RepositoryRole.WRITE, null); + /** + * The {@link ProjectRoles} of a public repository whose guests have the {@link RepositoryRole#READ} role. + */ + public static final ProjectRoles PUBLIC_PROJECT_ROLES = + ProjectRoles.of(RepositoryRole.WRITE, RepositoryRole.READ); + /** * Creates a new instance with default properties. */ diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadataTransformer.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadataTransformer.java index 5a007ffc6..a6f5b278c 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadataTransformer.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadataTransformer.java @@ -36,8 +36,8 @@ final class RepositoryMetadataTransformer extends ProjectMetadataTransformer { }); } - private static ProjectMetadata newProjectMetadata(ProjectMetadata projectMetadata, - RepositoryMetadata repositoryMetadata) { + static ProjectMetadata newProjectMetadata(ProjectMetadata projectMetadata, + RepositoryMetadata repositoryMetadata) { final ImmutableMap.Builder builder = ImmutableMap.builderWithExpectedSize(projectMetadata.repos().size()); for (Entry entry : projectMetadata.repos().entrySet()) { @@ -53,6 +53,7 @@ private static ProjectMetadata newProjectMetadata(ProjectMetadata projectMetadat projectMetadata.members(), null, projectMetadata.appIds(), + projectMetadata.allowPublicRepositories(), projectMetadata.creation(), projectMetadata.removal()); } diff --git a/server/src/test/java/com/linecorp/centraldogma/server/internal/admin/model/SerializationTest.java b/server/src/test/java/com/linecorp/centraldogma/server/internal/admin/model/SerializationTest.java index 2266816d5..aa2bb5e7e 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/internal/admin/model/SerializationTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/internal/admin/model/SerializationTest.java @@ -81,6 +81,7 @@ void testValidProject() throws IOException { new AppIdentityRegistration(token.id(), ProjectRole.MEMBER, newCreationTag())), + null, newCreationTag(), null); assertThatJson(metadata) @@ -136,6 +137,7 @@ void testValidProject() throws IOException { " }\n" + " }\n" + " },\n" + + " \"allowPublicRepositories\" : true,\n" + " \"creation\" : {\n" + " \"user\" : \"editor@dogma.org\",\n" + " \"timestamp\" : \"2017-01-01T00:00:00Z\"\n" + @@ -183,6 +185,7 @@ void testValidProject() throws IOException { " }\n" + " }\n" + " },\n" + + " \"allowPublicRepositories\" : true,\n" + " \"creation\" : {\n" + " \"user\" : \"editor@dogma.org\",\n" + " \"timestamp\" : \"2017-01-01T00:00:00Z\"\n" + @@ -218,6 +221,7 @@ void testRemovedProject() throws IOException { new AppIdentityRegistration(token.id(), ProjectRole.MEMBER, newCreationTag())), + null, newCreationTag(), newRemovalTag()); @@ -261,6 +265,7 @@ void testRemovedProject() throws IOException { " }\n" + " }\n" + " },\n" + + " \"allowPublicRepositories\" : true,\n" + " \"creation\" : {\n" + " \"user\" : \"editor@dogma.org\",\n" + " \"timestamp\" : \"2017-01-01T00:00:00Z\"\n" + diff --git a/server/src/test/java/com/linecorp/centraldogma/server/internal/api/AppIdentityRegistryServiceTest.java b/server/src/test/java/com/linecorp/centraldogma/server/internal/api/AppIdentityRegistryServiceTest.java index 9f582c209..1b21a1759 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/internal/api/AppIdentityRegistryServiceTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/internal/api/AppIdentityRegistryServiceTest.java @@ -109,12 +109,12 @@ public void tearDown() { @Test void systemAdminToken() { - final Token token = appIdentityRegistryService.createToken("forAdmin1", true, null, + final Token token = appIdentityRegistryService.createToken("forAdmin1", true, null, false, systemAdminAuthor, systemAdmin).join() .content(); assertThat(token.isActive()).isTrue(); assertThatThrownBy( - () -> appIdentityRegistryService.createToken("forAdmin2", true, null, guestAuthor, guest) + () -> appIdentityRegistryService.createToken("forAdmin2", true, null, false, guestAuthor, guest) .join()) .isInstanceOf(IllegalArgumentException.class); @@ -149,7 +149,7 @@ void systemAdminToken() { void systemAdminAppIdentity() { final CertificateAppIdentity certificate = (CertificateAppIdentity) appIdentityRegistryService.createAppIdentity( - "certAdmin1", true, AppIdentityType.CERTIFICATE, null, "cert/123", + "certAdmin1", true, AppIdentityType.CERTIFICATE, null, "cert/123", false, systemAdminAuthor, systemAdmin).join().content(); assertThat(certificate.isActive()).isTrue(); assertThat(certificate.certificateId()).isEqualTo("cert/123"); @@ -157,11 +157,11 @@ void systemAdminAppIdentity() { () -> appIdentityRegistryService.createAppIdentity( "certAdmin2", true, AppIdentityType.CERTIFICATE, null, - "cert-456", guestAuthor, guest).join()) + "cert-456", false, guestAuthor, guest).join()) .isInstanceOf(IllegalArgumentException.class); final Token token = (Token) appIdentityRegistryService.createAppIdentity( - "tokenAdmin1", true, AppIdentityType.TOKEN, null, null, + "tokenAdmin1", true, AppIdentityType.TOKEN, null, null, false, systemAdminAuthor, systemAdmin).join().content(); assertThat(token.isActive()).isTrue(); assertThat(token.secret()).isNotNull(); @@ -214,12 +214,12 @@ void systemAdminAppIdentity() { @Test void userToken() { - final Token userToken1 = appIdentityRegistryService.createToken("forUser1", false, null, + final Token userToken1 = appIdentityRegistryService.createToken("forUser1", false, null, false, systemAdminAuthor, systemAdmin) .join().content(); - final Token userToken2 = appIdentityRegistryService.createToken("forUser2", false, null, guestAuthor, - guest) + final Token userToken2 = appIdentityRegistryService.createToken("forUser2", false, null, false, + guestAuthor, guest) .join().content(); assertThat(userToken1.isActive()).isTrue(); assertThat(userToken2.isActive()).isTrue(); @@ -250,15 +250,31 @@ void userToken() { }); } + @Test + void certificateGuestAccess() { + final CertificateAppIdentity guestCert = + (CertificateAppIdentity) appIdentityRegistryService.createAppIdentity( + "certGuest", false, AppIdentityType.CERTIFICATE, null, "cert-guest", true, + guestAuthor, guest).join().content(); + assertThat(guestCert.allowGuestAccess()).isTrue(); + + // A system admin identity always allows guest access. + final CertificateAppIdentity adminCert = + (CertificateAppIdentity) appIdentityRegistryService.createAppIdentity( + "certAdminGuest", true, AppIdentityType.CERTIFICATE, null, "cert-admin-guest", + false, systemAdminAuthor, systemAdmin).join().content(); + assertThat(adminCert.allowGuestAccess()).isTrue(); + } + @Test void userCertificate() { final CertificateAppIdentity userCert1 = (CertificateAppIdentity) appIdentityRegistryService.createAppIdentity( - "certUser1", false, AppIdentityType.CERTIFICATE, null, "cert-user1", + "certUser1", false, AppIdentityType.CERTIFICATE, null, "cert-user1", false, systemAdminAuthor, systemAdmin).join().content(); final CertificateAppIdentity userCert2 = (CertificateAppIdentity) appIdentityRegistryService.createAppIdentity( - "certUser2", false, AppIdentityType.CERTIFICATE, null, "cert-user2", + "certUser2", false, AppIdentityType.CERTIFICATE, null, "cert-user2", false, guestAuthor, guest).join().content(); assertThat(userCert1.isActive()).isTrue(); assertThat(userCert2.isActive()).isTrue(); @@ -295,7 +311,7 @@ void userCertificate() { @Test void nonRandomToken() { - final Token token = appIdentityRegistryService.createToken("forAdmin1", true, "appToken-secret", + final Token token = appIdentityRegistryService.createToken("forAdmin1", true, "appToken-secret", false, systemAdminAuthor, systemAdmin) .join().content(); @@ -305,7 +321,8 @@ void nonRandomToken() { assertThat(tokens.stream().filter(t -> !StringUtil.isNullOrEmpty(t.secret()))).hasSize(1); assertThatThrownBy(() -> appIdentityRegistryService.createToken("forUser1", true, - "appToken-secret", guestAuthor, guest) + "appToken-secret", false, + guestAuthor, guest) .join()) .isInstanceOf(IllegalArgumentException.class); @@ -317,7 +334,7 @@ void nonRandomToken() { @Test public void updateToken() { - final Token token = appIdentityRegistryService.createToken("forUpdate", true, null, + final Token token = appIdentityRegistryService.createToken("forUpdate", true, null, false, systemAdminAuthor, systemAdmin).join() .content(); assertThat(token.isActive()).isTrue(); @@ -352,7 +369,7 @@ public void updateToken() { public void updateCertificate() { final CertificateAppIdentity certificate = (CertificateAppIdentity) appIdentityRegistryService.createAppIdentity( - "certUpdate", true, AppIdentityType.CERTIFICATE, null, "cert/update", + "certUpdate", true, AppIdentityType.CERTIFICATE, null, "cert/update", false, systemAdminAuthor, systemAdmin).join().content(); assertThat(certificate.isActive()).isTrue(); @@ -385,7 +402,7 @@ public void updateCertificate() { @Test void updateTokenLevel() { - final Token token = appIdentityRegistryService.createToken("forUpdate", false, null, + final Token token = appIdentityRegistryService.createToken("forUpdate", false, null, false, systemAdminAuthor, systemAdmin).join() .content(); assertThat(token.isActive()).isTrue(); @@ -415,7 +432,7 @@ void updateTokenLevel() { void updateCertificateLevel() { final CertificateAppIdentity certificate = (CertificateAppIdentity) appIdentityRegistryService.createAppIdentity( - "certLevelUpdate", false, AppIdentityType.CERTIFICATE, null, "cert-level", + "certLevelUpdate", false, AppIdentityType.CERTIFICATE, null, "cert-level", false, systemAdminAuthor, systemAdmin).join().content(); assertThat(certificate.isActive()).isTrue(); assertThat(certificate.isSystemAdmin()).isFalse(); diff --git a/server/src/test/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1Test.java b/server/src/test/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1Test.java index 8684eb24f..407fbc7e8 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1Test.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1Test.java @@ -26,6 +26,7 @@ import static net.javacrumbs.jsonunit.fluent.JsonFluentAssert.assertThatJson; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.awaitility.Awaitility.await; import java.io.IOException; import java.net.URI; @@ -209,6 +210,80 @@ void createRepositoryWithInvalidName() { assertThat(aRes.headers().status()).isSameAs(HttpStatus.BAD_REQUEST); } + @Test + void createPublicRepository() { + // Create a repository with the "isPublic" flag set. + final RequestHeaders headers = RequestHeaders.of(HttpMethod.POST, REPOS_PREFIX, + HttpHeaderNames.CONTENT_TYPE, MediaType.JSON); + final AggregatedHttpResponse aRes = + systemAdminClient.execute(headers, "{\"name\": \"publicRepo\", \"isPublic\": true}") + .aggregate().join(); + assertThat(ResponseHeaders.of(aRes.headers()).status()).isEqualTo(HttpStatus.CREATED); + + // A public repository grants the guest the READ role. + assertThat(repoRoles("publicRepo").projectRoles().member()).isSameAs(RepositoryRole.WRITE); + assertThat(repoRoles("publicRepo").projectRoles().guest()).isSameAs(RepositoryRole.READ); + + // Creating a repository without the flag keeps it private (the guest has no role). + final AggregatedHttpResponse privateRes = createRepository(systemAdminClient, "privateRepo"); + assertThat(ResponseHeaders.of(privateRes.headers()).status()).isEqualTo(HttpStatus.CREATED); + assertThat(repoRoles("privateRepo").projectRoles().guest()).isNull(); + + // Keep the shared project free of public repositories for the other tests. + final RequestHeaders rolesHeaders = RequestHeaders.of( + HttpMethod.POST, "/api/v1/metadata/myPro/repos/publicRepo/roles/projects", + HttpHeaderNames.CONTENT_TYPE, MediaType.JSON); + final AggregatedHttpResponse reverted = + systemAdminClient.execute(rolesHeaders, "{\"member\": \"WRITE\", \"guest\": null}") + .aggregate().join(); + assertThat(reverted.status()).isSameAs(HttpStatus.OK); + } + + @Test + void createPublicRepositoryRejectedWhenProjectDisallows() { + // Disallow public repositories in the project. + AggregatedHttpResponse res = updateAllowPublicRepositories(false); + assertThat(res.status()).isSameAs(HttpStatus.OK); + // Wait until the cached project metadata catches up; the create API pre-checks against it. + await().untilAsserted(() -> assertThat(projectMetadata().allowPublicRepositories()).isFalse()); + + // Creating a public repository is rejected before the repository is created. + final RequestHeaders headers = RequestHeaders.of(HttpMethod.POST, REPOS_PREFIX, + HttpHeaderNames.CONTENT_TYPE, MediaType.JSON); + res = systemAdminClient.execute(headers, "{\"name\": \"rejectedRepo\", \"isPublic\": true}") + .aggregate().join(); + assertThat(res.status()).isSameAs(HttpStatus.BAD_REQUEST); + assertThat(res.contentUtf8()).contains("Public repositories are not allowed"); + + // No orphaned repository is left behind: creating with the same name succeeds. + res = createRepository(systemAdminClient, "rejectedRepo"); + assertThat(ResponseHeaders.of(res.headers()).status()).isEqualTo(HttpStatus.CREATED); + + // Re-allow public repositories for other tests. + res = updateAllowPublicRepositories(true); + assertThat(res.status()).isSameAs(HttpStatus.OK); + await().untilAsserted(() -> assertThat(projectMetadata().allowPublicRepositories()).isTrue()); + } + + private static AggregatedHttpResponse updateAllowPublicRepositories(boolean allow) { + final RequestHeaders headers = RequestHeaders.of( + HttpMethod.PUT, "/api/v1/metadata/myPro/settings", + HttpHeaderNames.CONTENT_TYPE, MediaType.JSON); + return systemAdminClient.execute(headers, "{\"allowPublicRepositories\": " + allow + '}') + .aggregate().join(); + } + + private static ProjectMetadata projectMetadata() { + return systemAdminClient.blocking().prepare().get(PROJECTS_PREFIX + "/myPro") + .asJson(ProjectMetadata.class, new ObjectMapper()) + .execute() + .content(); + } + + private static Roles repoRoles(String repoName) { + return projectMetadata().repo(repoName).roles(); + } + @Test void createRepositoryInAbsentProject() { final RequestHeaders headers = RequestHeaders.of(HttpMethod.POST, diff --git a/server/src/test/java/com/linecorp/centraldogma/server/metadata/MetadataApiServiceTest.java b/server/src/test/java/com/linecorp/centraldogma/server/metadata/MetadataApiServiceTest.java index 423585ee6..d73d14757 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/metadata/MetadataApiServiceTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/metadata/MetadataApiServiceTest.java @@ -21,7 +21,9 @@ import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.getAccessToken; import static net.javacrumbs.jsonunit.fluent.JsonFluentAssert.assertThatJson; import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; +import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; @@ -377,6 +379,93 @@ void shouldNotAllowWritePermissionForGuest() { assertThat(successResponse.content().major()).isGreaterThan(0); } + @Test + void updateProjectRolesWithoutGuestField() { + // A missing guest field means a private repository, not an internal server error. + final ResponseEntity response = + systemAdminClient.prepare() + .post("/api/v1/metadata/{project}/repos/{repo}/roles/projects") + .pathParam("project", PROJECT_NAME) + .pathParam("repo", REPOSITORY_NAME) + .content(MediaType.JSON, "{ \"member\": \"WRITE\" }") + .asJson(Revision.class) + .execute(); + assertThat(response.status()).isEqualTo(HttpStatus.OK); + await().untilAsserted( + () -> assertThat(projectMetadata().repo(REPOSITORY_NAME).roles().projectRoles().guest()) + .isNull()); + } + + @Test + void allowPublicRepositoriesLifecycle() { + // Make the repository public. + updateGuestRole(RepositoryRole.READ); + + // Cannot disallow public repositories while one exists. + AggregatedHttpResponse response = putAllowPublicRepositories(false); + assertThat(response.status()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.contentUtf8()).contains("Make the following repositories private first") + .contains(REPOSITORY_NAME); + + // Make the repository private, then disallow. + updateGuestRole(null); + response = putAllowPublicRepositories(false); + assertThat(response.status()).isEqualTo(HttpStatus.OK); + + // Disallowing again is a no-op returning the current head revision. + response = putAllowPublicRepositories(false); + assertThat(response.status()).isEqualTo(HttpStatus.OK); + + // Cannot make a repository public anymore. + final AggregatedHttpResponse guestUpdateResponse = + systemAdminClient.prepare() + .post("/api/v1/metadata/{project}/repos/{repo}/roles/projects") + .pathParam("project", PROJECT_NAME) + .pathParam("repo", REPOSITORY_NAME) + .contentJson(ProjectRoles.of(RepositoryRole.WRITE, RepositoryRole.READ)) + .execute(); + assertThat(guestUpdateResponse.status()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(guestUpdateResponse.contentUtf8()) + .contains("Public repositories are not allowed in the project"); + + // A missing repository is reported as 404, not as a policy violation. + final AggregatedHttpResponse notFound = + systemAdminClient.prepare() + .post("/api/v1/metadata/{project}/repos/{repo}/roles/projects") + .pathParam("project", PROJECT_NAME) + .pathParam("repo", "no_such_repo") + .contentJson(ProjectRoles.of(RepositoryRole.WRITE, RepositoryRole.READ)) + .execute(); + assertThat(notFound.status()).isEqualTo(HttpStatus.NOT_FOUND); + + // Re-allow public repositories and make sure the repository can be public again. + response = putAllowPublicRepositories(true); + assertThat(response.status()).isEqualTo(HttpStatus.OK); + updateGuestRole(RepositoryRole.READ); + // Restore the scaffold state. + updateGuestRole(null); + } + + private static void updateGuestRole(@Nullable RepositoryRole guestRole) { + final ResponseEntity response = + systemAdminClient.prepare() + .post("/api/v1/metadata/{project}/repos/{repo}/roles/projects") + .pathParam("project", PROJECT_NAME) + .pathParam("repo", REPOSITORY_NAME) + .contentJson(ProjectRoles.of(RepositoryRole.WRITE, guestRole)) + .asJson(Revision.class) + .execute(); + assertThat(response.status()).isEqualTo(HttpStatus.OK); + } + + private static AggregatedHttpResponse putAllowPublicRepositories(boolean allow) { + return systemAdminClient.prepare() + .put("/api/v1/metadata/{project}/settings") + .pathParam("project", PROJECT_NAME) + .content(MediaType.JSON, "{ \"allowPublicRepositories\": " + allow + " }") + .execute(); + } + @Test void repositoryAdminCanUpdateRepositoryMetadata() { addProjectMember(); @@ -447,6 +536,85 @@ void repositoryAdminCanUpdateRepositoryMetadataWithCert() { removeProjectMember(); } + @Test + void updateAllowPublicRepositories() throws JsonProcessingException { + // Use a dedicated project so the shared scaffold state is not mutated. + final String project = "allow_public_proj"; + dogma.client().createProject(project).join(); + + // A caller who is not the project owner is forbidden. + final AggregatedHttpResponse forbidden = + memberTokenClient.prepare() + .put("/api/v1/metadata/{project}/settings") + .pathParam("project", project) + .content(MediaType.JSON, "{ \"allowPublicRepositories\": false }") + .execute(); + assertThat(forbidden.status()).isSameAs(HttpStatus.FORBIDDEN); + + // A malformed body (non-boolean 'allow') is rejected with 400. + final AggregatedHttpResponse badRequest = + systemAdminClient.prepare() + .put("/api/v1/metadata/{project}/settings") + .pathParam("project", project) + .content(MediaType.JSON, "{ \"allowPublicRepositories\": \"not-a-boolean\" }") + .execute(); + assertThat(badRequest.status()).isSameAs(HttpStatus.BAD_REQUEST); + + // A body with no settings is rejected as well. + final AggregatedHttpResponse emptySettings = + systemAdminClient.prepare() + .put("/api/v1/metadata/{project}/settings") + .pathParam("project", project) + .content(MediaType.JSON, "{}") + .execute(); + assertThat(emptySettings.status()).isSameAs(HttpStatus.BAD_REQUEST); + + // A project MEMBER is forbidden as well. + final HttpRequest addMember = + HttpRequest.builder() + .post("/api/v1/metadata/" + project + "/appIdentities") + .contentJson(new IdAndProjectRole(MEMBER_TOKEN_APP_ID, ProjectRole.MEMBER)) + .build(); + assertThat(systemAdminClient.execute(addMember).status()).isSameAs(HttpStatus.OK); + final AggregatedHttpResponse memberForbidden = + memberTokenClient.prepare() + .put("/api/v1/metadata/{project}/settings") + .pathParam("project", project) + .content(MediaType.JSON, "{ \"allowPublicRepositories\": false }") + .execute(); + assertThat(memberForbidden.status()).isSameAs(HttpStatus.FORBIDDEN); + + // A project OWNER who is not a system administrator can update the setting. + final JsonPatch toOwner = JsonPatch.generate(Jackson.readTree("{\"role\":\"MEMBER\"}"), + Jackson.readTree("{\"role\":\"OWNER\"}"), + ReplaceMode.RFC6902); + final HttpRequest promote = + HttpRequest.builder() + .patch("/api/v1/metadata/" + project + "/appIdentities/" + MEMBER_TOKEN_APP_ID) + .content(MediaType.JSON_PATCH, Jackson.writeValueAsString(toOwner)) + .build(); + assertThat(systemAdminClient.execute(promote).status()).isSameAs(HttpStatus.OK); + final ResponseEntity ok = + memberTokenClient.prepare() + .put("/api/v1/metadata/{project}/settings") + .pathParam("project", project) + .content(MediaType.JSON, "{ \"allowPublicRepositories\": false }") + .asJson(Revision.class) + .execute(); + assertThat(ok.status()).isSameAs(HttpStatus.OK); + assertThat(ok.content().major()).isGreaterThan(0); + + await().untilAsserted(() -> { + final ProjectMetadata metadata = + systemAdminClient.prepare() + .get("/api/v1/projects/" + project) + .asJson(ProjectMetadata.class, new ObjectMapper()) + .execute() + .content(); + assertThat(metadata.allowPublicRepositories()).isFalse(); + }); + } + private static ProjectMetadata projectMetadata() { return systemAdminClient.prepare() .get("/api/v1/projects/" + PROJECT_NAME) diff --git a/server/src/test/java/com/linecorp/centraldogma/server/metadata/MetadataServiceTest.java b/server/src/test/java/com/linecorp/centraldogma/server/metadata/MetadataServiceTest.java index 8a897da20..1e100d8a7 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/metadata/MetadataServiceTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/metadata/MetadataServiceTest.java @@ -17,6 +17,7 @@ package com.linecorp.centraldogma.server.metadata; import static com.linecorp.centraldogma.server.metadata.RepositoryMetadata.DEFAULT_PROJECT_ROLES; +import static com.linecorp.centraldogma.server.metadata.RepositoryMetadata.PUBLIC_PROJECT_ROLES; import static com.linecorp.centraldogma.server.storage.project.Project.REPO_DOGMA; import static com.linecorp.centraldogma.server.storage.project.Project.REPO_META; import static org.assertj.core.api.Assertions.assertThat; @@ -44,6 +45,7 @@ import com.linecorp.centraldogma.common.RepositoryRole; import com.linecorp.centraldogma.common.RepositoryStatus; import com.linecorp.centraldogma.common.Revision; +import com.linecorp.centraldogma.internal.Jackson; import com.linecorp.centraldogma.server.command.Command; import com.linecorp.centraldogma.testing.internal.ProjectManagerExtension; @@ -78,12 +80,14 @@ protected boolean runForEachTest() { private static final String cert2 = "cert-2"; private static final String certificateId1 = "certificate/id/1"; private static final String certificateId2 = "certificate/id/2"; - private static final Token appToken1 = new Token(app1, "secret", false, true, UserAndTimestamp.of(author)); - private static final Token appToken2 = new Token(app2, "secret", false, true, UserAndTimestamp.of(author)); - private static final CertificateAppIdentity certificate1 = - new CertificateAppIdentity(cert1, certificateId1, false, true, UserAndTimestamp.of(author)); - private static final CertificateAppIdentity certificate2 = - new CertificateAppIdentity(cert2, certificateId2, false, true, UserAndTimestamp.of(author)); + private static final User appToken1 = new UserWithAppIdentity( + new Token(app1, "secret", false, true, UserAndTimestamp.of(author))); + private static final User appToken2 = new UserWithAppIdentity( + new Token(app2, "secret", false, true, UserAndTimestamp.of(author))); + private static final User certificate1 = new UserWithAppIdentity( + new CertificateAppIdentity(cert1, certificateId1, false, true, UserAndTimestamp.of(author))); + private static final User certificate2 = new UserWithAppIdentity( + new CertificateAppIdentity(cert2, certificateId2, false, true, UserAndTimestamp.of(author))); @Test void project() { @@ -222,6 +226,181 @@ void repositoryProjectRoles() { .hasMessageContaining("Can't update role for internal repository: meta"); } + @Test + void allowPublicRepositories_defaultAllowedAndToggle() { + final MetadataService mds = newMetadataService(manager); + + // A newly-created project allows public repositories by default. + final ProjectMetadata metadata = getProject(mds, project1); + assertThat(metadata.allowPublicRepositories()).isTrue(); + + // Allowing again is a no-op returning the current head revision, like a redundant status update. + final Revision headRevision = mds.updateAllowPublicRepositories(author, project1, true).join(); + assertThat(mds.updateAllowPublicRepositories(author, project1, true).join()) + .isEqualTo(headRevision); + + // Disallow public repositories. + final Revision disallowed = mds.updateAllowPublicRepositories(author, project1, false).join(); + assertThat(disallowed.major()).isGreaterThan(headRevision.major()); + await().untilAsserted( + () -> assertThat(getProject(mds, project1).allowPublicRepositories()).isFalse()); + assertThat(getProject(mds, project1).allowPublicRepositories()).isFalse(); + + // Disallowing again is a no-op. + assertThat(mds.updateAllowPublicRepositories(author, project1, false).join()).isEqualTo(disallowed); + + // Allow public repositories again. + mds.updateAllowPublicRepositories(author, project1, true).join(); + await().untilAsserted( + () -> assertThat(getProject(mds, project1).allowPublicRepositories()).isTrue()); + assertThat(getProject(mds, project1).allowPublicRepositories()).isTrue(); + } + + @Test + void publicRepository_isReadableByGuest() { + final MetadataService mds = newMetadataService(manager); + + // Create a public repository whose guests have the READ role. + mds.addRepo(author, project1, repo1, PUBLIC_PROJECT_ROLES).join(); + await().until(() -> getRepo1(mds) != null); + assertThat(getRepo1(mds).roles().projectRoles().guest()).isSameAs(RepositoryRole.READ); + + // A non-member (guest) can read the public repository without being granted any role. + assertThat(mds.findRepositoryRole(project1, repo1, guest).join()).isSameAs(RepositoryRole.READ); + + // Turning the repository private removes the guest's access. + mds.updateRepositoryProjectRoles(author, project1, repo1, DEFAULT_PROJECT_ROLES).join(); + await().untilAsserted( + () -> assertThat(mds.findRepositoryRole(project1, repo1, guest).join()).isNull()); + } + + @Test + void createPublicRepository_rejectedWhenProjectDisallows() { + final MetadataService mds = newMetadataService(manager); + + // Disallow public repositories in the project. + mds.updateAllowPublicRepositories(author, project1, false).join(); + await().untilAsserted( + () -> assertThat(getProject(mds, project1).allowPublicRepositories()).isFalse()); + + // Creating a public repository is rejected. + assertThatThrownBy(() -> mds.addRepo(author, project1, repo1, PUBLIC_PROJECT_ROLES).join()) + .hasCauseInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Public repositories are not allowed"); + + // A private repository can still be created. + mds.addRepo(author, project1, repo2, DEFAULT_PROJECT_ROLES).join(); + await().untilAsserted(() -> assertThat(getProject(mds, project1).repos()).containsKey(repo2)); + assertThat(getProject(mds, project1).repos().get(repo2).roles().projectRoles().guest()).isNull(); + } + + @Test + void updateRepositoryToPublic_rejectedWhenProjectDisallows() { + final MetadataService mds = newMetadataService(manager); + + // Start with a private repository. + mds.addRepo(author, project1, repo1, DEFAULT_PROJECT_ROLES).join(); + await().until(() -> getRepo1(mds) != null); + + // Disallow public repositories. + mds.updateAllowPublicRepositories(author, project1, false).join(); + await().untilAsserted( + () -> assertThat(getProject(mds, project1).allowPublicRepositories()).isFalse()); + + // Turning the repository public is rejected and the guest gains no access. + assertThatThrownBy(() -> mds.updateRepositoryProjectRoles( + author, project1, repo1, PUBLIC_PROJECT_ROLES).join()) + .hasCauseInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Public repositories are not allowed"); + assertThat(mds.findRepositoryRole(project1, repo1, guest).join()).isNull(); + + // A missing repository is reported as not found, not as a policy violation. + assertThatThrownBy(() -> mds.updateRepositoryProjectRoles( + author, project1, "missing", PUBLIC_PROJECT_ROLES).join()) + .hasCauseInstanceOf(RepositoryNotFoundException.class); + } + + @Test + void disallowPublicRepositories_blockedWhilePublicRepositoryExists() { + final MetadataService mds = newMetadataService(manager); + + // Create a public repository. + mds.addRepo(author, project1, repo1, PUBLIC_PROJECT_ROLES).join(); + await().until(() -> getRepo1(mds) != null); + + // Disallowing is rejected while a public repository still exists; the message names the repo. + assertThatThrownBy(() -> mds.updateAllowPublicRepositories(author, project1, false).join()) + .hasCauseInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot disallow public repositories") + .hasMessageContaining(repo1); + // The change was rejected, so public repositories remain allowed. + assertThat(getProject(mds, project1).allowPublicRepositories()).isTrue(); + + // After privatising the repository, disallowing succeeds. + mds.updateRepositoryProjectRoles(author, project1, repo1, DEFAULT_PROJECT_ROLES).join(); + await().untilAsserted(() -> assertThat(getRepo1(mds).roles().projectRoles().guest()).isNull()); + mds.updateAllowPublicRepositories(author, project1, false).join(); + await().untilAsserted( + () -> assertThat(getProject(mds, project1).allowPublicRepositories()).isFalse()); + } + + @Test + void disallowPublicRepositories_blockedByRemovedPublicRepository() { + final MetadataService mds = newMetadataService(manager); + + // Create a public repository and remove it. + mds.addRepo(author, project1, repo1, PUBLIC_PROJECT_ROLES).join(); + await().until(() -> getRepo1(mds) != null); + mds.removeRepo(author, project1, repo1).join(); + await().untilAsserted(() -> assertThat(getRepo1(mds).removal()).isNotNull()); + + // A removed public repository still blocks disallowing; restoring it must not resurrect + // a guest role into a project which disallows public repositories. + assertThatThrownBy(() -> mds.updateAllowPublicRepositories(author, project1, false).join()) + .hasCauseInstanceOf(IllegalArgumentException.class) + .hasMessageContaining(repo1 + " (removed)"); + + // After purging the repository, disallowing succeeds. + mds.purgeRepo(author, project1, repo1).join(); + await().untilAsserted(() -> assertThatThrownBy(() -> getRepo1(mds)) + .isInstanceOf(RepositoryNotFoundException.class)); + mds.updateAllowPublicRepositories(author, project1, false).join(); + await().untilAsserted( + () -> assertThat(getProject(mds, project1).allowPublicRepositories()).isFalse()); + } + + @Test + void updateRepositoryProjectRoles_allowedForLegacyPublicRepository() { + final MetadataService mds = newMetadataService(manager); + + // Simulate legacy metadata: a public repository in a project which disallows public + // repositories. This state is unreachable via the API but may exist in old metadata. + mds.addRepo(author, project1, repo1, PUBLIC_PROJECT_ROLES).join(); + await().until(() -> getRepo1(mds) != null); + final ProjectMetadata current = getProject(mds, project1); + final ProjectMetadata legacy = new ProjectMetadata(current.name(), + current.repos(), + current.members(), + null, + current.appIds(), + false, + current.creation(), + current.removal()); + manager.executor().execute(Command.push( + author, project1, REPO_DOGMA, Revision.HEAD, "Simulate legacy metadata", "", + Markup.PLAINTEXT, + Change.ofJsonUpsert(MetadataService.METADATA_JSON, Jackson.valueToTree(legacy)))).join(); + await().untilAsserted( + () -> assertThat(getProject(mds, project1).allowPublicRepositories()).isFalse()); + + // Updating the member role of an already-public repository is not a private-to-public + // transition, so it is not rejected. + mds.updateRepositoryProjectRoles(author, project1, repo1, + ProjectRoles.of(RepositoryRole.READ, RepositoryRole.READ)).join(); + await().untilAsserted(() -> assertThat(getRepo1(mds).roles().projectRoles().member()) + .isSameAs(RepositoryRole.READ)); + } + @Test void userRepositoryRole() { final MetadataService mds = newMetadataService(manager); @@ -699,7 +878,8 @@ void effectiveRepositoryRole_appIdentityWithoutGuestAccess() { // but has an explicit repository role, it should still be accessible. final MetadataService mds = newMetadataService(manager); - final Token noGuestToken = new Token(app1, "secret", false, false, UserAndTimestamp.of(author)); + final User noGuestToken = new UserWithAppIdentity( + new Token(app1, "secret", false, false, UserAndTimestamp.of(author))); mds.addRepo(author, project1, repo1, ProjectRoles.of(RepositoryRole.WRITE, RepositoryRole.READ)).join(); @@ -714,8 +894,8 @@ void effectiveRepositoryRole_appIdentityWithoutGuestAccess() { .isSameAs(RepositoryRole.READ)); // Without an explicit repository role, allowGuestAccess=false should deny access. - assertThat(mds.findRepositoryRole(project1, repo1, - new Token(app2, "secret2", false, false, UserAndTimestamp.of(author))).join()) + assertThat(mds.findRepositoryRole(project1, repo1, new UserWithAppIdentity( + new Token(app2, "secret2", false, false, UserAndTimestamp.of(author)))).join()) .isNull(); } @@ -776,6 +956,36 @@ void updateRepositoryStatus() { assertThat(revision).isEqualTo(new Revision(9)); } + @Test + void createTokenWithGuestAccess() { + final MetadataService mds = newMetadataService(manager); + mds.addRepo(author, project1, repo1, PUBLIC_PROJECT_ROLES).join(); + await().until(() -> getRepo1(mds) != null); + + // A token which is created with allowGuestAccess reads a public repository without registration. + mds.createToken(author, app1, null, false, true).join(); + waitUntilAppIdentityRegistered(mds, app1); + assertThat(mds.findAppIdentity(app1).allowGuestAccess()).isTrue(); + final User guestAccessToken = new UserWithAppIdentity(mds.findAppIdentity(app1)); + assertThat(mds.findRepositoryRole(project1, repo1, guestAccessToken).join()) + .isSameAs(RepositoryRole.READ); + + // A token which is created without allowGuestAccess cannot. + mds.createToken(author, app2).join(); + waitUntilAppIdentityRegistered(mds, app2); + assertThat(mds.findAppIdentity(app2).allowGuestAccess()).isFalse(); + final User noGuestAccessToken = new UserWithAppIdentity(mds.findAppIdentity(app2)); + assertThat(mds.findRepositoryRole(project1, repo1, noGuestAccessToken).join()).isNull(); + + // A certificate which is created with allowGuestAccess reads a public repository as well. + mds.createCertificate(author, cert1, certificateId1, false, true).join(); + waitUntilAppIdentityRegistered(mds, cert1); + assertThat(mds.findAppIdentity(cert1).allowGuestAccess()).isTrue(); + final User guestAccessCertificate = new UserWithAppIdentity(mds.findAppIdentity(cert1)); + assertThat(mds.findRepositoryRole(project1, repo1, guestAccessCertificate).join()) + .isSameAs(RepositoryRole.READ); + } + private static RepositoryMetadata getRepo1(MetadataService mds) { final ProjectMetadata metadata = mds.getProject(project1).join(); return metadata.repo(repo1); diff --git a/server/src/test/java/com/linecorp/centraldogma/server/metadata/ProjectMetadataTest.java b/server/src/test/java/com/linecorp/centraldogma/server/metadata/ProjectMetadataTest.java index e3500660e..95b4d8d40 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/metadata/ProjectMetadataTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/metadata/ProjectMetadataTest.java @@ -78,6 +78,7 @@ void serialize() throws Exception { ImmutableMap.of("app-id-1", new AppIdentityRegistration("app-id-1", ProjectRole.MEMBER, creation) ), + null, new UserAndTimestamp(User.SYSTEM.id()), null); diff --git a/server/src/test/java/com/linecorp/centraldogma/server/metadata/TokenGuestPermissionTest.java b/server/src/test/java/com/linecorp/centraldogma/server/metadata/TokenGuestPermissionTest.java index 6e33409c7..9465f0ab0 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/metadata/TokenGuestPermissionTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/metadata/TokenGuestPermissionTest.java @@ -31,6 +31,9 @@ import com.linecorp.armeria.client.BlockingWebClient; import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.Cookie; +import com.linecorp.armeria.common.HttpHeaderNames; import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.MediaType; import com.linecorp.armeria.common.QueryParams; @@ -44,7 +47,9 @@ import com.linecorp.centraldogma.common.ProjectRole; import com.linecorp.centraldogma.common.RepositoryRole; import com.linecorp.centraldogma.common.Revision; +import com.linecorp.centraldogma.internal.Jackson; import com.linecorp.centraldogma.server.CentralDogmaBuilder; +import com.linecorp.centraldogma.server.internal.admin.auth.SessionUtil; import com.linecorp.centraldogma.server.internal.api.MetadataApiService.IdAndProjectRole; import com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil; import com.linecorp.centraldogma.testing.internal.auth.TestAuthProviderFactory; @@ -54,6 +59,7 @@ class TokenGuestPermissionTest { private static final String FOO_PROJ = "foo"; private static final String BAR_REPO = "bar"; + private static final String PRIVATE_REPO = "qux"; @RegisterExtension static final CentralDogmaExtension dogma = new CentralDogmaExtension() { @@ -76,6 +82,9 @@ protected void scaffold(CentralDogma client) { client.createProject(FOO_PROJ).join(); final CentralDogmaRepository repo = client.createRepository(FOO_PROJ, BAR_REPO).join(); repo.commit("test", Change.ofTextUpsert("/a.txt", "foo")).push().join(); + final CentralDogmaRepository privateRepo = + client.createRepository(FOO_PROJ, PRIVATE_REPO).join(); + privateRepo.commit("test", Change.ofTextUpsert("/a.txt", "secret")).push().join(); } }; @@ -142,6 +151,94 @@ void testNormalToken() throws UnknownHostException { assertThat(entry.contentAsText().trim()).isEqualTo("foo"); } + @Test + void testGuestAccessToken() throws UnknownHostException { + final BlockingWebClient client = dogma.blockingHttpClient(); + + final String appId = "guest-access-test"; + final ResponseEntity response = + client.prepare() + .post("/api/v1/appIdentities") + .content(MediaType.FORM_DATA, + QueryParams.of("appId", appId, "type", "TOKEN", + "allowGuestAccess", true) + .toQueryString()) + .asJson(Token.class, new ObjectMapper()) + .execute(); + assertThat(response.status()).isEqualTo(HttpStatus.CREATED); + final Token token = response.content(); + assertThat(token.isSystemAdmin()).isFalse(); + assertThat(token.allowGuestAccess()).isTrue(); + + final CentralDogma dogmaClient = new ArmeriaCentralDogmaBuilder() + .host(dogma.serverAddress().getHostString(), dogma.serverAddress().getPort()) + .accessToken(token.secret()) + .build(); + + // Reads the public (guest READ) repository without any registration. + final Entry entry = dogmaClient.forRepo(FOO_PROJ, BAR_REPO) + .file("/a.txt") + .get().join(); + assertThat(entry.contentAsText().trim()).isEqualTo("foo"); + + // Guests can never write. + assertThatThrownBy(() -> { + dogmaClient.forRepo(FOO_PROJ, BAR_REPO) + .commit("write", Change.ofTextUpsert("/b.txt", "bar")) + .push().join(); + }).isInstanceOf(CompletionException.class) + .hasCauseInstanceOf(PermissionException.class) + .hasMessageContaining("You must have the WRITE repository role to access the 'foo/bar'"); + + // A private repository is still inaccessible. + assertThatThrownBy(() -> { + dogmaClient.forRepo(FOO_PROJ, PRIVATE_REPO) + .file("/a.txt") + .get().join(); + }).isInstanceOf(CompletionException.class) + .hasCauseInstanceOf(PermissionException.class) + .hasMessageContaining("You must have the READ repository role to access the 'foo/qux'"); + } + + @Test + void testNonMemberUser() throws Exception { + // A signed-in user who is not a project member reads the public repository, but not the + // private one. + final WebClient client = WebClient.of("http://127.0.0.1:" + dogma.serverAddress().getPort()); + final AggregatedHttpResponse loginRes = TestAuthMessageUtil.login( + client, TestAuthMessageUtil.USERNAME2, TestAuthMessageUtil.PASSWORD2); + assertThat(loginRes.status()).isEqualTo(HttpStatus.OK); + final Cookie sessionCookie = TestAuthMessageUtil.getSessionCookie(loginRes); + final String csrfToken = Jackson.readTree(loginRes.contentUtf8()).get("csrf_token").asText(); + final BlockingWebClient userClient = + WebClient.builder(client.uri()) + .addHeader(HttpHeaderNames.COOKIE, sessionCookie.toCookieHeader()) + .addHeader(SessionUtil.X_CSRF_TOKEN, csrfToken) + .build() + .blocking(); + + assertThat(userClient.get("/api/v1/projects/foo/repos/bar/list/a.txt").status()) + .isEqualTo(HttpStatus.OK); + assertThat(userClient.get("/api/v1/projects/foo/repos/qux/list/a.txt").status()) + .isEqualTo(HttpStatus.FORBIDDEN); + + // A signed-in non-member can never write to the public repository. + final AggregatedHttpResponse writeRes = + userClient.prepare() + .post("/api/v1/projects/foo/repos/bar/contents") + .content(MediaType.JSON, + "{\"commitMessage\":{\"summary\":\"write\"}," + + "\"changes\":[{\"path\":\"/b.txt\",\"type\":\"UPSERT_TEXT\"," + + "\"content\":\"b\"}]}") + .execute(); + assertThat(writeRes.status()).isEqualTo(HttpStatus.FORBIDDEN); + + // Public does not mean anonymous: an unauthenticated client is rejected. + final AggregatedHttpResponse unauthenticated = + WebClient.of(client.uri()).blocking().get("/api/v1/projects/foo/repos/bar/list/a.txt"); + assertThat(unauthenticated.status()).isEqualTo(HttpStatus.UNAUTHORIZED); + } + @Test void testSystemAdminToken() throws UnknownHostException { final BlockingWebClient client = dogma.blockingHttpClient(); diff --git a/site/src/sphinx/_images/auth_1.png b/site/src/sphinx/_images/auth_1.png index 29265c377..297cd7804 100644 Binary files a/site/src/sphinx/_images/auth_1.png and b/site/src/sphinx/_images/auth_1.png differ diff --git a/site/src/sphinx/_images/auth_2.png b/site/src/sphinx/_images/auth_2.png index 1ef1ef4b0..b50b85f46 100644 Binary files a/site/src/sphinx/_images/auth_2.png and b/site/src/sphinx/_images/auth_2.png differ diff --git a/site/src/sphinx/_images/auth_3.png b/site/src/sphinx/_images/auth_3.png index 9d8e7c714..6f39d5044 100644 Binary files a/site/src/sphinx/_images/auth_3.png and b/site/src/sphinx/_images/auth_3.png differ diff --git a/site/src/sphinx/_images/auth_4.png b/site/src/sphinx/_images/auth_4.png index 9aaa98d34..5faf97a1c 100644 Binary files a/site/src/sphinx/_images/auth_4.png and b/site/src/sphinx/_images/auth_4.png differ diff --git a/site/src/sphinx/auth.rst b/site/src/sphinx/auth.rst index 5bf206ac6..2ace90ad0 100644 --- a/site/src/sphinx/auth.rst +++ b/site/src/sphinx/auth.rst @@ -98,7 +98,7 @@ the authentication to. "sessionValidationSchedule": "0 30 */4 ? * *", "properties": { "entityId": "dogma", - "hostname": "dogma-example.linecorp.com", + "hostname": "dogma.example.com", "signingKey": "signing", "encryptionKey": "encryption", "keyStore": { @@ -114,11 +114,11 @@ the authentication to. "acs": { "endpoints": [ { - "uri": "https://dogma-example.linecorp.com/saml/acs/post", + "uri": "https://dogma.example.com/saml/acs/post", "binding": "HTTP_POST" }, { - "uri": "https://dogma-example.linecorp.com/saml/acs/redirect", + "uri": "https://dogma.example.com/saml/acs/redirect", "binding": "HTTP_REDIRECT" } ] @@ -287,13 +287,14 @@ You may configure a project with HTTP APIs, but we recommend the web UI because Everyone who is logged in is able to create a new project, and he or she would be an owner of the project. If you have the right to configure a project, in other words, if you are an owner of the project, -you can access the configuration UI of the project by clicking the cog icon which is shown on the right -of the project name. +you can access the configuration UI of the project by clicking the ``Project Settings`` button on the +project page. .. image:: _images/auth_1.png -If you click the icon, you can see the configuration UI for a project like below. In this page, you can -add a user or a token as a member of the project and can also remove them from the project. +If you click the button, you can see the configuration UI for a project like below. In the ``Members`` +tab, you can add a user as a member of the project and can also remove them from the project. +An application identity can be added to the project in the ``App Identities`` tab in the same way. .. image:: _images/auth_2.png @@ -309,9 +310,9 @@ of ``Owner`` and ``Member`` role in the UI. More information about the role is a - ``Owner`` of a project - the administrator of a project. A user who creates a project is to be an owner of the project by - default. Owners can add a user or a token as an owner or a member of the project, and can create - a new repository. Also, they can remove the repository or the project from the system and can - configure permissions for each role, member and token. + default. Owners can add a user or an application identity as an owner or a member of the project, + and can create a new repository. Also, they can remove the repository or the project from the + system and can configure repository roles for each project role, member and application identity. - ``Member`` of a project @@ -324,29 +325,42 @@ of ``Owner`` and ``Member`` role in the UI. More information about the role is a .. note:: - Do not forget to make a new ``Application Token`` before adding a token to a project. ``Add a token`` - button would be disabled if there is no token. The cog icon on the right of the ``Tokens`` title - brings you to the ``Application Token`` management page. + Do not forget to create a new application identity before adding it to a project. You can create + one in the ``Application Identities`` page under the ``Settings`` menu of the web UI. -You can see the configuration UI for a repository when you click the name of repository in the -``Repository Permission`` list. The following image shows the configuration of the repository called ``main``. +You can see the configuration UI for a repository when you click the ``Repository Settings`` button +on the repository page. The following image shows the configuration of the repository called ``bar``. In this page, you can do the followings. -- Changing the role of a member or a token in a project -- Setting permissions of each role for a repository -- Setting permissions of a specific member or token for a repository +- Changing the repository role granted to the project members +- Switching the visibility of the repository between private and public +- Granting a repository role to a specific user or application identity in the ``Users`` and + ``App Identities`` tabs .. image:: _images/auth_3.png -Permissions can be specified for a repository only. So a user can configure their repositories with different -access control levels. There are only two permission types currently, which are ``READ`` and ``WRITE``. -``WRITE`` permission implies ``READ`` permission, so you cannot give only WRITE permission to a user, -a token or any role. +Roles can be specified for a repository only. So a user can configure their repositories with different +access control levels. There are three repository roles, which are ``READ``, ``WRITE`` and ``ADMIN``. +``ADMIN`` implies ``WRITE``, and ``WRITE`` implies ``READ``. Every access of HTTP API will be controlled by the access control system. A request is allowed only if the -user of the request has sufficient permissions. If permissions for the user are specified in the repository -configuration, it would be used first to control the request. If it does not exist, permissions for each role -of the repository would be used to do that. +user of the request has a sufficient repository role. The effective role of a user is the highest of the +role granted to the user directly and the role granted to the user's project role (member or guest) in the +repository configuration. A system administrator and an owner of the project always have the ``ADMIN`` +role for every repository of the project. + +Public repositories +^^^^^^^^^^^^^^^^^^^ + +A repository whose guest role is ``READ`` is a *public* repository. A public repository can be read by +everyone who can sign in to the Central Dogma server — and by the application tokens and certificates +with guest access allowed — without being granted any role. Making a repository public only grants read +access; writing still requires a ``WRITE`` or higher role granted explicitly. A repository is private by +default; you can switch the visibility in the ``Roles`` tab of the repository settings page. + +A project owner can prevent the repositories of the project from being made public by disabling +``Allow public repositories`` in the project settings. Disabling it is rejected while the project still +has a public repository. Application Token ^^^^^^^^^^^^^^^^^ @@ -358,9 +372,9 @@ code because the user should log in again when the session token is expired. ``A useful for this case. ``Application Token`` is like a virtual user, so it can have any role in a project. Also, its permissions -can be specified in a repository configuration like a member. To get a new token, a user can use -``Application Tokens`` menu of the web UI. ``Application ID`` has to be unique to identify where a client -request comes from. +can be specified in a repository configuration like a member. To get a new token, a user can use the +``Application Identities`` page under the ``Settings`` menu of the web UI. ``Application ID`` has to be +unique to identify where a client request comes from. .. image:: _images/auth_4.png @@ -371,3 +385,8 @@ creator and the system administrator are allowed to deactivate and/or remove the There are two levels of a token, which are ``System Admin`` and ``User``. ``System Admin`` level token can be created by only the system administrators. A client who sends a request with the token is allowed to access system administrator-level APIs. + +When creating an application identity — a token or an mTLS certificate — you can also choose its scope. +By default, it can access only the projects and repositories it is granted a role for. If you allow +*guest access* when creating it, it can additionally read public repositories without being granted any +role. A ``System Admin`` level application identity always allows guest access. diff --git a/webapp/src/dogma/common/components/RepoIcon.tsx b/webapp/src/dogma/common/components/RepoIcon.tsx index e7c891f76..708ef4a64 100644 --- a/webapp/src/dogma/common/components/RepoIcon.tsx +++ b/webapp/src/dogma/common/components/RepoIcon.tsx @@ -1,5 +1,5 @@ import { isInternalRepo } from 'dogma/util/repo-util'; -import { Box, HStack } from '@chakra-ui/react'; +import { Badge, Box, HStack, Tooltip } from '@chakra-ui/react'; import { GoRepo, GoRepoLocked } from 'react-icons/go'; import { FiArchive } from 'react-icons/fi'; import { ChakraLink } from 'dogma/common/components/ChakraLink'; @@ -8,9 +8,11 @@ type RepoProps = { projectName: string; repoName: string; isActive: boolean; + isPublic?: boolean; + isAccessible?: boolean; }; -export const RepoIcon = ({ projectName, repoName, isActive }: RepoProps) => { +export const RepoIcon = ({ projectName, repoName, isActive, isPublic, isAccessible = true }: RepoProps) => { const isInternal = isInternalRepo(repoName); if (!isActive) { return ( @@ -36,6 +38,25 @@ export const RepoIcon = ({ projectName, repoName, isActive }: RepoProps) => { ); } + if (!isAccessible) { + return ( + + + + + + {repoName} + + + ); + } + return ( @@ -43,6 +64,11 @@ export const RepoIcon = ({ projectName, repoName, isActive }: RepoProps) => { {repoName} + {isPublic && ( + + Public + + )} ); diff --git a/webapp/src/dogma/features/api/apiSlice.ts b/webapp/src/dogma/features/api/apiSlice.ts index e5d65d02c..4260496f3 100644 --- a/webapp/src/dogma/features/api/apiSlice.ts +++ b/webapp/src/dogma/features/api/apiSlice.ts @@ -210,6 +210,14 @@ export const apiSlice = createApi({ }), invalidatesTags: ['Metadata'], }), + updateAllowPublicRepositories: builder.mutation({ + query: ({ projectName, allow }) => ({ + url: `/api/v1/metadata/${projectName}/settings`, + method: 'PUT', + body: { allowPublicRepositories: allow }, + }), + invalidatesTags: ['Metadata'], + }), addUserRepositoryRole: builder.mutation({ query: ({ projectName, repoName, data }) => ({ url: `/api/v1/metadata/${projectName}/repos/${repoName}/roles/users`, @@ -644,6 +652,7 @@ export const { useAddNewAppIdentityMemberMutation, useDeleteAppIdentityMemberMutation, useUpdateRepositoryProjectRolesMutation, + useUpdateAllowPublicRepositoriesMutation, useAddUserRepositoryRoleMutation, useDeleteUserRepositoryRoleMutation, useAddAppIdentityRepositoryRoleMutation, diff --git a/webapp/src/dogma/features/app-identity/NewAppIdentity.tsx b/webapp/src/dogma/features/app-identity/NewAppIdentity.tsx index 5c2c9e0aa..dbc7e4aa9 100644 --- a/webapp/src/dogma/features/app-identity/NewAppIdentity.tsx +++ b/webapp/src/dogma/features/app-identity/NewAppIdentity.tsx @@ -19,6 +19,7 @@ import { RadioGroup, Spacer, Stack, + Text, useDisclosure, } from '@chakra-ui/react'; import { SerializedError } from '@reduxjs/toolkit'; @@ -39,6 +40,7 @@ type FormData = { type: 'TOKEN' | 'CERTIFICATE'; certificateId?: string; isSystemAdmin: boolean; + scope: 'registered' | 'guest'; }; export const NewAppIdentity = () => { @@ -52,6 +54,7 @@ export const NewAppIdentity = () => { type: 'TOKEN', certificateId: '', isSystemAdmin: false, + scope: 'registered', }); onClose(); }; @@ -70,6 +73,7 @@ export const NewAppIdentity = () => { } = useForm({ defaultValues: { type: 'TOKEN', + scope: 'registered', }, }); const [addNewAppIdentity, { isLoading }] = useAddNewAppIdentityMutation(); @@ -78,6 +82,7 @@ export const NewAppIdentity = () => { const { user } = useAppSelector((state) => state.auth); const selectedType = watch('type'); + const isSystemAdminChecked = watch('isSystemAdmin'); const onSubmit = async (formData: FormData) => { if (formData.type === 'CERTIFICATE' && !formData.certificateId) { @@ -89,6 +94,8 @@ export const NewAppIdentity = () => { params.set('appId', formData.appId); params.set('type', formData.type); params.set('isSystemAdmin', String(formData.isSystemAdmin || false)); + // A system admin identity always allows guest access. + params.set('allowGuestAccess', String(formData.isSystemAdmin || formData.scope === 'guest')); if (formData.type === 'CERTIFICATE' && formData.certificateId) { params.set('certificateId', formData.certificateId); } @@ -159,6 +166,42 @@ export const NewAppIdentity = () => { )} + + Scope + ( + + + + Registered repositories only + + Accesses only the projects and repositories this app identity is granted a role for. + + + + Allow guest access + + Can also read public repositories without being granted any role. + + + + + )} + /> + + {isSystemAdminChecked + ? 'A system administrator-level app identity always allows guest access.' + : 'The scope cannot be changed after creation.'} + + + {selectedType === 'CERTIFICATE' && ( Certificate ID diff --git a/webapp/src/dogma/features/project/ProjectMetadataDto.ts b/webapp/src/dogma/features/project/ProjectMetadataDto.ts index ee4c60205..23008f1b1 100644 --- a/webapp/src/dogma/features/project/ProjectMetadataDto.ts +++ b/webapp/src/dogma/features/project/ProjectMetadataDto.ts @@ -12,5 +12,11 @@ export interface ProjectMetadataDto { repos: RepositoriesMetadataDto; members: AppMemberDto; appIds: AppIdDto; + allowPublicRepositories?: boolean | null; creation: ProjectCreatorDto; } + +// Public repositories are allowed unless disallowed explicitly. +export function allowsPublicRepositories(metadata: ProjectMetadataDto | undefined): boolean { + return metadata?.allowPublicRepositories !== false; +} diff --git a/webapp/src/dogma/features/project/settings/AllowPublicRepositoriesToggle.tsx b/webapp/src/dogma/features/project/settings/AllowPublicRepositoriesToggle.tsx new file mode 100644 index 000000000..fcf3a57b2 --- /dev/null +++ b/webapp/src/dogma/features/project/settings/AllowPublicRepositoriesToggle.tsx @@ -0,0 +1,62 @@ +import { Box, Flex, FormControl, FormLabel, Spacer, Switch, Text } from '@chakra-ui/react'; +import { useUpdateAllowPublicRepositoriesMutation } from 'dogma/features/api/apiSlice'; +import { newNotification } from 'dogma/features/notification/notificationSlice'; +import ErrorMessageParser from 'dogma/features/services/ErrorMessageParser'; +import { useAppDispatch } from 'dogma/hooks'; +import { WithProjectRole } from 'dogma/features/auth/ProjectRole'; + +export const AllowPublicRepositoriesToggle = ({ + projectName, + allowed, +}: { + projectName: string; + allowed: boolean; +}) => { + const dispatch = useAppDispatch(); + const [updateAllowPublicRepositories, { isLoading }] = useUpdateAllowPublicRepositoriesMutation(); + + const onChange = async (allow: boolean) => { + try { + await updateAllowPublicRepositories({ projectName, allow }).unwrap(); + dispatch( + newNotification( + allow ? 'Public repositories allowed' : 'Public repositories disallowed', + `Successfully updated ${projectName}`, + 'success', + ), + ); + } catch (error) { + dispatch( + newNotification('Failed to update the project setting', ErrorMessageParser.parse(error), 'error'), + ); + } + }; + + return ( + + {() => ( + + + + + Allow public repositories + + + When disabled, repositories in this project cannot be made public. Existing public repositories + must be switched to private first. + + + + onChange(e.target.checked)} + /> + + + )} + + ); +}; diff --git a/webapp/src/dogma/features/project/settings/repositories/RepoMetaList.tsx b/webapp/src/dogma/features/project/settings/repositories/RepoMetaList.tsx index 95374310a..181a2b017 100644 --- a/webapp/src/dogma/features/project/settings/repositories/RepoMetaList.tsx +++ b/webapp/src/dogma/features/project/settings/repositories/RepoMetaList.tsx @@ -3,7 +3,7 @@ import { ColumnDef, createColumnHelper } from '@tanstack/react-table'; import { DateWithTooltip } from 'dogma/common/components/DateWithTooltip'; import { DataTableClientPagination } from 'dogma/common/components/table/DataTableClientPagination'; import { DeleteRepo } from 'dogma/features/repo/DeleteRepo'; -import { RepositoryMetadataDto } from 'dogma/features/repo/RepositoriesMetadataDto'; +import { isPublicRepo, RepositoryMetadataDto } from 'dogma/features/repo/RepositoriesMetadataDto'; import { RestoreRepo } from 'dogma/features/repo/RestoreRepo'; import { useMemo } from 'react'; import { RepoIcon } from 'dogma/common/components/RepoIcon'; @@ -24,6 +24,7 @@ const RepoMetaList = ({ data, projectName }: RepoListProps< projectName={projectName} repoName={info.getValue()} isActive={info.row.original.removal == null} + isPublic={isPublicRepo(info.row.original)} /> ); }, diff --git a/webapp/src/dogma/features/repo/NewRepo.tsx b/webapp/src/dogma/features/repo/NewRepo.tsx index e52e7045c..ebffd92a3 100644 --- a/webapp/src/dogma/features/repo/NewRepo.tsx +++ b/webapp/src/dogma/features/repo/NewRepo.tsx @@ -1,7 +1,10 @@ import { + Box, Button, + Checkbox, FormControl, FormErrorMessage, + FormHelperText, FormLabel, Input, Popover, @@ -13,9 +16,11 @@ import { PopoverHeader, PopoverTrigger, Spacer, + Tooltip, useDisclosure, } from '@chakra-ui/react'; -import { useAddNewRepoMutation } from 'dogma/features/api/apiSlice'; +import { useAddNewRepoMutation, useGetMetadataByProjectNameQuery } from 'dogma/features/api/apiSlice'; +import { allowsPublicRepositories } from 'dogma/features/project/ProjectMetadataDto'; import { newNotification } from 'dogma/features/notification/notificationSlice'; import { useAppDispatch } from 'dogma/hooks'; import Router from 'next/router'; @@ -30,10 +35,14 @@ const ENTITY_NAME_PATTERN = /^[0-9A-Za-z](?:[-+_0-9A-Za-z.]*[0-9A-Za-z])?$/; type FormData = { name: string; + isPublic: boolean; }; export const NewRepo = ({ projectName }: { projectName: string }) => { const [addNewRepo, { isLoading }] = useAddNewRepoMutation(); + const { data: metadata, isLoading: isMetadataLoading } = useGetMetadataByProjectNameQuery(projectName, { + skip: projectName === 'dogma', + }); const { register, handleSubmit, @@ -46,6 +55,7 @@ export const NewRepo = ({ projectName }: { projectName: string }) => { if (projectName === 'dogma') { return null; } + const publicAllowed = allowsPublicRepositories(metadata); const onSubmit = async (data: FormData) => { try { @@ -90,6 +100,26 @@ export const NewRepo = ({ projectName }: { projectName: string }) => { The first/last character must be alphanumeric )} + + + + + Make this repository public + + + + + Readable by everyone who can sign in, including application tokens and certificates with + guest access allowed. + + = { data: Data[]; projectName: string; + metadata?: ProjectMetadataDto; }; -const RepoList = ({ data, projectName }: RepoListProps) => { +const RepoList = ({ data, projectName, metadata }: RepoListProps) => { + const { user, isInAnonymousMode } = useAppSelector((state) => state.auth); const columnHelper = createColumnHelper(); const columns = useMemo( () => [ columnHelper.accessor((row: RepoDto) => row.name, { - cell: (info) => , + cell: (info) => { + const repoName = info.getValue(); + const isAccessible = + isInAnonymousMode || + !metadata || + !user || + findUserRepositoryRole(repoName, user, metadata) !== null; + return ( + + ); + }, header: 'Name', }), columnHelper.accessor((row: RepoDto) => row.creator.name, { @@ -36,7 +58,7 @@ const RepoList = ({ data, projectName }: RepoListProps[]} data={data} />; }; diff --git a/webapp/src/dogma/features/repo/RepoRoleList.tsx b/webapp/src/dogma/features/repo/RepoRoleList.tsx index b8d294ab5..f4e323288 100644 --- a/webapp/src/dogma/features/repo/RepoRoleList.tsx +++ b/webapp/src/dogma/features/repo/RepoRoleList.tsx @@ -1,10 +1,10 @@ -import { Icon, Tag, TagLabel, Wrap, WrapItem } from '@chakra-ui/react'; +import { Badge, Icon, Tag, TagLabel, Wrap, WrapItem } from '@chakra-ui/react'; import { ColumnDef, createColumnHelper } from '@tanstack/react-table'; import { DataTableClientPagination } from 'dogma/common/components/table/DataTableClientPagination'; -import { RepositoryMetadataDto } from 'dogma/features/repo/RepositoriesMetadataDto'; +import { isPublicRepo, RepositoryMetadataDto } from 'dogma/features/repo/RepositoriesMetadataDto'; import { useMemo } from 'react'; import { ChakraLink } from 'dogma/common/components/ChakraLink'; -import { RiGitRepositoryPrivateLine } from 'react-icons/ri'; +import { GoRepo } from 'react-icons/go'; export type RepoRoleListProps = { data: Data[]; @@ -21,7 +21,7 @@ const RepoRoleList = ({ data, projectName }: RepoRoleListPr fontWeight={'semibold'} href={`/app/projects/${projectName}/repos/${info.getValue()}/settings`} > - {info.getValue()} + {info.getValue()} ), header: 'Name', @@ -42,18 +42,17 @@ const RepoRoleList = ({ data, projectName }: RepoRoleListPr enableSorting: false, }), columnHelper.accessor((row: RepositoryMetadataDto) => row.roles.projects.guest, { - cell: (info) => ( - - {info.getValue() !== null && ( - - - {info.getValue()} - - - )} - - ), - header: 'Guest', + cell: (info) => + isPublicRepo(info.row.original) ? ( + + Public + + ) : ( + + Private + + ), + header: 'Visibility', enableSorting: false, }), ], diff --git a/webapp/src/dogma/features/repo/RepositoriesMetadataDto.ts b/webapp/src/dogma/features/repo/RepositoriesMetadataDto.ts index 3087a9dda..898365137 100644 --- a/webapp/src/dogma/features/repo/RepositoriesMetadataDto.ts +++ b/webapp/src/dogma/features/repo/RepositoriesMetadataDto.ts @@ -23,6 +23,11 @@ export interface ProjectRolesDto { guest: 'READ' | null; } +// A repository is public when its guest role is READ. +export function isPublicRepo(repo: RepositoryMetadataDto | undefined): boolean { + return repo?.roles?.projects?.guest === 'READ'; +} + export interface UserOrAppIdentityRepositoryRoleDto { [key: string]: RepositoryRole; } diff --git a/webapp/src/dogma/features/repo/roles/ConfirmUpdateRepositoryProjectRoles.tsx b/webapp/src/dogma/features/repo/roles/ConfirmUpdateRepositoryProjectRoles.tsx index e184797f3..8f97311a7 100644 --- a/webapp/src/dogma/features/repo/roles/ConfirmUpdateRepositoryProjectRoles.tsx +++ b/webapp/src/dogma/features/repo/roles/ConfirmUpdateRepositoryProjectRoles.tsx @@ -1,4 +1,8 @@ import { + Alert, + AlertDescription, + AlertIcon, + Box, Button, HStack, Modal, @@ -8,6 +12,8 @@ import { ModalFooter, ModalHeader, ModalOverlay, + UnorderedList, + ListItem, useDisclosure, } from '@chakra-ui/react'; import { SerializedError } from '@reduxjs/toolkit'; @@ -19,18 +25,22 @@ import ErrorMessageParser from 'dogma/features/services/ErrorMessageParser'; import { useAppDispatch } from 'dogma/hooks'; import { UseFormHandleSubmit, UseFormReset } from 'react-hook-form'; +export type VisibilityChange = 'toPublic' | 'toPrivate' | null; + export const ConfirmUpdateRepositoryProjectRoles = ({ projectName, repoName, handleSubmit, isDirty, reset, + visibilityChange, }: { projectName: string; repoName: string; handleSubmit: UseFormHandleSubmit; isDirty: boolean; reset: UseFormReset; + visibilityChange: VisibilityChange; }) => { const { isOpen, onOpen, onClose } = useDisclosure(); const dispatch = useAppDispatch(); @@ -68,9 +78,47 @@ export const ConfirmUpdateRepositoryProjectRoles = ({ - Are you sure? + + {visibilityChange === 'toPublic' + ? 'Make this repository public?' + : visibilityChange === 'toPrivate' + ? 'Make this repository private?' + : 'Are you sure?'} + - Update project roles for the repository {repoName}? + + + Update project roles for the repository{' '} + + {projectName}/{repoName} + + ? + + {visibilityChange === 'toPublic' && ( + + + + Everyone who can sign in — and all application tokens and certificates with guest access + allowed — will be able to read: + + All files and commit history, including diffs + The repository over git clone + Repository variables + + Write access is never granted to guests. + + + )} + {visibilityChange === 'toPrivate' && ( + + + + Guest read access will be revoked. Only members and users or app identities granted a role + will be able to read this repository. + + + )} + diff --git a/webapp/src/dogma/features/repo/roles/ProjectRolesForm.tsx b/webapp/src/dogma/features/repo/roles/ProjectRolesForm.tsx index 3e93a4d3d..8141232dd 100644 --- a/webapp/src/dogma/features/repo/roles/ProjectRolesForm.tsx +++ b/webapp/src/dogma/features/repo/roles/ProjectRolesForm.tsx @@ -1,6 +1,23 @@ -import { Box, Flex, FormControl, FormLabel, HStack, Radio, RadioGroup, Spacer, VStack } from '@chakra-ui/react'; +import { + Badge, + Box, + Flex, + FormControl, + FormLabel, + HStack, + Radio, + RadioGroup, + Spacer, + Stack, + Text, + Tooltip, + VStack, +} from '@chakra-ui/react'; import { RepositoryRole } from 'dogma/features/auth/RepositoryRole'; -import { ConfirmUpdateRepositoryProjectRoles } from 'dogma/features/repo/roles/ConfirmUpdateRepositoryProjectRoles'; +import { + ConfirmUpdateRepositoryProjectRoles, + VisibilityChange, +} from 'dogma/features/repo/roles/ConfirmUpdateRepositoryProjectRoles'; import { ProjectRolesDto } from 'dogma/features/repo/RepositoriesMetadataDto'; import { Controller, useForm } from 'react-hook-form'; @@ -12,19 +29,29 @@ export const ProjectRolesForm = ({ projectName, repoName, projectRoles, + allowPublicRepositories, }: { projectName: string; repoName: string; projectRoles: ProjectRolesDto; + allowPublicRepositories: boolean; }) => { const { handleSubmit, control, reset, + watch, formState: { isDirty }, } = useForm({ - defaultValues: projectRoles, + values: projectRoles, + resetOptions: { keepDirtyValues: true }, }); + const isPublic = projectRoles?.guest === 'READ'; + const selectedPublic = watch('guest') === 'READ'; + let visibilityChange: VisibilityChange = null; + if (selectedPublic !== isPublic) { + visibilityChange = selectedPublic ? 'toPublic' : 'toPrivate'; + } return (

@@ -50,16 +77,46 @@ export const ProjectRolesForm = ({ - Guest + + Visibility{' '} + + {isPublic ? 'Public' : 'Private'} + + ( - - Read - Forbidden - + + + Private + + Only members and users or app identities granted a role can access this repository. + + + + + + Public + + Everyone who can sign in — and all application tokens and certificates with guest + access allowed — can read this repository. Write access is never granted to + guests. + + + + + )} /> @@ -74,6 +131,7 @@ export const ProjectRolesForm = ({ handleSubmit={handleSubmit} isDirty={isDirty} reset={reset} + visibilityChange={visibilityChange} /> diff --git a/webapp/src/pages/app/projects/[projectName]/index.tsx b/webapp/src/pages/app/projects/[projectName]/index.tsx index 589d591cb..f0a9d8a8f 100644 --- a/webapp/src/pages/app/projects/[projectName]/index.tsx +++ b/webapp/src/pages/app/projects/[projectName]/index.tsx @@ -1,7 +1,7 @@ import { Box, Flex, Heading, HStack, Spacer, Tab, TabList, TabPanel, TabPanels, Tabs } from '@chakra-ui/react'; import { Breadcrumbs } from 'dogma/common/components/Breadcrumbs'; import { Deferred } from 'dogma/common/components/Deferred'; -import { useGetReposQuery } from 'dogma/features/api/apiSlice'; +import { useGetMetadataByProjectNameQuery, useGetReposQuery } from 'dogma/features/api/apiSlice'; import { NewRepo } from 'dogma/features/repo/NewRepo'; import RepoList from 'dogma/features/repo/RepoList'; import { useRouter } from 'next/router'; @@ -18,6 +18,9 @@ const ProjectDetailPage = () => { } = useGetReposQuery(projectName, { refetchOnMountOrArgChange: true, }); + const { data: metadata } = useGetMetadataByProjectNameQuery(projectName, { + refetchOnFocus: true, + }); return ( {() => ( @@ -46,7 +49,7 @@ const ProjectDetailPage = () => { - + diff --git a/webapp/src/pages/app/projects/[projectName]/repos/[repoName]/settings/index.tsx b/webapp/src/pages/app/projects/[projectName]/repos/[repoName]/settings/index.tsx index c0d5990ed..4312fe382 100644 --- a/webapp/src/pages/app/projects/[projectName]/repos/[repoName]/settings/index.tsx +++ b/webapp/src/pages/app/projects/[projectName]/repos/[repoName]/settings/index.tsx @@ -17,6 +17,7 @@ import { useRouter } from 'next/router'; import RepositorySettingsView from 'dogma/features/repo/settings/RepositorySettingsView'; import { ProjectRolesForm } from 'dogma/features/repo/roles/ProjectRolesForm'; +import { allowsPublicRepositories } from 'dogma/features/project/ProjectMetadataDto'; const RepositorySettingsPage = () => { const router = useRouter(); @@ -31,6 +32,7 @@ const RepositorySettingsPage = () => { projectName={projectName} repoName={repoName} projectRoles={metadata?.repos[repoName]?.roles.projects} + allowPublicRepositories={allowsPublicRepositories(metadata)} /> )} diff --git a/webapp/src/pages/app/projects/[projectName]/settings/index.tsx b/webapp/src/pages/app/projects/[projectName]/settings/index.tsx index 407cdbbd1..d07c50fcc 100644 --- a/webapp/src/pages/app/projects/[projectName]/settings/index.tsx +++ b/webapp/src/pages/app/projects/[projectName]/settings/index.tsx @@ -19,6 +19,8 @@ import { Flex, Spacer } from '@chakra-ui/react'; import { NewRepo } from 'dogma/features/repo/NewRepo'; import RepoMetaList from 'dogma/features/project/settings/repositories/RepoMetaList'; import ProjectSettingsView from 'dogma/features/project/settings/ProjectSettingsView'; +import { AllowPublicRepositoriesToggle } from 'dogma/features/project/settings/AllowPublicRepositoriesToggle'; +import { allowsPublicRepositories } from 'dogma/features/project/ProjectMetadataDto'; const ProjectSettingsPage = () => { const router = useRouter(); @@ -28,6 +30,10 @@ const ProjectSettingsPage = () => { {(metadata) => ( <> + diff --git a/webapp/src/pages/app/settings/app-identities/index.tsx b/webapp/src/pages/app/settings/app-identities/index.tsx index f6667b653..46c1b761f 100644 --- a/webapp/src/pages/app/settings/app-identities/index.tsx +++ b/webapp/src/pages/app/settings/app-identities/index.tsx @@ -49,6 +49,15 @@ const AppIdentityPage = () => { ), header: 'Type', }), + // A system admin identity always has guest access regardless of the stored flag. + columnHelper.accessor((row: AppIdentityDto) => row.systemAdmin || row.allowGuestAccess, { + cell: (info) => ( + + {info.getValue() ? 'Guest access' : 'Registered only'} + + ), + header: 'Scope', + }), ...(systemAdmin ? [ columnHelper.accessor((row: AppIdentityDto) => row.systemAdmin, { diff --git a/webapp/tests/dogma/feature/repo/RepoList.test.tsx b/webapp/tests/dogma/feature/repo/RepoList.test.tsx index 28ffadd15..0aff1db48 100644 --- a/webapp/tests/dogma/feature/repo/RepoList.test.tsx +++ b/webapp/tests/dogma/feature/repo/RepoList.test.tsx @@ -1,4 +1,4 @@ -import { render } from '@testing-library/react'; +import { renderWithProviders } from 'dogma/util/test-utils'; import { RepoDto } from 'dogma/features/repo/RepoDto'; import RepoList, { RepoListProps } from 'dogma/features/repo/RepoList'; @@ -45,7 +45,7 @@ describe('RepoList', () => { }); it('renders the repo names', () => { - const { getByText } = render(); + const { getByText } = renderWithProviders(); let name; expectedProps.data.forEach((repo: RepoDto) => { name = getByText(repo.name); @@ -54,12 +54,12 @@ describe('RepoList', () => { }); it('renders a table with a row for each repo', () => { - const { container } = render(); + const { container } = renderWithProviders(); expect(container.querySelector('tbody').children.length).toBe(3); }); it('has `${projectName}/repos/${repoName}/files/head{fileName}/tree/head` on the view icon', () => { - const { container } = render(); + const { container } = renderWithProviders(); const actionCell = container.querySelector('tbody').firstChild.firstChild.lastChild; const firstRepoName = 'meta'; expect(actionCell).toHaveAttribute( @@ -69,7 +69,7 @@ describe('RepoList', () => { }); it('has `${projectName}/repos/${repoName}/files/head{fileName}/tree/head` on the file path cell', () => { - const { container } = render(); + const { container } = renderWithProviders(); const firstCell = container.querySelector('tbody').firstChild.firstChild.firstChild; const firstRepoName = 'meta'; expect(firstCell).toHaveAttribute( @@ -77,4 +77,57 @@ describe('RepoList', () => { `/app/projects/${expectedProps.projectName}/repos/${firstRepoName}/tree/head`, ); }); + + describe('with project metadata', () => { + // The signed-in user is not a member; repo1 is public, repo2 is private without any grant. + const metadata = { + name: 'ProjectAlpha', + repos: { + repo1: { + name: 'repo1', + roles: { projects: { member: 'WRITE', guest: 'READ' }, users: {}, appIds: {} }, + creation: { user: 'dummy', timestamp: '2022-11-23T03:16:17.880Z' }, + }, + repo2: { + name: 'repo2', + roles: { projects: { member: 'WRITE', guest: null }, users: {}, appIds: {} }, + creation: { user: 'dummy', timestamp: '2022-11-28T03:01:47.262Z' }, + }, + }, + members: {}, + appIds: {}, + creation: { user: 'dummy', timestamp: '2022-11-23T03:13:49.581Z' }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any; + const preloadedState = { + auth: { + isInAnonymousMode: false, + csrfToken: null as string, + isLoading: false, + user: { + login: 'guest', + name: 'guest', + email: 'guest@localhost.localdomain', + roles: [] as string[], + systemAdmin: false, + }, + }, + }; + + it('marks a public repo with a badge and keeps it accessible', () => { + const { getByText } = renderWithProviders(, { + preloadedState, + }); + expect(getByText('Public')).toBeVisible(); + expect(getByText('repo1').closest('a')).not.toBeNull(); + }); + + it('dims an inaccessible private repo without a link', () => { + const { getByText } = renderWithProviders(, { + preloadedState, + }); + const repo2 = getByText('repo2'); + expect(repo2.closest('a')).toBeNull(); + }); + }); }); diff --git a/webapp/tests/dogma/feature/repo/roles/ProjectRolesForm.test.tsx b/webapp/tests/dogma/feature/repo/roles/ProjectRolesForm.test.tsx new file mode 100644 index 000000000..da04a4195 --- /dev/null +++ b/webapp/tests/dogma/feature/repo/roles/ProjectRolesForm.test.tsx @@ -0,0 +1,61 @@ +import { renderWithProviders } from 'dogma/util/test-utils'; +import { ProjectRolesForm } from 'dogma/features/repo/roles/ProjectRolesForm'; + +describe('ProjectRolesForm', () => { + it('shows a private repository with Private and Public choices', () => { + const { getByText, getAllByText, getByRole } = renderWithProviders( + , + ); + expect(getByText('Visibility')).toBeVisible(); + // Shown as both the current-state badge and the radio label. + expect(getAllByText('Private').length).toBeGreaterThan(1); + const publicRadio = getByRole('radio', { name: /Public/ }); + expect(publicRadio).not.toBeChecked(); + expect(publicRadio).toBeEnabled(); + }); + + it('marks a guest READ repository as Public', () => { + const { getByRole } = renderWithProviders( + , + ); + expect(getByRole('radio', { name: /Public/ })).toBeChecked(); + }); + + it('disables the Public option when the project disallows public repositories', () => { + const { getByRole } = renderWithProviders( + , + ); + expect(getByRole('radio', { name: /Public/ })).toBeDisabled(); + }); + + it('keeps the Public option enabled for a legacy public repository in a disallowing project', () => { + // The owner must still be able to switch such a repository to private. + const { getByRole } = renderWithProviders( + , + ); + const publicRadio = getByRole('radio', { name: /Public/ }); + expect(publicRadio).toBeChecked(); + expect(publicRadio).toBeEnabled(); + expect(getByRole('radio', { name: /Private/ })).toBeEnabled(); + }); +});