Mind review remarks

scm-ui/ui-webapp/src/users/components/apiKeys/AddApiKey.tsx

@@ -64,7 +64,7 @@ const AddApiKey: FC<Props> = ({
     if (!availableRepositoryRoles) {
       fetchAvailablePermissionsIfNeeded(repositoryRolesLink, repositoryVerbsLink);
     }
-  });
+  }, [repositoryRolesLink, repositoryVerbsLink]);
 
   const isValid = () => {
     return !!displayName && !!permissionRole;

scm-ui/ui-webapp/src/users/components/apiKeys/ApiKeyCreatedModal.tsx

@@ -36,6 +36,11 @@ const KeyArea = styled.textarea`
   white-space: nowrap;
   overflow: auto;
   font-family: "Courier New", Monaco, Menlo, "Ubuntu Mono", "source-code-pro", monospace;
+  height: 3rem;
+`;
+
+const NoLeftMargin = styled.div`
+  margin-left: -1rem;
 `;
 
 const ApiKeyCreatedModal: FC<Props> = ({ addedKey, close }) => {
@@ -60,9 +65,9 @@ const ApiKeyCreatedModal: FC<Props> = ({ addedKey, close }) => {
         <div className={"column is-11"}>
           <KeyArea wrap={"soft"} ref={keyRef} className={"input"} value={addedKey} />
         </div>
-        <div className={"column is-1"}>
+        <NoLeftMargin className={"column is-1"}>
           <Icon className={"is-hidden-mobile fa-2x"} name={copied ? "clipboard-check" : "clipboard"} title={t("apiKey.modal.clipboard")} onClick={copy} />
-        </div>
+        </NoLeftMargin>
       </div>
     </div>
   );

scm-ui/ui-webapp/src/users/components/apiKeys/ApiKeyEntry.tsx

@@ -23,7 +23,7 @@
  */
 
 import React, { FC } from "react";
-import { DateFromNow, Icon } from "@scm-manager/ui-components";
+import { DateFromNow, DeleteButton } from "@scm-manager/ui-components";
 import { ApiKey } from "./SetApiKeys";
 import { Link } from "@scm-manager/ui-types";
 import { useTranslation } from "react-i18next";
@@ -38,11 +38,7 @@ export const ApiKeyEntry: FC<Props> = ({ apiKey, onDelete }) => {
   let deleteButton;
   if (apiKey?._links?.delete) {
     deleteButton = (
-      <a className="level-item" onClick={() => onDelete((apiKey._links.delete as Link).href)}>
-        <span className="icon is-small">
-          <Icon name="trash" className="fas" title={t("apiKey.delete")} />
-        </span>
-      </a>
+      <DeleteButton label={t("apiKey.delete")} action={() => onDelete((apiKey._links.delete as Link).href)}/>
     );
   }
 
@@ -54,7 +50,7 @@ export const ApiKeyEntry: FC<Props> = ({ apiKey, onDelete }) => {
         <td className="is-hidden-mobile">
           <DateFromNow date={apiKey.created}/>
         </td>
-        <td className="is-darker">{deleteButton}</td>
+        <td>{deleteButton}</td>
       </tr>
     </>
   );

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

@@ -93,13 +93,13 @@ public class  ApiKeyResource {
 
   @GET
   @Path("{id}")
-  @Produces(VndMediaType.API_KEY_COLLECTION)
+  @Produces(VndMediaType.API_KEY)
   @Operation(summary = "Get one api key for the current user", description = "Returns the registered api key with the given id for the logged in user.", tags = "User")
   @ApiResponse(
     responseCode = "200",
     description = "success",
     content = @Content(
-      mediaType = VndMediaType.API_KEY_COLLECTION,
+      mediaType = VndMediaType.API_KEY,
       schema = @Schema(implementation = HalRepresentation.class)
     )
   )

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

@@ -21,7 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-    
+
 package sonia.scm.api.v2.resources;
 
 import de.otto.edison.hal.Embedded;
@@ -61,8 +61,10 @@ public abstract class BrowserResultToFileObjectDtoMapper extends BaseFileObjectD
   @Override
   void applyEnrichers(Links.Builder links, Embedded.Builder embeddedBuilder, NamespaceAndName namespaceAndName, BrowserResult browserResult, FileObject fileObject) {
     EdisonHalAppender appender = new EdisonHalAppender(links, embeddedBuilder);
-    // we call enrichers, which are only responsible for top level browseresults
-    applyEnrichers(appender, browserResult, namespaceAndName);
+    if (browserResult.getFile().equals(fileObject)) {
+      // we call enrichers, which are only responsible for top level browseresults
+      applyEnrichers(appender, browserResult, namespaceAndName);
+    }
     // we call enrichers, which are responsible for all file object top level browse result and its children
     applyEnrichers(appender, fileObject, namespaceAndName, browserResult, browserResult.getRevision());
   }

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

@@ -28,6 +28,7 @@ import lombok.AccessLevel;
 import lombok.AllArgsConstructor;
 import lombok.Getter;
 import lombok.NoArgsConstructor;
+import org.apache.commons.collections.CollectionUtils;
 
 import javax.xml.bind.annotation.XmlAccessType;
 import javax.xml.bind.annotation.XmlAccessorType;
@@ -37,6 +38,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.function.Predicate;
 
+import static java.util.Collections.singletonList;
 import static java.util.stream.Collectors.toList;
 
 @AllArgsConstructor
@@ -50,13 +52,13 @@ class ApiKeyCollection {
 
   public ApiKeyCollection add(ApiKeyWithPassphrase key) {
     Collection<ApiKeyWithPassphrase> newKeys;
-    if (keys == null) {
-      newKeys = new ArrayList<>();
+    if (CollectionUtils.isEmpty(keys)) {
+      newKeys = singletonList(key);
     } else {
       newKeys = new ArrayList<>(keys.size() + 1);
       newKeys.addAll(keys);
+      newKeys.add(key);
     }
-    newKeys.add(key);
     return new ApiKeyCollection(newKeys);
   }
 

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

@@ -42,6 +42,34 @@ public class ScmWildcardPermission extends WildcardPermission {
     super(permissionString);
   }
 
+  /**
+   * Limits this permission to the given scope. This will result in a collection of new permissions. This
+   * collection can be empty (but this will not return <code>null</code>). Three examples:
+   * <table>
+   *   <tr>
+   *     <th>This permission</th>
+   *     <th>Scope</th>
+   *     <th>Resulting permission(s)</th>
+   *   </tr>
+   *   <tr>
+   *     <td><code>repository:*:42</code></td>
+   *     <td><code>repository:read,pull:*</code></td>
+   *     <td><code>repository:read,pull:42</code></td>
+   *   </tr>
+   *   <tr>
+   *     <td><code>repository:read:*</code></td>
+   *     <td><code>repository:*:42</code>, <code>repository:*:1337</code></td>
+   *     <td><code>repository:read:42</code>, <code>repository:read:1337</code></td>
+   *   </tr>
+   *   <tr>
+   *     <td><code>user:*:*</code></td>
+   *     <td><code>repository:read,pull:*</code></td>
+   *     <td><i>empty</i></td>
+   *   </tr>
+   * </table>
+   * @param scope The scope this permission should be limited to.
+   * @return A collection with the resulting permissions (mind that this can be empty, but not <code>null</code>).
+   */
   Collection<ScmWildcardPermission> limit(Scope scope) {
     Collection<ScmWildcardPermission> result = new ArrayList<>();
     for (String s : scope) {
@@ -50,11 +78,24 @@ public class ScmWildcardPermission extends WildcardPermission {
     return result;
   }
 
+  /**
+   * Limits this permission to a scope with a single permission. For examples see {@link #limit(String)}.
+   * @param scope The single scope.
+   * @return An {@link Optional} with the resulting permission if there was a overlap between this and the scope, or
+   *   an empty {@link Optional} otherwise.
+   */
   Optional<ScmWildcardPermission> limit(String scope) {
     return limit(new ScmWildcardPermission(scope));
   }
 
+  /**
+   * Limits this permission to a scope with a single permission. For examples see {@link #limit(String)}.
+   * @param scope The single scope.
+   * @return An {@link Optional} with the resulting permission if there was a overlap between this and the scope, or
+   *   an empty {@link Optional} otherwise.
+   */
   Optional<ScmWildcardPermission> limit(ScmWildcardPermission scope) {
+    // if one permission is a subset of the other, we can return the smaller one.
     if (this.implies(scope)) {
       return of(scope);
     }
@@ -62,6 +103,8 @@ public class ScmWildcardPermission extends WildcardPermission {
       return of(this);
     }
 
+    // First we check, whether the subjects are the same. We do not use permissions with different subjects, so we
+    // either have both this the same subject, or we have no overlap.
     final List<Set<String>> theseParts = getParts();
     final List<Set<String>> scopeParts = scope.getParts();
 
@@ -69,7 +112,10 @@ public class ScmWildcardPermission extends WildcardPermission {
       return empty();
     }
 
-    String type = getEntries(scopeParts, 0).iterator().next();
+    String subject = getEntries(scopeParts, 0).iterator().next();
+
+    // Now we create the intersections of verbs and ids to create the resulting permission
+    // (if not one of the resulting sets is empty)
     Collection<String> verbs = intersect(theseParts, scopeParts, 1);
     Collection<String> ids = intersect(theseParts, scopeParts, 2);
 
@@ -77,7 +123,7 @@ public class ScmWildcardPermission extends WildcardPermission {
       return empty();
     }
 
-    return of(new ScmWildcardPermission(type + ":" + String.join(",", verbs) + ":" + String.join(",", ids)));
+    return of(new ScmWildcardPermission(subject + ":" + String.join(",", verbs) + ":" + String.join(",", ids)));
   }
 
   private Collection<String> intersect(List<Set<String>> theseParts, List<Set<String>> scopeParts, int position) {
@@ -92,6 +138,9 @@ public class ScmWildcardPermission extends WildcardPermission {
     return CollectionUtils.intersection(theseEntries, scopeEntries);
   }
 
+  /**
+   * Handles "shortened" permissions like <code>repository:read</code> that should be <code>repository:read:*</code>.
+   */
   private Set<String> getEntries(List<Set<String>> theseParts, int position) {
     if (position >= theseParts.size()) {
       return singleton(WILDCARD_TOKEN);

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

@@ -38,11 +38,9 @@ class ApiKeyTokenHandlerTest {
 
   @Test
   void shouldSerializeAndDeserializeToken() {
-    final String tokenString = handler.createToken("dent", new ApiKey("42", "hg2g", "READ", now()), "some secret");
+    String tokenString = handler.createToken("dent", new ApiKey("42", "hg2g", "READ", now()), "some secret");
 
-    System.out.println(tokenString);
-
-    final Optional<ApiKeyTokenHandler.Token> token = handler.readToken(tokenString);
+    Optional<ApiKeyTokenHandler.Token> token = handler.readToken(tokenString);
 
     assertThat(token).isNotEmpty();
     assertThat(token).get().extracting("user").isEqualTo("dent");
@@ -52,14 +50,14 @@ class ApiKeyTokenHandlerTest {
 
   @Test
   void shouldNotFailWithInvalidTokenEncoding() {
-    final Optional<ApiKeyTokenHandler.Token> token = handler.readToken("invalid token");
+    Optional<ApiKeyTokenHandler.Token> token = handler.readToken("invalid token");
 
     assertThat(token).isEmpty();
   }
 
   @Test
   void shouldNotFailWithInvalidTokenContent() {
-    final Optional<ApiKeyTokenHandler.Token> token = handler.readToken(Encoders.BASE64URL.encode("{\"invalid\":\"token\"}".getBytes()));
+    Optional<ApiKeyTokenHandler.Token> token = handler.readToken(Encoders.BASE64URL.encode("{\"invalid\":\"token\"}".getBytes()));
 
     assertThat(token).isEmpty();
   }

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

@@ -71,7 +71,7 @@ public class ScopesTest {
   }
 
   @Test
-  public void testFilterX() {
+  public void testFilterIntersectingPermissions() {
     Scope scope = Scope.valueOf("repository:read,write:*");
     AuthorizationInfo authz = authz("repository:*:123");