With this change, the creation of API keys will throw an error if one tries to create a new API key. To make this error distinguishable from other errors, we use a 404 (not found) in this case (a 409 would be indistinguishable from a "real" conflict, 401 or 403 could be misleading). Doing this, the cli client can print better error messages.

In addition, this removes the links to API keys in user hal objects, when API keys are disabled.

Committed-by: Eduard Heimbuch <eduard.heimbuch@cloudogu.com>
Co-authored-by: René Pfeuffer <rene.pfeuffer@cloudogu.com>

scm-dao-xml/src/test/java/sonia/scm/store/TypedStoreContextTest.java

@@ -47,7 +47,6 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 @ExtendWith(MockitoExtension.class)
-@SuppressWarnings("unstableApiUsage")
 class TypedStoreContextTest {
 
   @Test

scm-webapp/src/main/java/sonia/scm/api/v2/ApiKeysDisabledExceptionMapper.java

@@ -0,0 +1,46 @@
+/*
+ * MIT License
+ *
+ * Copyright (c) 2020-present Cloudogu GmbH and Contributors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+package sonia.scm.api.v2;
+
+import sonia.scm.api.rest.ContextualExceptionMapper;
+import sonia.scm.api.v2.resources.ExceptionWithContextToErrorDtoMapper;
+import sonia.scm.security.ApiKeysDisabledException;
+
+import javax.inject.Inject;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.ext.Provider;
+
+/**
+ * @since 2.43.0
+ */
+@Provider
+public class ApiKeysDisabledExceptionMapper extends ContextualExceptionMapper<ApiKeysDisabledException> {
+  @Inject
+  public ApiKeysDisabledExceptionMapper(ExceptionWithContextToErrorDtoMapper mapper) {
+    // We will use the http status code 404 (not found) here, to make a distinction
+    // between the case of disabled api keys and an already existing api key.
+    super(ApiKeysDisabledException.class, Response.Status.NOT_FOUND, mapper);
+  }
+}

scm-webapp/src/main/java/sonia/scm/api/v2/resources/CliResource.java

@@ -45,7 +45,6 @@ import javax.ws.rs.PathParam;
 import javax.ws.rs.QueryParam;
 import javax.ws.rs.core.Context;
 import javax.ws.rs.core.MediaType;
-import javax.ws.rs.core.Response;
 import javax.ws.rs.core.StreamingOutput;
 import java.util.List;
 
@@ -104,11 +103,13 @@ public class CliResource {
     )
   )
   @ApiResponse(responseCode = "401", description = "not authenticated / invalid credentials")
+  @ApiResponse(responseCode = "404", description = "API keys disabled globally")
+  @ApiResponse(responseCode = "409", description = "There already is an API key with this name")
   @ApiResponse(responseCode = "500", description = "internal server error")
-  public Response login(CliAuthenticationDto auth) {
+  public String login(CliAuthenticationDto auth) {
     String username = SecurityUtils.getSubject().getPrincipal().toString();
     ApiKeyService.CreationResult newKey = service.createNewKey(username, auth.getApiKey(), "*");
-    return Response.ok(newKey.getToken()).build();
+    return newKey.getToken();
   }
 
   @DELETE
@@ -117,14 +118,13 @@ public class CliResource {
   @ApiResponse(responseCode = "204", description = "delete success or nothing to delete")
   @ApiResponse(responseCode = "400", description = "bad request, required parameter is missing")
   @ApiResponse(responseCode = "401", description = "not authenticated / invalid credentials")
-  public Response logout(@PathParam("apiKey") String apiKeyName) {
+  public void logout(@PathParam("apiKey") String apiKeyName) {
     String username = SecurityUtils.getSubject().getPrincipal().toString();
     service.getKeys(username)
       .stream()
       .filter(apiKey -> apiKey.getDisplayName().equals(apiKeyName))
       .findFirst()
       .ifPresent(apiKey -> service.remove(username, apiKey.getId()));
-    return Response.noContent().build();
   }
 
   @Data

scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserToUserDtoMapper.java

@@ -29,6 +29,7 @@ import de.otto.edison.hal.Links;
 import org.mapstruct.Mapper;
 import org.mapstruct.Mapping;
 import org.mapstruct.ObjectFactory;
+import sonia.scm.config.ScmConfiguration;
 import sonia.scm.group.GroupPermissions;
 import sonia.scm.security.PermissionPermissions;
 import sonia.scm.user.User;
@@ -49,6 +50,8 @@ public abstract class UserToUserDtoMapper extends BaseMapper<User, UserDto> {
 
   @Inject
   private UserManager userManager;
+  @Inject
+  private ScmConfiguration scmConfiguration;
 
   @Override
   @Mapping(target = "attributes", ignore = true)
@@ -67,7 +70,9 @@ public abstract class UserToUserDtoMapper extends BaseMapper<User, UserDto> {
     if (UserPermissions.modify(user).isPermitted()) {
       linksBuilder.single(link("update", resourceLinks.user().update(user.getName())));
       linksBuilder.single(link("publicKeys", resourceLinks.user().publicKeys(user.getName())));
-      linksBuilder.single(link("apiKeys", resourceLinks.user().apiKeys(user.getName())));
+      if (scmConfiguration.isEnabledApiKeys()) {
+        linksBuilder.single(link("apiKeys", resourceLinks.user().apiKeys(user.getName())));
+      }
       if (user.isExternal()) {
         linksBuilder.single(link("convertToInternal", resourceLinks.user().toInternal(user.getName())));
       } else {

scm-webapp/src/main/java/sonia/scm/security/ApiKeyService.java

@@ -34,6 +34,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import sonia.scm.ContextEntry;
 import sonia.scm.HandlerEventType;
+import sonia.scm.config.ScmConfiguration;
 import sonia.scm.store.DataStore;
 import sonia.scm.store.DataStoreFactory;
 import sonia.scm.user.UserEvent;
@@ -53,6 +54,7 @@ import static java.util.stream.Collectors.toList;
 import static org.apache.commons.lang.RandomStringUtils.random;
 import static sonia.scm.AlreadyExistsException.alreadyExists;
 
+@SuppressWarnings("UnstableApiUsage")
 public class ApiKeyService {
 
   private static final Logger LOG = LoggerFactory.getLogger(ApiKeyService.class);
@@ -63,24 +65,29 @@ public class ApiKeyService {
   private final KeyGenerator keyGenerator;
   private final Supplier<String> passphraseGenerator;
   private final ApiKeyTokenHandler tokenHandler;
+  private final ScmConfiguration scmConfiguration;
 
   private final Striped<ReadWriteLock> locks = Striped.readWriteLock(10);
 
   @Inject
-  ApiKeyService(DataStoreFactory storeFactory, KeyGenerator keyGenerator, PasswordService passwordService, ApiKeyTokenHandler tokenHandler) {
-    this(storeFactory, passwordService, keyGenerator, tokenHandler, () -> random(PASSPHRASE_LENGTH, 0, 0, true, true, null, new SecureRandom()));
+  ApiKeyService(DataStoreFactory storeFactory, KeyGenerator keyGenerator, PasswordService passwordService, ApiKeyTokenHandler tokenHandler, ScmConfiguration scmConfiguration) {
+    this(storeFactory, passwordService, keyGenerator, tokenHandler, () -> random(PASSPHRASE_LENGTH, 0, 0, true, true, null, new SecureRandom()), scmConfiguration);
   }
 
-  ApiKeyService(DataStoreFactory storeFactory, PasswordService passwordService, KeyGenerator keyGenerator, ApiKeyTokenHandler tokenHandler, Supplier<String> passphraseGenerator) {
+  ApiKeyService(DataStoreFactory storeFactory, PasswordService passwordService, KeyGenerator keyGenerator, ApiKeyTokenHandler tokenHandler, Supplier<String> passphraseGenerator, ScmConfiguration scmConfiguration) {
     this.store = storeFactory.withType(ApiKeyCollection.class).withName("apiKeys").build();
     this.passwordService = passwordService;
     this.keyGenerator = keyGenerator;
     this.tokenHandler = tokenHandler;
     this.passphraseGenerator = passphraseGenerator;
+    this.scmConfiguration = scmConfiguration;
   }
 
   public CreationResult createNewKey(String username, String keyDisplayName, String permissionRole) {
     UserPermissions.changeApiKeys(username).check();
+    if (!scmConfiguration.isEnabledApiKeys()) {
+      throw new ApiKeysDisabledException();
+    }
     String passphrase = passphraseGenerator.get();
     String hashedPassphrase = passwordService.encryptPassword(passphrase);
     String id = keyGenerator.createKey();

scm-webapp/src/main/java/sonia/scm/security/ApiKeysDisabledException.java

@@ -0,0 +1,46 @@
+/*
+ * MIT License
+ *
+ * Copyright (c) 2020-present Cloudogu GmbH and Contributors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+package sonia.scm.security;
+
+import sonia.scm.ExceptionWithContext;
+
+import static java.util.Collections.emptyList;
+
+/**
+ * @since 2.43.0
+ */
+public class ApiKeysDisabledException extends ExceptionWithContext {
+
+  private static final String CODE = "3ITWbABz91";
+
+  protected ApiKeysDisabledException() {
+    super(emptyList(), "API keys are disabled");
+  }
+
+  @Override
+  public String getCode() {
+    return CODE;
+  }
+}

scm-webapp/src/main/resources/locales/de/plugins.json

@@ -359,18 +359,26 @@
       "displayName": "Repository wird exportiert",
       "description": "Das Repository wird momentan exportiert und darf nicht modifiziert werden."
     },
-    "8YR7aawFW1": {
-      "displayName": "Aktuelles Passwort falsch",
-      "description": "Das aktuelle Passwort ist falsch. Bitte versuchen Sie es noch einmal."
-    },
     "5GSO9ZkzX1": {
       "displayName": "Inkompatible Umgebung",
       "description": "Die Version dieses SCM-Managers oder eines der installierten Plugins ist zu alt für den Import des Dumps. Bitte installieren Sie die neuesten Versionen. Nähere Informationen finden sich im Log."
     },
+    "8YR7aawFW1": {
+      "displayName": "Aktuelles Passwort falsch",
+      "description": "Das aktuelle Passwort ist falsch. Bitte versuchen Sie es noch einmal."
+    },
     "CISPvega31": {
       "displayName": "Ungültiger Repository-Typ für Import",
       "description": "Der Import ist für den gegebenen Repository-Typen nicht möglich."
     },
+    "3mSmwOtOd1": {
+      "displayName": "Datei gesperrt",
+      "description": "Die Datei oder ihre Sperre kann nicht bearbeitet werden, da eine andere Sperre existiert. Die andere Sperre kann ggf. durch eine 'forcierte' Aktion umgangen werden."
+    },
+    "8wSpi62oJ1": {
+      "displayName": "Änderung fehlgeschlagen",
+      "description": "Die Änderung konnte nicht durchgeführt werden. Dieses kann mehrere Ursachen haben, z. B. eine Datei die nicht verschoben werden kann, ungültige Dateinamen o. ä."
+    },
     "5DSqG6Mcg1": {
       "displayName": "Fehlender Source Parameter",
       "description": "Der Source Parameter wird für die Authentifizierung benötigt."
@@ -402,6 +410,10 @@
     "8OT4gBVvp1": {
       "displayName": "Repository Typ ungültig",
       "description": "Dieser Repository Typ wird nicht unterstützt."
+    },
+    "3ITWbABz91": {
+      "displayName": "API Schlüssel deaktiviert",
+      "description": "Die Nutzung von API Schlüsseln ist in der globalen Konfiguration deaktiviert. Zur Nutzung der API Schlüssel muss diese Option wieder aktiviert werden."
     }
   },
   "healthCheckFailures": {
@@ -461,14 +473,6 @@
     "5FSV2kreE1": {
       "summary": "'svn verify' fehlgeschlagen",
       "description": "Die Prüfung 'svn verify' ist für das Repository fehlgeschlagen."
-    },
-    "3mSmwOtOd1": {
-      "displayName": "Datei gesperrt",
-      "description": "Die Datei oder ihre Sperre kann nicht bearbeitet werden, da eine andere Sperre existiert. Die andere Sperre kann ggf. durch eine 'forcierte' Aktion umgangen werden."
-    },
-    "8wSpi62oJ1": {
-      "displayName": "Änderung fehlgeschlagen",
-      "description": "Die Änderung konnte nicht durchgeführt werden. Dieses kann mehrere Ursachen haben, z. B. eine Datei die nicht verschoben werden kann, ungültige Dateinamen o. ä."
     }
   },
   "namespaceStrategies": {

scm-webapp/src/main/resources/locales/en/plugins.json

@@ -410,6 +410,10 @@
     "8OT4gBVvp1": {
       "displayName": "Repository type invalid",
       "description": "The repository type is not supported."
+    },
+    "3ITWbABz91": {
+      "displayName": "API keys disabled",
+      "description": "The usage of API keys has been disabled in the global configuration. To use API keys, this has to be enabled again."
     }
   },
   "healthChecksFailures": {

scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java

@@ -41,6 +41,7 @@ import org.mockito.Mock;
 import sonia.scm.ContextEntry;
 import sonia.scm.NotFoundException;
 import sonia.scm.PageResult;
+import sonia.scm.config.ScmConfiguration;
 import sonia.scm.group.GroupManager;
 import sonia.scm.security.ApiKeyService;
 import sonia.scm.security.PermissionAssigner;
@@ -110,6 +111,8 @@ public class UserRootResourceTest {
   private GroupManager groupManager;
   @Mock
   private GroupToGroupDtoMapper groupToGroupDtoMapper;
+  @Mock
+  private ScmConfiguration scmConfiguration;
   @InjectMocks
   private UserDtoToUserMapperImpl dtoToUserMapper;
   @InjectMocks

scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserToUserDtoMapperTest.java

@@ -33,6 +33,7 @@ import org.junit.Before;
 import org.junit.Test;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
+import sonia.scm.config.ScmConfiguration;
 import sonia.scm.user.User;
 import sonia.scm.user.UserManager;
 import sonia.scm.user.UserTestData;
@@ -55,6 +56,8 @@ public class UserToUserDtoMapperTest {
 
   @Mock
   private UserManager userManager;
+  @Mock
+  private ScmConfiguration scmConfiguration;
 
   @InjectMocks
   private UserToUserDtoMapperImpl mapper;
@@ -188,4 +191,26 @@ public class UserToUserDtoMapperTest {
     assertEquals("expected permissions link", expectedBaseUri.resolve("abc/permissions").toString(), userDto.getLinks().getLinkBy("permissions").get().getHref());
     assertEquals("expected permission overview link", expectedBaseUri.resolve("abc/permissionOverview").toString(), userDto.getLinks().getLinkBy("permissionOverview").get().getHref());
   }
+
+  @Test
+  public void shouldMapApiKeyLinks_IfEnabled() {
+    User user = createDefaultUser();
+    when(subject.isPermitted("user:modify:abc")).thenReturn(true);
+    when(scmConfiguration.isEnabledApiKeys()).thenReturn(true);
+
+    UserDto userDto = mapper.map(user);
+
+    assertEquals("expected api key link", expectedBaseUri.resolve("abc/api_keys").toString(), userDto.getLinks().getLinkBy("apiKeys").get().getHref());
+  }
+
+  @Test
+  public void shouldNotMapApiKeyLinks_IfDisabled() {
+    User user = createDefaultUser();
+    when(subject.isPermitted("user:modify:abc")).thenReturn(true);
+    when(scmConfiguration.isEnabledApiKeys()).thenReturn(false);
+
+    UserDto userDto = mapper.map(user);
+
+    assertThat(userDto.getLinks().getLinkBy("apiKeys")).isEmpty();
+  }
 }

scm-webapp/src/test/java/sonia/scm/security/ApiKeyServiceTest.java

@@ -35,6 +35,7 @@ import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import sonia.scm.AlreadyExistsException;
 import sonia.scm.HandlerEventType;
+import sonia.scm.config.ScmConfiguration;
 import sonia.scm.store.DataStore;
 import sonia.scm.store.DataStoreFactory;
 import sonia.scm.store.InMemoryDataStore;
@@ -61,7 +62,8 @@ class ApiKeyServiceTest {
   ApiKeyTokenHandler tokenHandler = new ApiKeyTokenHandler();
   DataStoreFactory storeFactory = new InMemoryDataStoreFactory(new InMemoryDataStore<ApiKeyCollection>());
   DataStore<ApiKeyCollection> store = storeFactory.withType(ApiKeyCollection.class).withName("apiKeys").build();
-  ApiKeyService service = new ApiKeyService(storeFactory, passwordService, keyGenerator, tokenHandler, passphraseGenerator);
+  ScmConfiguration scmConfiguration = new ScmConfiguration();
+  ApiKeyService service = new ApiKeyService(storeFactory, passwordService, keyGenerator, tokenHandler, passphraseGenerator, scmConfiguration);
 
   @BeforeEach
   void mockPasswordService() {
@@ -177,5 +179,13 @@ class ApiKeyServiceTest {
 
       assertThat(store.get("dent")).isNull();
     }
+
+    @Test
+    void shouldFailIfApiKeysAreDisabled() {
+      scmConfiguration.setEnabledApiKeys(false);
+
+      assertThrows(ApiKeysDisabledException.class, () -> service.createNewKey("dent", "1", "READ"));
+
+    }
   }
 }