Skip to content

Commit

Permalink
Add verboseResponses flag and write tests
Browse files Browse the repository at this point in the history
  • Loading branch information
ikhoon committed Oct 7, 2024
1 parent a0530ee commit 19d095a
Show file tree
Hide file tree
Showing 16 changed files with 225 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@

import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.centraldogma.common.Change;
import com.linecorp.centraldogma.common.MirrorException;
import com.linecorp.centraldogma.internal.Jackson;
import com.linecorp.centraldogma.server.CentralDogmaBuilder;
import com.linecorp.centraldogma.common.MirrorException;
import com.linecorp.centraldogma.server.MirroringService;
import com.linecorp.centraldogma.server.internal.mirror.MirrorState;
import com.linecorp.centraldogma.server.mirror.MirrorDirection;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,41 @@

package com.linecorp.centraldogma.it.mirror.git;

import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.PASSWORD;
import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.USERNAME;
import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.getAccessToken;
import static org.assertj.core.api.Assertions.assertThat;

import java.nio.charset.StandardCharsets;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import com.google.common.io.Resources;

import com.linecorp.armeria.client.BlockingWebClient;
import com.linecorp.armeria.client.WebClient;
import com.linecorp.armeria.common.HttpStatus;
import com.linecorp.armeria.common.ResponseEntity;
import com.linecorp.armeria.common.auth.AuthToken;
import com.linecorp.centraldogma.client.CentralDogma;
import com.linecorp.centraldogma.internal.api.v1.MirrorDto;
import com.linecorp.centraldogma.internal.api.v1.PushResultDto;
import com.linecorp.centraldogma.server.CentralDogmaBuilder;
import com.linecorp.centraldogma.server.internal.credential.PublicKeyCredential;
import com.linecorp.centraldogma.server.mirror.MirrorResult;
import com.linecorp.centraldogma.server.mirror.MirrorStatus;
import com.linecorp.centraldogma.testing.internal.auth.TestAuthProviderFactory;
import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension;

class MirrorRunnerTest {

private static final String FOO_PROJ = "foo";
private static final String BAR_REPO = "bar";
private static final String PRIVATE_KEY_FILE = "ecdsa_256.openssh";
private static final String TEST_MIRROR_ID = "test-mirror";

@RegisterExtension
static final CentralDogmaExtension dogma = new CentralDogmaExtension() {

Expand All @@ -38,10 +62,98 @@ protected void configure(CentralDogmaBuilder builder) {

@Override
protected void scaffold(CentralDogma client) {
client.createProject("foo").join();
client.createRepository("foo", "bar").join();
client.createProject(FOO_PROJ).join();
client.createRepository(FOO_PROJ, BAR_REPO).join();
}
};

// TODO(ikhoon): Implement integration tests
private BlockingWebClient adminClient;

@BeforeEach
void setUp() throws Exception {
final String adminToken = getAccessToken(dogma.httpClient(), USERNAME, PASSWORD);
adminClient = WebClient.builder(dogma.httpClient().uri())
.auth(AuthToken.ofOAuth2(adminToken))
.build()
.blocking();
}

@Test
void triggerMirroring() throws Exception {
final PublicKeyCredential credential = getCredential();
ResponseEntity<PushResultDto> response =
adminClient.prepare()
.post("/api/v1/projects/{proj}/credentials")
.pathParam("proj", FOO_PROJ)
.contentJson(credential)
.asJson(PushResultDto.class)
.execute();
assertThat(response.status()).isEqualTo(HttpStatus.CREATED);

final MirrorDto newMirror = newMirror();
response = adminClient.prepare()
.post("/api/v1/projects/{proj}/mirrors")
.pathParam("proj", FOO_PROJ)
.contentJson(newMirror)
.asJson(PushResultDto.class)
.execute();
assertThat(response.status()).isEqualTo(HttpStatus.CREATED);

for (int i = 0; i < 3; i++) {
final ResponseEntity<MirrorResult> mirrorResponse =
adminClient.prepare()
.post("/api/v1/projects/{proj}/mirrors/{mirrorId}/run")
.pathParam("proj", FOO_PROJ)
.pathParam("mirrorId", TEST_MIRROR_ID)
.asJson(MirrorResult.class)
.execute();

assertThat(mirrorResponse.status()).isEqualTo(HttpStatus.OK);
if (i == 0) {
assertThat(mirrorResponse.content().mirrorStatus()).isEqualTo(MirrorStatus.SUCCESS);
assertThat(mirrorResponse.content().description())
.contains("'git+ssh://github.com/line/centraldogma-authtest.git#main' to " +
"the repository 'bar', revision: 2");
} else {
assertThat(mirrorResponse.content().mirrorStatus()).isEqualTo(MirrorStatus.UP_TO_DATE);
assertThat(mirrorResponse.content().description())
.contains("Repository 'foo/bar' already at");
}
}
}

private static MirrorDto newMirror() {
return new MirrorDto(TEST_MIRROR_ID,
true,
FOO_PROJ,
null,
"REMOTE_TO_LOCAL",
BAR_REPO,
"/",
"git+ssh",
"github.com/line/centraldogma-authtest.git",
"/",
"main",
null,
PRIVATE_KEY_FILE);
}

private static PublicKeyCredential getCredential() throws Exception {
final String publicKeyFile = "ecdsa_256.openssh.pub";

final byte[] privateKeyBytes =
Resources.toByteArray(GitMirrorAuthTest.class.getResource(PRIVATE_KEY_FILE));
final byte[] publicKeyBytes =
Resources.toByteArray(GitMirrorAuthTest.class.getResource(publicKeyFile));
final String privateKey = new String(privateKeyBytes, StandardCharsets.UTF_8).trim();
final String publicKey = new String(publicKeyBytes, StandardCharsets.UTF_8).trim();

return new PublicKeyCredential(
PRIVATE_KEY_FILE,
true,
"git",
publicKey,
privateKey,
null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.FetchResult;
import org.eclipse.jgit.transport.PushResult;
import org.eclipse.jgit.transport.RefSpec;
import org.eclipse.jgit.transport.TagOpt;
import org.eclipse.jgit.transport.URIish;
Expand Down Expand Up @@ -236,14 +235,11 @@ dirCache, new InsertText(mirrorStatePath.substring(1), // Strip the leading '/'.
updateRef(gitRepository, revWalk, headBranchRefName, nextCommitId);
}

final Iterable<PushResult> pushResults =
git.push()
.setRefSpecs(new RefSpec(headBranchRefName))
.setAtomic(true)
.setTimeout(GIT_TIMEOUT_SECS)
.call();
final PushResult pushResult = pushResults.iterator().next();
// TODO(ikhoon): Append remove ref to description;
git.push()
.setRefSpecs(new RefSpec(headBranchRefName))
.setAtomic(true)
.setTimeout(GIT_TIMEOUT_SECS)
.call();
return newMirrorResult(MirrorStatus.SUCCESS, description);
}

Expand Down Expand Up @@ -357,7 +353,7 @@ MirrorResult mirrorRemoteToLocal(
final CommitResult commitResult = executor.execute(Command.push(
MIRROR_AUTHOR, localRepo().parent().name(), localRepo().name(),
Revision.HEAD, summary, detail, Markup.PLAINTEXT, changes.values())).join();
final String description = summary + ", Revision: " + commitResult.revision();
final String description = summary + ", revision: " + commitResult.revision().text();
return newMirrorResult(MirrorStatus.SUCCESS, description);
} catch (CompletionException e) {
if (e.getCause() instanceof RedundantChangeException) {
Expand Down Expand Up @@ -389,11 +385,7 @@ private boolean needsFetch(Ref headBranchRef, String mirrorStatePath, Revision l
}

final ObjectId headCommitId = headBranchRef.getObjectId();
if (headCommitId.name().equals(localSourceRevision)) {
// TODO(ikhoon): Revert
return true;
}
return true;
return !headCommitId.name().equals(localSourceRevision);
}

private Ref getHeadBranchRef(GitWithAuth git) throws GitAPIException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -699,8 +699,9 @@ private AuthProvider createAuthProvider(
checkState(sessionManager != null, "SessionManager is null");
final AuthProviderParameters parameters = new AuthProviderParameters(
// Find application first, then find the session token.
new ApplicationTokenAuthorizer(mds::findTokenBySecret).orElse(
new SessionTokenAuthorizer(sessionManager, authCfg.administrators())),
new ApplicationTokenAuthorizer(mds::findTokenBySecret, cfg.verboseResponses())
.orElse(new SessionTokenAuthorizer(sessionManager, authCfg.administrators(),
cfg.verboseResponses())),
cfg,
sessionManager::generateSessionId,
// Propagate login and logout events to the other replicas.
Expand Down Expand Up @@ -764,9 +765,9 @@ private Function<? super HttpService, AuthService> authService(
assert authCfg != null : "authCfg";
assert sessionManager != null : "sessionManager";
final Authorizer<HttpRequest> tokenAuthorizer =
new ApplicationTokenAuthorizer(mds::findTokenBySecret)
new ApplicationTokenAuthorizer(mds::findTokenBySecret, cfg.verboseResponses())
.orElse(new SessionTokenAuthorizer(sessionManager,
authCfg.administrators()));
authCfg.administrators(), cfg.verboseResponses()));
return AuthService.builder()
.add(tokenAuthorizer)
.onFailure(new CentralDogmaAuthFailureHandler())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ public final class CentralDogmaBuilder {
private final List<PluginConfig> pluginConfigs = new ArrayList<>();
@Nullable
private ManagementConfig managementConfig;
private boolean verboseResponses;

/**
* Creates a new builder with the specified data directory.
Expand Down Expand Up @@ -540,6 +541,21 @@ public CentralDogmaBuilder management(ManagementConfig managementConfig) {
return this;
}

/**
* Sets whether to enable the verbose response mode. When enabled, the error responses will contain
* its full stack trace, which may be useful for debugging while potentially
* insecure.
*
* <p>When disabled, the error responses will not expose such server-side details to the normal users.
* Only administrators can see the verbose responses.
*
* <p>This option is disabled by default.
*/
public CentralDogmaBuilder verboseResponses(boolean verboseResponses) {
this.verboseResponses = verboseResponses;
return this;
}

/**
* Returns a newly-created {@link CentralDogma} server.
*/
Expand Down Expand Up @@ -573,6 +589,6 @@ private CentralDogmaConfig buildConfig() {
maxRemovedRepositoryAgeMillis, gracefulShutdownTimeout,
webAppEnabled, webAppTitle,replicationConfig,
null, accessLogFormat, authCfg, quotaConfig,
corsConfig, pluginConfigs, managementConfig);
corsConfig, pluginConfigs, managementConfig, verboseResponses);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,8 @@ public static CentralDogmaConfig load(String json) throws JsonMappingException,
@Nullable
private final ManagementConfig managementConfig;

private final boolean verboseResponses;

CentralDogmaConfig(
@JsonProperty(value = "dataDir", required = true) File dataDir,
@JsonProperty(value = "ports", required = true)
Expand All @@ -295,7 +297,8 @@ public static CentralDogmaConfig load(String json) throws JsonMappingException,
@JsonProperty("writeQuotaPerRepository") @Nullable QuotaConfig writeQuotaPerRepository,
@JsonProperty("cors") @Nullable CorsConfig corsConfig,
@JsonProperty("pluginConfigs") @Nullable List<PluginConfig> pluginConfigs,
@JsonProperty("management") @Nullable ManagementConfig managementConfig) {
@JsonProperty("management") @Nullable ManagementConfig managementConfig,
@JsonProperty("verboseResponses") @Nullable Boolean verboseResponses) {

this.dataDir = requireNonNull(dataDir, "dataDir");
this.ports = ImmutableList.copyOf(requireNonNull(ports, "ports"));
Expand Down Expand Up @@ -344,6 +347,7 @@ public static CentralDogmaConfig load(String json) throws JsonMappingException,
pluginConfigMap = this.pluginConfigs.stream().collect(
toImmutableMap(PluginConfig::getClass, Function.identity()));
this.managementConfig = managementConfig;
this.verboseResponses = firstNonNull(verboseResponses, false);
}

/**
Expand Down Expand Up @@ -582,6 +586,21 @@ public ManagementConfig managementConfig() {
return managementConfig;
}

/**
* Returns whether the verbose response mode is enabled. When enabled, the error responses will contain
* its full stack trace, which may be useful for debugging while potentially
* insecure.
*
* <p>When disabled, the error responses will not expose such server-side details to the normal users.
* Only administrators can see the verbose responses.
*
* <p>This option is disabled by default.
*/
@JsonProperty("verboseResponses")
public boolean verboseResponses() {
return verboseResponses;
}

@Override
public String toString() {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.linecorp.armeria.server.auth.Authorizer;
import com.linecorp.armeria.server.thrift.THttpService;
import com.linecorp.centraldogma.internal.CsrfToken;
import com.linecorp.centraldogma.server.internal.api.HttpApiUtil;
import com.linecorp.centraldogma.server.metadata.User;

/**
Expand All @@ -39,6 +40,7 @@ public CompletionStage<Boolean> authorize(ServiceRequestContext ctx, HttpRequest
final OAuth2Token token = AuthTokenExtractors.oAuth2().apply(data.headers());
if (token != null && CsrfToken.ANONYMOUS.equals(token.accessToken())) {
AuthUtil.setCurrentUser(ctx, User.ADMIN);
HttpApiUtil.setVerboseResponses(ctx, User.ADMIN, true);
return CompletableFuture.completedFuture(true);
} else {
return CompletableFuture.completedFuture(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.linecorp.armeria.server.auth.AuthTokenExtractors;
import com.linecorp.armeria.server.auth.Authorizer;
import com.linecorp.centraldogma.server.auth.SessionManager;
import com.linecorp.centraldogma.server.internal.api.HttpApiUtil;
import com.linecorp.centraldogma.server.metadata.User;

/**
Expand All @@ -41,10 +42,13 @@ public class SessionTokenAuthorizer implements Authorizer<HttpRequest> {

private final SessionManager sessionManager;
private final Set<String> administrators;
private final boolean verboseResponses;

public SessionTokenAuthorizer(SessionManager sessionManager, Set<String> administrators) {
public SessionTokenAuthorizer(SessionManager sessionManager, Set<String> administrators,
boolean verboseResponses) {
this.sessionManager = requireNonNull(sessionManager, "sessionManager");
this.administrators = requireNonNull(administrators, "administrators");
this.verboseResponses = verboseResponses;
}

@Override
Expand All @@ -64,6 +68,7 @@ public CompletionStage<Boolean> authorize(ServiceRequestContext ctx, HttpRequest
final User user = new User(username, roles);
ctx.logBuilder().authenticatedUser("user/" + username);
AuthUtil.setCurrentUser(ctx, user);
HttpApiUtil.setVerboseResponses(ctx, user, verboseResponses);
return true;
});
}
Expand Down
Loading

0 comments on commit 19d095a

Please sign in to comment.