Skip to content

Commit

Permalink
Rename Admin Role to SystemAdmin for Clarity (#1054)
Browse files Browse the repository at this point in the history 
Motivation:
The term "Admin" does not clearly convey its purpose as a system-wide administrator role, as discussed in [issue #1048](#1048). Renaming this role improves clarity for users and developers.

Modifications:
- Renamed all "Admin" to "SystemAdmin" across the codebase.
- Updated `MetadataService.updateTokenLevel()` to avoid directly accessing the `admin` property.

Result:
- The role name "Admin" has been replaced with "SystemAdmin".
- (Breaking)
  - `authentication.administrators` in dogma.json is now `authentication.systemAdministrators`.
- (Deprecation)
  - Use `systemAdmin` property instead of `admin` when creating a token via REST API:
    ```json
    {"appId": "foo", "isSystemAdmin": true, ...}
     ```
  - Use `SYSTEMADMIN` instead of `admin` when changing the token level:
    ```json
    {"level":"SYSTEMADMIN"}
    ```
  • Loading branch information
@minwoox
minwoox authored Dec 10, 2024
1 parent 810d02f commit b0d7708
Show file tree
Hide file tree
Showing 64 changed files with 511 additions and 379 deletions.
2 changes: 1 addition & 1 deletion ...eria-xds/src/test/java/com/linecorp/centraldogma/client/armeria/xds/AuthUpstreamTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class AuthUpstreamTest {

@Override
protected void configure(CentralDogmaBuilder builder) {
builder.administrators(TestAuthMessageUtil.USERNAME);
builder.systemAdministrators(TestAuthMessageUtil.USERNAME);
builder.authProviderFactory(new TestAuthProviderFactory());
}

Expand Down
2 changes: 1 addition & 1 deletion ...or/src/test/java/com/linecorp/centraldogma/it/mirror/git/LegacyGitMirrorSettingsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class LegacyGitMirrorSettingsTest {
@Override
protected void configure(CentralDogmaBuilder builder) {
builder.authProviderFactory(new TestAuthProviderFactory());
builder.administrators(TestAuthMessageUtil.USERNAME);
builder.systemAdministrators(TestAuthMessageUtil.USERNAME);
}
};

Expand Down
48 changes: 24 additions & 24 deletions it/mirror/src/test/java/com/linecorp/centraldogma/it/mirror/git/MirrorRunnerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class MirrorRunnerTest {
@Override
protected void configure(CentralDogmaBuilder builder) {
builder.authProviderFactory(new TestAuthProviderFactory());
builder.administrators(USERNAME);
builder.systemAdministrators(USERNAME);
}

@Override
Expand All @@ -82,47 +82,47 @@ protected void scaffold(CentralDogma client) {
}
};

private BlockingWebClient adminClient;
private BlockingWebClient systemAdminClient;

@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();
systemAdminClient = WebClient.builder(dogma.httpClient().uri())
.auth(AuthToken.ofOAuth2(adminToken))
.build()
.blocking();
TestMirrorRunnerListener.reset();
}

@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();
systemAdminClient.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();
response = systemAdminClient.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();
systemAdminClient.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) {
Expand Down
2 changes: 1 addition & 1 deletion it/mirror/src/test/java/com/linecorp/centraldogma/it/mirror/git/ZoneAwareMirrorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class ZoneAwareMirrorTest {
@Override
protected void configureEach(int serverId, CentralDogmaBuilder builder) {
builder.authProviderFactory(new TestAuthProviderFactory());
builder.administrators(USERNAME);
builder.systemAdministrators(USERNAME);
builder.zone(new ZoneConfig(ZONES.get(serverId - 1), ZONES));
builder.pluginConfigs(new MirroringServicePluginConfig(true, null, null, null, true));
}
Expand Down
12 changes: 6 additions & 6 deletions it/server/src/test/java/com/linecorp/centraldogma/it/NonRandomTokenTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class NonRandomTokenTest {
static final CentralDogmaExtension dogma = new CentralDogmaExtension() {
@Override
protected void configure(CentralDogmaBuilder builder) {
builder.administrators(TestAuthMessageUtil.USERNAME);
builder.systemAdministrators(TestAuthMessageUtil.USERNAME);
builder.authProviderFactory(new TestAuthProviderFactory());
}
};
Expand All @@ -56,17 +56,17 @@ void createNonRandomToken() throws Exception {
assertThat(response.status()).isEqualTo(HttpStatus.OK);
final String sessionId = Jackson.readValue(response.content().array(), AccessToken.class)
.accessToken();
final WebClient adminClient = WebClient.builder(client.uri())
.auth(AuthToken.ofOAuth2(sessionId)).build();
final WebClient systemAdminClient = WebClient.builder(client.uri())
.auth(AuthToken.ofOAuth2(sessionId)).build();

final HttpRequest request = HttpRequest.builder()
.post("/api/v1/tokens")
.content(MediaType.FORM_DATA,
"secret=appToken-secret&isAdmin=true&appId=foo")
"secret=appToken-secret&isSystemAdmin=true&appId=foo")
.build();
AggregatedHttpResponse res = adminClient.execute(request).aggregate().join();
AggregatedHttpResponse res = systemAdminClient.execute(request).aggregate().join();
assertThat(res.status()).isEqualTo(HttpStatus.CREATED);
res = adminClient.get("/api/v1/tokens").aggregate().join();
res = systemAdminClient.get("/api/v1/tokens").aggregate().join();
assertThat(res.contentUtf8()).contains("\"secret\":\"appToken-secret\"");
}
}
2 changes: 1 addition & 1 deletion it/server/src/test/java/com/linecorp/centraldogma/it/ReplicationWriteQuotaTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ private static CompletableFuture<Void> startNewReplica(
int port, int serverId, Map<Integer, ZooKeeperServerConfig> servers) throws IOException {
return new CentralDogmaBuilder(tempDir.newFolder().toFile())
.port(port, SessionProtocol.HTTP)
.administrators(TestAuthMessageUtil.USERNAME)
.systemAdministrators(TestAuthMessageUtil.USERNAME)
.authProviderFactory(factory)
.pluginConfigs(new MirroringServicePluginConfig(false))
.writeQuotaPerRepository(5, 1)
Expand Down
2 changes: 1 addition & 1 deletion it/server/src/test/java/com/linecorp/centraldogma/it/StandaloneWriteQuotaTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class StandaloneWriteQuotaTest extends WriteQuotaTestBase {
static final CentralDogmaExtension dogma = new CentralDogmaExtension() {
@Override
protected void configure(CentralDogmaBuilder builder) {
builder.administrators(TestAuthMessageUtil.USERNAME);
builder.systemAdministrators(TestAuthMessageUtil.USERNAME);
builder.authProviderFactory(new TestAuthProviderFactory());
// Default write quota
builder.writeQuotaPerRepository(5, 1);
Expand Down
8 changes: 5 additions & 3 deletions it/server/src/test/java/com/linecorp/centraldogma/it/WriteQuotaTestBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,16 @@ void updateWriteQuota() throws Exception {
assertThat(CompletableFutures.allAsList(futures4).join()).hasSize(8);
}

private static QuotaConfig updateWriteQuota(WebClient adminClient, String repoName, QuotaConfig writeQuota)
private static QuotaConfig updateWriteQuota(
WebClient systemAdminClient, String repoName, QuotaConfig writeQuota)
throws JsonProcessingException {
final String updatePath = "/api/v1/metadata/test_prj/repos/" + repoName + "/quota/write";
final String content = mapper.writeValueAsString(writeQuota);
final HttpRequest req = HttpRequest.of(HttpMethod.PATCH, updatePath, MediaType.JSON_PATCH, content);
assertThat(adminClient.execute(req).aggregate().join().status()).isEqualTo(HttpStatus.OK);
assertThat(systemAdminClient.execute(req).aggregate().join().status()).isEqualTo(HttpStatus.OK);

final AggregatedHttpResponse res = adminClient.get("/api/v1/projects/test_prj").aggregate().join();
final AggregatedHttpResponse res = systemAdminClient.get("/api/v1/projects/test_prj")
.aggregate().join();
final ProjectMetadata meta = Jackson.readValue(res.contentUtf8(), ProjectMetadata.class);
return meta.repo(repoName).writeQuota();
}
Expand Down
2 changes: 1 addition & 1 deletion ...rmission/src/test/java/com/linecorp/centraldogma/server/test/XdsMemberPermissionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class XdsMemberPermissionTest {

@Override
protected void configure(CentralDogmaBuilder builder) {
builder.administrators(USERNAME)
builder.systemAdministrators(USERNAME)
.cors("*")
.authProviderFactory(new ShiroAuthProviderFactory(unused -> {
final Ini iniConfig = new Ini();
Expand Down
42 changes: 21 additions & 21 deletions ...com/linecorp/centraldogma/server/internal/mirror/MirroringAndCredentialServiceV1Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class MirroringAndCredentialServiceV1Test {
@Override
protected void configure(CentralDogmaBuilder builder) {
builder.authProviderFactory(new TestAuthProviderFactory());
builder.administrators(USERNAME);
builder.systemAdministrators(USERNAME);
}

@Override
Expand All @@ -87,16 +87,16 @@ protected void scaffold(CentralDogma client) {
}
};

private BlockingWebClient adminClient;
private BlockingWebClient systemAdminClient;
private BlockingWebClient userClient;

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

final String userToken = getAccessToken(dogma.httpClient(), USERNAME2, PASSWORD2);
userClient = WebClient.builder(dogma.httpClient().uri())
Expand Down Expand Up @@ -146,12 +146,12 @@ private void rejectInvalidRepositoryUri() {

private void setUpRole() {
final ResponseEntity<Revision> res =
adminClient.prepare()
.post("/api/v1/metadata/{proj}/members")
.pathParam("proj", FOO_PROJ)
.contentJson(ImmutableMap.of("id", USERNAME2, "role", "OWNER"))
.asJson(Revision.class)
.execute();
systemAdminClient.prepare()
.post("/api/v1/metadata/{proj}/members")
.pathParam("proj", FOO_PROJ)
.contentJson(ImmutableMap.of("id", USERNAME2, "role", "OWNER"))
.asJson(Revision.class)
.execute();
assertThat(res.status()).isEqualTo(HttpStatus.OK);
}

Expand Down Expand Up @@ -181,8 +181,8 @@ private void createAndReadCredential() {
assertThat(creationResponse.status()).isEqualTo(HttpStatus.CREATED);
assertThat(creationResponse.content().revision().major()).isEqualTo(i + 2);

for (BlockingWebClient client : ImmutableList.of(adminClient, userClient)) {
final boolean isAdmin = client == adminClient;
for (BlockingWebClient client : ImmutableList.of(systemAdminClient, userClient)) {
final boolean isSystemAdmin = client == systemAdminClient;
final ResponseEntity<Credential> fetchResponse =
client.prepare()
.get("/api/v1/projects/{proj}/credentials/{id}")
Expand All @@ -197,14 +197,14 @@ private void createAndReadCredential() {
if ("password".equals(credentialType)) {
final PasswordCredential actual = (PasswordCredential) credentialDto;
assertThat(actual.username()).isEqualTo(credential.get("username"));
if (isAdmin) {
if (isSystemAdmin) {
assertThat(actual.password()).isEqualTo(credential.get("password"));
} else {
assertThat(actual.password()).isEqualTo("****");
}
} else if ("access_token".equals(credentialType)) {
final AccessTokenCredential actual = (AccessTokenCredential) credentialDto;
if (isAdmin) {
if (isSystemAdmin) {
assertThat(actual.accessToken()).isEqualTo(credential.get("accessToken"));
} else {
assertThat(actual.accessToken()).isEqualTo("****");
Expand All @@ -213,7 +213,7 @@ private void createAndReadCredential() {
final PublicKeyCredential actual = (PublicKeyCredential) credentialDto;
assertThat(actual.username()).isEqualTo(credential.get("username"));
assertThat(actual.publicKey()).isEqualTo(credential.get("publicKey"));
if (isAdmin) {
if (isSystemAdmin) {
assertThat(actual.rawPrivateKey()).isEqualTo(credential.get("privateKey"));
assertThat(actual.rawPassphrase()).isEqualTo(credential.get("passphrase"));
} else {
Expand Down Expand Up @@ -248,8 +248,8 @@ private void updateCredential() {
.execute();
assertThat(creationResponse.status()).isEqualTo(HttpStatus.OK);

for (BlockingWebClient client : ImmutableList.of(adminClient, userClient)) {
final boolean isAdmin = client == adminClient;
for (BlockingWebClient client : ImmutableList.of(systemAdminClient, userClient)) {
final boolean isSystemAdmin = client == systemAdminClient;
final ResponseEntity<Credential> fetchResponse =
client.prepare()
.get("/api/v1/projects/{proj}/credentials/{id}")
Expand All @@ -261,7 +261,7 @@ private void updateCredential() {
assertThat(actual.id()).isEqualTo((String) credential.get("id"));
assertThat(actual.username()).isEqualTo(credential.get("username"));
assertThat(actual.publicKey()).isEqualTo(credential.get("publicKey"));
if (isAdmin) {
if (isSystemAdmin) {
assertThat(actual.rawPrivateKey()).isEqualTo(credential.get("privateKey"));
assertThat(actual.rawPassphrase()).isEqualTo(credential.get("passphrase"));
} else {
Expand Down
8 changes: 4 additions & 4 deletions server/src/main/java/com/linecorp/centraldogma/server/CentralDogma.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@
import com.linecorp.centraldogma.server.internal.admin.service.DefaultLogoutService;
import com.linecorp.centraldogma.server.internal.admin.service.RepositoryService;
import com.linecorp.centraldogma.server.internal.admin.service.UserService;
import com.linecorp.centraldogma.server.internal.api.AdministrativeService;
import com.linecorp.centraldogma.server.internal.api.ContentServiceV1;
import com.linecorp.centraldogma.server.internal.api.CredentialServiceV1;
import com.linecorp.centraldogma.server.internal.api.GitHttpService;
Expand All @@ -141,6 +140,7 @@
import com.linecorp.centraldogma.server.internal.api.MirroringServiceV1;
import com.linecorp.centraldogma.server.internal.api.ProjectServiceV1;
import com.linecorp.centraldogma.server.internal.api.RepositoryServiceV1;
import com.linecorp.centraldogma.server.internal.api.SystemAdministrativeService;
import com.linecorp.centraldogma.server.internal.api.TokenService;
import com.linecorp.centraldogma.server.internal.api.WatchService;
import com.linecorp.centraldogma.server.internal.api.auth.ApplicationTokenAuthorizer;
Expand Down Expand Up @@ -742,7 +742,7 @@ private AuthProvider createAuthProvider(
final AuthProviderParameters parameters = new AuthProviderParameters(
// Find application first, then find the session token.
new ApplicationTokenAuthorizer(mds::findTokenBySecret).orElse(
new SessionTokenAuthorizer(sessionManager, authCfg.administrators())),
new SessionTokenAuthorizer(sessionManager, authCfg.systemAdministrators())),
cfg,
sessionManager::generateSessionId,
// Propagate login and logout events to the other replicas.
Expand Down Expand Up @@ -817,7 +817,7 @@ private Function<? super HttpService, AuthService> authService(
final Authorizer<HttpRequest> tokenAuthorizer =
new ApplicationTokenAuthorizer(mds::findTokenBySecret)
.orElse(new SessionTokenAuthorizer(sessionManager,
authCfg.administrators()));
authCfg.systemAdministrators()));
return AuthService.builder()
.add(tokenAuthorizer)
.onFailure(new CentralDogmaAuthFailureHandler())
Expand Down Expand Up @@ -862,7 +862,7 @@ private void configureHttpApi(ServerBuilder sb,
assert statusManager != null;
final ContextPathServicesBuilder apiV1ServiceBuilder = sb.contextPath(API_V1_PATH_PREFIX);
apiV1ServiceBuilder
.annotatedService(new AdministrativeService(executor, statusManager))
.annotatedService(new SystemAdministrativeService(executor, statusManager))
.annotatedService(new ProjectServiceV1(projectApiManager, executor))
.annotatedService(new RepositoryServiceV1(executor, mds))
.annotatedService(new CredentialServiceV1(projectApiManager, executor));
Expand Down
Loading

0 comments on commit b0d7708

Please sign in to comment.