Update AuthorizationFactory interface (#1704)

Update `AuthorizationFactory` interface to make it more secure.

## The issues

### Wrong method invoked during on-the-fly authorization
```java
TeletraanAuthorizer<TeletraanPrincipal> authorizer = authorizationFactory.create(context);
```
When `CompositeAuthorizationFactory` is configured, this always returns the `BasePastisAuthorizer`. It is a problem because `ScriptTokenPrincipal` can only be authorized by `ScriptTokenRoleAuthorizer`, otherwise illegal access is possible. It was prevented before the introduction of on-the-fly authorization via bundling the authenticator and the authorizer in the auth filter creation. 

### `AuthorizationFactory` interface
The interface of `AuthorizationFactory` makes it possible get wrong authorizer. 
```java
<P extends TeletraanPrincipal> TeletraanAuthorizer<P> create(TeletraanServiceContext context)
            throws Exception;
```

## Fixes

### Dissociate `TeletraanAuthorizer` and `Authorizer` interfaces. 
`TeletraanAuthorizer` stops extending `Authorizer`. All authorizers implementing `TeletraanAuthorizer` now need to implement `TeletraanAuthorizer` and `Authorizer` respectively.

### New method in `AuthorizationFactory`
The existing methods are updated to return `Authorizer`.
```java
<P extends TeletraanPrincipal> Authorizer<P> create(TeletraanServiceContext context);
```

Introduced a new method to return `TeletraanAuthorizer`.

### Call new method in `EnvCapacities`
```java
authorizationFactory.createSecondaryAuthorizer(
                        context, teletraanPrincipal.getClass());
```
And it's the only option now, no way to be mistaken.

deploy-service/pom.xml

@@ -58,7 +58,7 @@
             <dependency>
                 <groupId>com.pinterest.teletraan</groupId>
                 <artifactId>universal</artifactId>
-                <version>2.2-SNAPSHOT</version>
+                <version>2.3-SNAPSHOT</version>
             </dependency>
             <dependency>
                 <groupId>com.pinterest.teletraan</groupId>

deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/config/AuthorizationFactory.java

@@ -17,17 +17,24 @@
 
 import com.fasterxml.jackson.annotation.JsonTypeInfo;
 import com.pinterest.teletraan.TeletraanServiceContext;
+import com.pinterest.teletraan.universal.security.OpenAuthorizer;
 import com.pinterest.teletraan.universal.security.TeletraanAuthorizer;
 import com.pinterest.teletraan.universal.security.bean.TeletraanPrincipal;
+import io.dropwizard.auth.Authorizer;
 import io.dropwizard.jackson.Discoverable;
 
 @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "type")
 public interface AuthorizationFactory extends Discoverable {
-    <P extends TeletraanPrincipal> TeletraanAuthorizer<P> create(TeletraanServiceContext context)
-            throws Exception;
+    <P extends TeletraanPrincipal> Authorizer<P> create(TeletraanServiceContext context);
 
-    default <P extends TeletraanPrincipal> TeletraanAuthorizer<? extends TeletraanPrincipal> create(
-            TeletraanServiceContext context, Class<P> principalClass) throws Exception {
+    default <P extends TeletraanPrincipal> Authorizer<P> create(
+            TeletraanServiceContext context, Class<P> principalClass) {
         return create(context);
     }
+
+    /** Create a secondary authorizer for on-the-fly authorization. */
+    default TeletraanAuthorizer<TeletraanPrincipal> createSecondaryAuthorizer(
+            TeletraanServiceContext context, Class<? extends TeletraanPrincipal> principalClass) {
+        return new OpenAuthorizer();
+    }
 }

deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/config/CompositeAuthorizationFactory.java

@@ -20,9 +20,12 @@
 import com.pinterest.teletraan.TeletraanServiceContext;
 import com.pinterest.teletraan.security.ScriptTokenRoleAuthorizer;
 import com.pinterest.teletraan.universal.security.BasePastisAuthorizer;
+import com.pinterest.teletraan.universal.security.DenyAllAuthorizer;
 import com.pinterest.teletraan.universal.security.TeletraanAuthorizer;
-import com.pinterest.teletraan.universal.security.bean.ServicePrincipal;
+import com.pinterest.teletraan.universal.security.bean.ScriptTokenPrincipal;
 import com.pinterest.teletraan.universal.security.bean.TeletraanPrincipal;
+import io.dropwizard.auth.Authorizer;
+import javax.ws.rs.ForbiddenException;
 
 @JsonTypeName("composite")
 public class CompositeAuthorizationFactory implements AuthorizationFactory {
@@ -39,25 +42,41 @@ public String getPastisServiceName() {
         return pastisServiceName;
     }
 
-    @Override
-    public <P extends TeletraanPrincipal> TeletraanAuthorizer<P> create(
-            TeletraanServiceContext context) throws Exception {
+    private TeletraanAuthorizer<TeletraanPrincipal> getOrCreateAuthorizer(
+            TeletraanServiceContext context) {
         if (pastisAuthorizer == null) {
             pastisAuthorizer =
                     BasePastisAuthorizer.builder()
                             .factory(context.getAuthZResourceExtractorFactory())
                             .serviceName(pastisServiceName)
                             .build();
         }
-        return (TeletraanAuthorizer<P>) pastisAuthorizer;
+        return pastisAuthorizer;
+    }
+
+    @Override
+    public <P extends TeletraanPrincipal> Authorizer<P> create(TeletraanServiceContext context) {
+        return (Authorizer<P>) getOrCreateAuthorizer(context);
     }
 
     @Override
-    public <P extends TeletraanPrincipal> TeletraanAuthorizer<? extends TeletraanPrincipal> create(
-            TeletraanServiceContext context, Class<P> principalClass) throws Exception {
-        if (ServicePrincipal.class.equals(principalClass)) {
-            return new ScriptTokenRoleAuthorizer(context.getAuthZResourceExtractorFactory());
+    public <P extends TeletraanPrincipal> Authorizer<P> create(
+            TeletraanServiceContext context, Class<P> principalClass) {
+        if (ScriptTokenPrincipal.class.equals(principalClass)) {
+            return (Authorizer<P>)
+                    new ScriptTokenRoleAuthorizer(context.getAuthZResourceExtractorFactory());
         }
         return create(context);
     }
+
+    @Override
+    public TeletraanAuthorizer<TeletraanPrincipal> createSecondaryAuthorizer(
+            TeletraanServiceContext context, Class<? extends TeletraanPrincipal> principalClass)
+            throws ForbiddenException {
+        if (ScriptTokenPrincipal.class.equals(principalClass)) {
+            // Deny all on-the-fly authorization requests for script token principals
+            return new DenyAllAuthorizer();
+        }
+        return getOrCreateAuthorizer(context);
+    }
 }

deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/config/OpenAuthorizationFactory.java

@@ -17,31 +17,14 @@
 
 import com.fasterxml.jackson.annotation.JsonTypeName;
 import com.pinterest.teletraan.TeletraanServiceContext;
-import com.pinterest.teletraan.universal.security.TeletraanAuthorizer;
-import com.pinterest.teletraan.universal.security.bean.AuthZResource;
 import com.pinterest.teletraan.universal.security.bean.TeletraanPrincipal;
-import javax.annotation.Nullable;
-import javax.ws.rs.container.ContainerRequestContext;
+import io.dropwizard.auth.Authorizer;
+import io.dropwizard.auth.PermitAllAuthorizer;
 
 @JsonTypeName("open")
 public class OpenAuthorizationFactory implements AuthorizationFactory {
     @Override
-    public TeletraanAuthorizer<TeletraanPrincipal> create(TeletraanServiceContext context)
-            throws Exception {
-        return new TeletraanAuthorizer<TeletraanPrincipal>() {
-            @Override
-            public boolean authorize(TeletraanPrincipal principal, String resource) {
-                return true;
-            }
-
-            @Override
-            public boolean authorize(
-                    TeletraanPrincipal principal,
-                    String role,
-                    AuthZResource requestedResource,
-                    @Nullable ContainerRequestContext context) {
-                return true;
-            }
-        };
+    public Authorizer<TeletraanPrincipal> create(TeletraanServiceContext context) {
+        return new PermitAllAuthorizer<>();
     }
 }

deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/config/RoleAuthorizationFactory.java

@@ -20,29 +20,30 @@
 import com.pinterest.teletraan.TeletraanServiceContext;
 import com.pinterest.teletraan.security.ScriptTokenRoleAuthorizer;
 import com.pinterest.teletraan.security.UserRoleAuthorizer;
-import com.pinterest.teletraan.universal.security.TeletraanAuthorizer;
-import com.pinterest.teletraan.universal.security.bean.ServicePrincipal;
+import com.pinterest.teletraan.universal.security.bean.ScriptTokenPrincipal;
 import com.pinterest.teletraan.universal.security.bean.TeletraanPrincipal;
 import com.pinterest.teletraan.universal.security.bean.UserPrincipal;
+import io.dropwizard.auth.Authorizer;
 
 @JsonTypeName("role")
 public class RoleAuthorizationFactory implements AuthorizationFactory {
     @JsonProperty private String roleCacheSpec; // Unused, for backwards compatibility
 
     @Override
-    public <P extends TeletraanPrincipal> TeletraanAuthorizer<P> create(
-            TeletraanServiceContext context) throws Exception {
+    public <P extends TeletraanPrincipal> Authorizer<P> create(TeletraanServiceContext context) {
         throw new UnsupportedOperationException(
                 "RoleAuthorizationFactory does not support this method. Use create(TeletraanServiceContext, Class<P>) instead.");
     }
 
     @Override
-    public <P extends TeletraanPrincipal> TeletraanAuthorizer<? extends TeletraanPrincipal> create(
-            TeletraanServiceContext context, Class<P> principalClass) throws Exception {
-        if (ServicePrincipal.class.equals(principalClass)) {
-            return new ScriptTokenRoleAuthorizer(context.getAuthZResourceExtractorFactory());
+    public <P extends TeletraanPrincipal> Authorizer<P> create(
+            TeletraanServiceContext context, Class<P> principalClass) {
+        if (ScriptTokenPrincipal.class.equals(principalClass)) {
+            return (Authorizer<P>)
+                    new ScriptTokenRoleAuthorizer(context.getAuthZResourceExtractorFactory());
         } else if (UserPrincipal.class.equals(principalClass)) {
-            return new UserRoleAuthorizer(context, context.getAuthZResourceExtractorFactory());
+            return (Authorizer<P>)
+                    new UserRoleAuthorizer(context, context.getAuthZResourceExtractorFactory());
         }
         throw new UnsupportedOperationException("Unsupported principal class: " + principalClass);
     }

deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/config/TokenAuthenticationFactory.java

@@ -24,7 +24,6 @@
 import com.pinterest.teletraan.universal.security.OAuthAuthenticator;
 import com.pinterest.teletraan.universal.security.ScriptTokenAuthenticator;
 import com.pinterest.teletraan.universal.security.bean.ScriptTokenPrincipal;
-import com.pinterest.teletraan.universal.security.bean.ServicePrincipal;
 import com.pinterest.teletraan.universal.security.bean.UserPrincipal;
 import com.pinterest.teletraan.universal.security.bean.ValueBasedRole;
 import io.dropwizard.auth.AuthFilter;
@@ -34,6 +33,7 @@
 import io.dropwizard.auth.JSONUnauthorizedHandler;
 import io.dropwizard.auth.chained.ChainedAuthFilter;
 import io.dropwizard.auth.oauth.OAuthCredentialAuthFilter;
+import java.net.MalformedURLException;
 import java.util.Arrays;
 import javax.validation.constraints.NotEmpty;
 import javax.ws.rs.container.ContainerRequestFilter;
@@ -103,7 +103,7 @@ public ContainerRequestFilter create(TeletraanServiceContext context) throws Exc
 
     @SuppressWarnings({"unchecked"})
     AuthFilter<String, ScriptTokenPrincipal<ValueBasedRole>> createScriptTokenAuthFilter(
-            TeletraanServiceContext context) throws Exception {
+            TeletraanServiceContext context) {
         Authenticator<String, ScriptTokenPrincipal<ValueBasedRole>> scriptTokenAuthenticator =
                 new ScriptTokenAuthenticator<>(new TeletraanScriptTokenProvider(context));
         if (StringUtils.isNotBlank(getTokenCacheSpec())) {
@@ -117,17 +117,17 @@ AuthFilter<String, ScriptTokenPrincipal<ValueBasedRole>> createScriptTokenAuthFi
                 .setAuthenticator(scriptTokenAuthenticator)
                 .setAuthorizer(
                         (Authorizer<ScriptTokenPrincipal<ValueBasedRole>>)
-                                context.getAuthorizationFactory()
-                                        .create(context, ServicePrincipal.class))
+                                (Authorizer<?>)
+                                        context.getAuthorizationFactory()
+                                                .create(context, ScriptTokenPrincipal.class))
                 .setPrefix("token")
                 .setUnauthorizedHandler(new JSONUnauthorizedHandler())
                 .buildAuthFilter();
     }
 
     // TODO: CDP-7837 remove this after all the clients are updated to use the new token scheme
-    @SuppressWarnings({"unchecked"})
     AuthFilter<String, UserPrincipal> createOauthTokenAuthFilter(TeletraanServiceContext context)
-            throws Exception {
+            throws MalformedURLException {
         Authenticator<String, UserPrincipal> oauthAuthenticator =
                 new OAuthAuthenticator(getUserDataUrl(), getGroupDataUrl());
         if (StringUtils.isNotBlank(getTokenCacheSpec())) {
@@ -140,17 +140,14 @@ AuthFilter<String, UserPrincipal> createOauthTokenAuthFilter(TeletraanServiceCon
         return new OAuthCredentialAuthFilter.Builder<UserPrincipal>()
                 .setAuthenticator(oauthAuthenticator)
                 .setAuthorizer(
-                        (Authorizer<UserPrincipal>)
-                                context.getAuthorizationFactory()
-                                        .create(context, UserPrincipal.class))
+                        context.getAuthorizationFactory().create(context, UserPrincipal.class))
                 .setPrefix("token")
                 .setUnauthorizedHandler(new JSONUnauthorizedHandler())
                 .buildAuthFilter();
     }
 
-    @SuppressWarnings({"unchecked"})
     AuthFilter<String, UserPrincipal> createJwtTokenAuthFilter(TeletraanServiceContext context)
-            throws Exception {
+            throws MalformedURLException {
         Authenticator<String, UserPrincipal> oauthJwtAuthenticator =
                 new OAuthAuthenticator(getUserDataUrl(), getGroupDataUrl());
         if (StringUtils.isNotBlank(getTokenCacheSpec())) {
@@ -163,9 +160,7 @@ AuthFilter<String, UserPrincipal> createJwtTokenAuthFilter(TeletraanServiceConte
         return new OAuthCredentialAuthFilter.Builder<UserPrincipal>()
                 .setAuthenticator(oauthJwtAuthenticator)
                 .setAuthorizer(
-                        (Authorizer<UserPrincipal>)
-                                context.getAuthorizationFactory()
-                                        .create(context, UserPrincipal.class))
+                        context.getAuthorizationFactory().create(context, UserPrincipal.class))
                 .setPrefix("Bearer")
                 .setUnauthorizedHandler(new JSONUnauthorizedHandler())
                 .buildAuthFilter();

deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/resource/EnvCapacities.java

@@ -233,8 +233,7 @@ void authorize(
             EnvironBean targetEnvironBean,
             Principal principal,
             CapacityType capacityType,
-            List<String> capacities)
-            throws Exception {
+            List<String> capacities) {
         if (isSidecarEnvironment(targetEnvironBean)) {
             // Allow sidecars to add capacity
             return;
@@ -243,15 +242,15 @@ void authorize(
         if (!(principal instanceof TeletraanPrincipal)) {
             throw new UnsupportedOperationException("Only TeletraanPrincipal is allowed");
         }
-        HashSet<AuthZResource> resources = getCapacityMainEnvironments(capacityType, capacities);
+        TeletraanPrincipal teletraanPrincipal = (TeletraanPrincipal) principal;
+        TeletraanAuthorizer<TeletraanPrincipal> authorizer =
+                authorizationFactory.createSecondaryAuthorizer(
+                        context, teletraanPrincipal.getClass());
 
-        TeletraanAuthorizer<TeletraanPrincipal> authorizer = authorizationFactory.create(context);
+        HashSet<AuthZResource> resources = getCapacityMainEnvironments(capacityType, capacities);
         for (AuthZResource resource : resources) {
             if (!authorizer.authorize(
-                    (TeletraanPrincipal) principal,
-                    TeletraanPrincipalRole.Names.WRITE,
-                    resource,
-                    null)) {
+                    teletraanPrincipal, TeletraanPrincipalRole.Names.WRITE, resource, null)) {
                 throw new ForbiddenException(
                         String.format(
                                 "Principal %s is not allowed to modify capacity owned by env %s",

deploy-service/teletraanservice/src/test/java/com/pinterest/teletraan/config/CompositeAuthorizationFactoryTest.java

@@ -20,24 +20,81 @@
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import com.pinterest.teletraan.TeletraanServiceContext;
+import com.pinterest.teletraan.security.ScriptTokenRoleAuthorizer;
 import com.pinterest.teletraan.security.TeletraanAuthZResourceExtractorFactory;
 import com.pinterest.teletraan.universal.security.BasePastisAuthorizer;
+import com.pinterest.teletraan.universal.security.DenyAllAuthorizer;
+import com.pinterest.teletraan.universal.security.TeletraanAuthorizer;
+import com.pinterest.teletraan.universal.security.bean.ScriptTokenPrincipal;
+import com.pinterest.teletraan.universal.security.bean.ServicePrincipal;
+import com.pinterest.teletraan.universal.security.bean.TeletraanPrincipal;
+import com.pinterest.teletraan.universal.security.bean.UserPrincipal;
 import io.dropwizard.auth.Authorizer;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 class CompositeAuthorizationFactoryTest {
-    @Test
-    void testCreate() throws Exception {
-        TeletraanServiceContext context = new TeletraanServiceContext();
+    private TeletraanServiceContext context;
+    private CompositeAuthorizationFactory sut;
+
+    @BeforeEach
+    void setUp() {
+        context = new TeletraanServiceContext();
         context.setAuthZResourceExtractorFactory(
                 new TeletraanAuthZResourceExtractorFactory(context));
-        CompositeAuthorizationFactory factory = new CompositeAuthorizationFactory();
+        sut = new CompositeAuthorizationFactory();
+    }
 
-        Authorizer<?> authorizer = factory.create(context);
+    @Test
+    void testCreate() {
+        Authorizer<?> authorizer = sut.create(context);
         assertNotNull(authorizer);
         assertTrue(authorizer instanceof BasePastisAuthorizer);
 
-        Authorizer<?> authorizer2 = factory.create(context);
+        Authorizer<?> authorizer2 = sut.create(context);
         assertSame(authorizer, authorizer2);
     }
+
+    @Test
+    void testCreateWithNullPrincipalClass() {
+        Authorizer<?> authorizer = sut.create(context, null);
+        assertNotNull(authorizer);
+        assertTrue(authorizer instanceof BasePastisAuthorizer);
+    }
+
+    @Test
+    void testCreateWithScriptTokenPrincipalClass() {
+        Authorizer<?> authorizer = sut.create(context, ScriptTokenPrincipal.class);
+        assertTrue(authorizer instanceof ScriptTokenRoleAuthorizer);
+    }
+
+    @Test
+    void testCreateWithUserPrincipalClass() {
+        Authorizer<?> authorizer = sut.create(context, UserPrincipal.class);
+        assertTrue(authorizer instanceof BasePastisAuthorizer);
+    }
+
+    @Test
+    void testCreateSecondaryAuthorizerWithNullPrincipalClass() {
+        TeletraanAuthorizer<TeletraanPrincipal> authorizer =
+                sut.createSecondaryAuthorizer(context, null);
+        assertNotNull(authorizer);
+        assertTrue(authorizer instanceof BasePastisAuthorizer);
+    }
+
+    @Test
+    void testCreateSecondaryAuthorizerWithScriptTokenPrincipalClass() {
+        TeletraanPrincipal scriptTokenPrincipal = new ScriptTokenPrincipal<>(null, null, null);
+        TeletraanAuthorizer<TeletraanPrincipal> authorizer =
+                sut.createSecondaryAuthorizer(context, scriptTokenPrincipal.getClass());
+        assertTrue(authorizer instanceof DenyAllAuthorizer);
+    }
+
+    @Test
+    void testCreateSecondaryAuthorizerWithServicePrincipalClass() {
+        TeletraanPrincipal servicePrincipal = new ServicePrincipal("");
+        TeletraanAuthorizer<TeletraanPrincipal> authorizer =
+                sut.createSecondaryAuthorizer(context, servicePrincipal.getClass());
+        assertTrue(authorizer instanceof BasePastisAuthorizer);
+    }
 }