Convert role and API management to REST API (#107)

Converted role and API key management to REST API
bazaarvoice · Jun 7, 2017 · 2800f22 · 2800f22
1 parent 4ed3691
commit 2800f22
Show file tree

Hide file tree

Showing 78 changed files with 5,429 additions and 1,691 deletions.
diff --git a/README.md b/README.md
@@ -42,7 +42,9 @@ Documentation
 
 [API Keys](https://bazaarvoice.github.io/emodb/security/)
 
-[API Key Management](https://bazaarvoice.github.io/emodb/securityadmin/)
+[User Access Control](https://bazaarvoice.github.io/emodb/useraccesscontrol/)
+
+[Security](https://bazaarvoice.github.io/emodb/securityadmin/)
 
 [Legacy Java Client Support](markdowns/LegacyJavaClients.md)
 

diff --git a/...h-core/src/main/java/com/bazaarvoice/emodb/auth/identity/InMemoryAuthIdentityManager.java b/...h-core/src/main/java/com/bazaarvoice/emodb/auth/identity/InMemoryAuthIdentityManager.java
@@ -47,6 +47,7 @@ synchronized public String createIdentity(String authenticationId, AuthIdentityM
         }
         String id = _uniqueIdSupplier.get();
         T identity = modification.buildNew(id);
+        identity.setMaskedId("***");
         identity.setIssued(new Date());
         _authenticationToIdMap.put(authenticationId, id);
         _identityMap.put(id, identity);

diff --git a/.../auth-core/src/main/java/com/bazaarvoice/emodb/auth/jersey/AuthResourceFilterFactory.java b/.../auth-core/src/main/java/com/bazaarvoice/emodb/auth/jersey/AuthResourceFilterFactory.java
@@ -50,7 +50,7 @@
  */
 public class AuthResourceFilterFactory implements ResourceFilterFactory {
 
-    private final static Pattern SUBSTITUTION_MATCHER = Pattern.compile("\\{(?<param>(\\?.|[^\\?]).*)\\}");
+    private final static Pattern SUBSTITUTION_MATCHER = Pattern.compile("\\{(?<param>(\\?[^}]|[^?}])[^}]*)}");
 
     private final SecurityManager _securityManager;
     private final AuthenticationTokenGenerator<?> _tokenGenerator;

diff --git a/...-core/src/main/java/com/bazaarvoice/emodb/auth/jersey/AuthenticationExceptionHandler.java b/...-core/src/main/java/com/bazaarvoice/emodb/auth/jersey/AuthenticationExceptionHandler.java
@@ -27,7 +27,7 @@ public AuthenticationExceptionHandler(@Context Providers providers) {
     public Response toResponse(AuthenticationException exception) {
         // AuthenticationException is only used internally to propagate authorization errors.  Convert the
         // exception to the equivalent public-facing exception from the API.
-        UnauthorizedException apiException = new UnauthorizedException("not authenticated");
+        UnauthorizedException apiException = new UnauthorizedException();
         return _providers.getExceptionMapper(UnauthorizedException.class).toResponse(apiException);
     }
 }
diff --git a/...h-core/src/main/java/com/bazaarvoice/emodb/auth/jersey/AuthorizationExceptionHandler.java b/...h-core/src/main/java/com/bazaarvoice/emodb/auth/jersey/AuthorizationExceptionHandler.java
@@ -25,7 +25,7 @@ public AuthorizationExceptionHandler(@Context Providers providers) {
     public Response toResponse(AuthorizationException exception) {
         // AuthorizationException is only used internally to propagate authorization errors.  Convert the
         // exception to the equivalent public-facing exception from the API.
-        UnauthorizedException apiException = new UnauthorizedException("not authorized");
+        UnauthorizedException apiException = new UnauthorizedException();
         return _providers.getExceptionMapper(UnauthorizedException.class).toResponse(apiException);
     }
 }
diff --git a/auth/auth-core/src/main/java/com/bazaarvoice/emodb/auth/permissions/MatchingPermission.java b/auth/auth-core/src/main/java/com/bazaarvoice/emodb/auth/permissions/MatchingPermission.java
@@ -7,6 +7,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import org.apache.shiro.authz.Permission;
+import org.apache.shiro.authz.permission.InvalidPermissionStringException;
 
 import java.io.Serializable;
 import java.util.Arrays;
@@ -45,7 +46,9 @@ public MatchingPermission(String permission) {
 
     protected MatchingPermission(String permission, boolean initializePermission) {
         _permission = checkNotNull(permission, "permission");
-        checkArgument(!"".equals(permission.trim()), "Permission must be a non-null, non-empty string");
+        if ("".equals(permission.trim())) {
+            throw new InvalidPermissionStringException("Permission must be a non-null, non-empty string", permission);
+        }
 
         if (initializePermission) {
             initializePermission();
@@ -58,17 +61,24 @@ protected MatchingPermission(String permission, boolean initializePermission) {
      * with the initialization parameter set to false and then call this method when ready.
      */
     protected void initializePermission() {
-        List<MatchingPart> parts = Lists.newArrayList();
+        try {
+            List<MatchingPart> parts = Lists.newArrayList();
 
-        for (String partString : split(_permission)) {
-            partString = partString.trim();
-            checkArgument(!"".equals(partString), "Permission cannot contain empty parts");
+            for (String partString : split(_permission)) {
+                partString = partString.trim();
+                checkArgument(!"".equals(partString), "Permission cannot contain empty parts");
 
-            MatchingPart part = toPart(Collections.unmodifiableList(parts), partString);
-            parts.add(part);
-        }
+                MatchingPart part = toPart(Collections.unmodifiableList(parts), partString);
+                parts.add(part);
+            }
 
-        _parts = ImmutableList.copyOf(parts);
+            _parts = ImmutableList.copyOf(parts);
+        } catch (InvalidPermissionStringException e) {
+            throw e;
+        } catch (Exception e) {
+            // Rethrow any uncaught exception as being caused by an invalid permission string
+            throw new InvalidPermissionStringException(e.getMessage(), _permission);
+        }
     }
 
     public MatchingPermission(String... parts) {

diff --git a/...th-core/src/main/java/com/bazaarvoice/emodb/auth/permissions/PermissionUpdateRequest.java b/...th-core/src/main/java/com/bazaarvoice/emodb/auth/permissions/PermissionUpdateRequest.java
@@ -57,4 +57,12 @@ public PermissionUpdateRequest revokeRest() {
     public boolean isRevokeRest() {
         return _revokeRest;
     }
+
+    /**
+     * Returns true if this request could potentially modify permissions if applied.  It returns true if there is at
+     * least one permission permitted or revoked, or {@link #revokeRest()} is true.
+     */
+    public boolean mayModifyPermissions() {
+        return !_permissions.isEmpty() || _revokeRest;
+    }
 }
diff --git a/auth/auth-core/src/main/java/com/bazaarvoice/emodb/auth/role/RoleExistsException.java b/auth/auth-core/src/main/java/com/bazaarvoice/emodb/auth/role/RoleExistsException.java
@@ -11,7 +11,7 @@ public class RoleExistsException extends RuntimeException {
     private final String _id;
 
     public RoleExistsException(String group, String id) {
-        super("Role not found");
+        super("Role exists");
         _group = group;
         _id = id;
     }

diff --git a/auth/auth-core/src/main/java/com/bazaarvoice/emodb/auth/role/RoleIdentifier.java b/auth/auth-core/src/main/java/com/bazaarvoice/emodb/auth/role/RoleIdentifier.java
@@ -2,6 +2,8 @@
 
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonValue;
+import com.google.common.collect.ComparisonChain;
+import com.google.common.collect.Ordering;
 import org.apache.shiro.authz.AuthorizationInfo;
 
 import javax.annotation.Nullable;
@@ -16,7 +18,7 @@
  * as demonstrated by {@link AuthorizationInfo#getRoles()}, so in all authentication and authorization interfaces
  * the string representation as returned by {@link #toString()} is used to identify roles.
  */
-public class RoleIdentifier {
+public class RoleIdentifier implements Comparable<RoleIdentifier> {
     private final String _group;
     private final String _id;
 
@@ -75,4 +77,12 @@ public boolean equals(Object o) {
     public int hashCode() {
         return Objects.hash(_group, _id);
     }
+
+    @Override
+    public int compareTo(RoleIdentifier o) {
+        return ComparisonChain.start()
+                .compare(getGroup(), o.getGroup(), Ordering.natural().nullsFirst())
+                .compare(getId(), o.getId())
+                .result();
+    }
 }
diff --git a/.../src/main/java/com/bazaarvoice/emodb/auth/permissions/CacheManagingPermissionManager.java b/.../src/main/java/com/bazaarvoice/emodb/auth/permissions/CacheManagingPermissionManager.java
@@ -30,8 +30,12 @@ public Set<Permission> getPermissions(String id) {
 
     @Override
     public void updatePermissions(String id, PermissionUpdateRequest updates) {
+        // If the request doesn't contain any modifications then still forward to the delegate but don't invalidate
+        // the cache.
         _manager.updatePermissions(id, updates);
-        _cacheManager.invalidateAll();
+        if (updates.mayModifyPermissions()) {
+            _cacheManager.invalidateAll();
+        }
     }
 
     @Override

diff --git a/...store/src/main/java/com/bazaarvoice/emodb/auth/permissions/TablePermissionManagerDAO.java b/...store/src/main/java/com/bazaarvoice/emodb/auth/permissions/TablePermissionManagerDAO.java
@@ -17,7 +17,6 @@
 import org.apache.shiro.authz.permission.InvalidPermissionStringException;
 import org.apache.shiro.authz.permission.PermissionResolver;
 
-import javax.annotation.Nullable;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
@@ -74,48 +73,36 @@ public void updatePermissions(String id, PermissionUpdateRequest request) {
         checkNotNull(request, "request");
         validateTable();
 
-        Delta delta = createDelta(request);
-        if (delta == null) {
-            // Request did not change the permissions at all.  Take no action.
-            return;
+        // Only update if the request may potentially modify the permissions
+        if (request.mayModifyPermissions()) {
+            Delta delta = createDelta(request);
+
+            _dataStore.update(
+                    _tableName,
+                    id,
+                    TimeUUIDs.newUUID(),
+                    delta,
+                    new AuditBuilder().setLocalHost().setComment("update permissions").build(),
+                    WriteConsistency.GLOBAL);
         }
-
-        _dataStore.update(
-                _tableName,
-                id,
-                TimeUUIDs.newUUID(),
-                delta,
-                new AuditBuilder().setLocalHost().setComment("update permissions").build(),
-                WriteConsistency.GLOBAL);
     }
 
     /**
      * Returns a delta constructed from this request, or null if the request contained no changes.
      */
-    @Nullable
     private Delta createDelta(PermissionUpdateRequest request) {
         MapDeltaBuilder builder = Deltas.mapBuilder();
-        boolean modified = false;
 
         for (String permissionString : request.getPermitted()) {
             builder.put("perm_" + validated(permissionString), 1);
-            modified = true;
         }
         for (String permissionString : request.getRevoked()) {
             builder.remove("perm_" + validated(permissionString));
-            modified = true;
         }
         if (request.isRevokeRest()) {
             builder.removeRest();
-            modified = true;
         }
-
-        if (modified) {
-            return builder.build();
-        }
-
-        // Request contained no changes
-        return null;
+        return builder.build();
     }
 
     /**