Don't try to subscribe with owner rule when using a different authType

aws-amplify · May 22, 2024 · 9d9703d · 9d9703d
1 parent 3d4071a
commit 9d9703d
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 2 deletions.
diff --git a/aws-api/src/main/java/com/amplifyframework/api/aws/auth/AuthRuleRequestDecorator.java b/aws-api/src/main/java/com/amplifyframework/api/aws/auth/AuthRuleRequestDecorator.java
@@ -83,13 +83,16 @@ public <R> GraphQLRequest<R> decorate(
         AppSyncGraphQLRequest<R> appSyncRequest = (AppSyncGraphQLRequest<R>) request;
         AuthRule ownerRuleWithReadRestriction = null;
         Map<String, Set<String>> readAuthorizedGroupsMap = new HashMap<>();
+        boolean subscribeAllowedForNonOwner = false;
 
         // Note that we are intentionally supporting only one owner rule with a READ operation at this time.
         // If there is more than one, the operation will fail because AppSync generates a parameter for each
         // one. The question then is which one do we pass. JavaScript currently doesn't support this use case
         // and it's not clear what a good solution would be until AppSync supports real time filters.
         for (AuthRule authRule : appSyncRequest.getModelSchema().getAuthRules()) {
-            if (isReadRestrictingOwner(authRule)) {
+            if (doesRuleAllowNonOwnerSubscribe(authRule, authType)) {
+                subscribeAllowedForNonOwner = true;
+            } else if (isReadRestrictingOwner(authRule)) {
                 if (ownerRuleWithReadRestriction == null) {
                     ownerRuleWithReadRestriction = authRule;
                 } else {
@@ -114,7 +117,8 @@ public <R> GraphQLRequest<R> decorate(
         // We only add the owner parameter to the subscription if there is an owner rule with a READ restriction
         // and either there are no group auth rules with read access or there are but the user isn't in any of
         // them.
-        if (ownerRuleWithReadRestriction != null
+        if (!subscribeAllowedForNonOwner &&
+            ownerRuleWithReadRestriction != null
                 && userNotInReadRestrictingGroups(readAuthorizedGroupsMap, authType)) {
             String idClaim = ownerRuleWithReadRestriction.getIdentityClaimOrDefault();
             String key = ownerRuleWithReadRestriction.getOwnerFieldOrDefault();
@@ -135,6 +139,17 @@ && userNotInReadRestrictingGroups(readAuthorizedGroupsMap, authType)) {
         return request;
     }
 
+    private boolean doesRuleAllowNonOwnerSubscribe(AuthRule authRule, AuthorizationType authMode) {
+        AuthorizationType typeForRule = AuthorizationType.from(authRule.getAuthProvider());
+        AuthStrategy strategy = authRule.getAuthStrategy();
+        List<ModelOperation> operations = authRule.getOperationsOrDefault();
+        return strategy != AuthStrategy.OWNER && strategy != AuthStrategy.GROUPS
+            && typeForRule != AuthorizationType.AMAZON_COGNITO_USER_POOLS
+            && typeForRule != AuthorizationType.OPENID_CONNECT
+            && typeForRule == authMode
+            && (operations.contains(ModelOperation.LISTEN) || operations.contains(ModelOperation.READ));
+    }
+
     private boolean isReadRestrictingOwner(AuthRule authRule) {
         return AuthStrategy.OWNER.equals(authRule.getAuthStrategy())
                 && authRule.getOperationsOrDefault().contains(ModelOperation.READ);

diff --git a/aws-api/src/test/java/com/amplifyframework/api/aws/auth/AuthRuleRequestDecoratorTest.java b/aws-api/src/test/java/com/amplifyframework/api/aws/auth/AuthRuleRequestDecoratorTest.java
@@ -293,6 +293,66 @@ public void ownerArgumentNotAddedIfOwnerIsInCustomGroup() throws AmplifyExceptio
         }
     }
 
+    /**
+     * Verify owner argument is NOT added if model contains both public key and owner-based authorization and the
+     * requested auth type is API_KEY.
+     * @throws AmplifyException if a ModelSchema can't be derived from the Model class.
+     */
+    @Test
+    public void doesNotAddOwnerWhenMultiAuthWithPublicKey() throws AmplifyException {
+        final AuthorizationType mode = AuthorizationType.API_KEY;
+
+        // PublicAndOwner combines public and owner-based auth
+        for (SubscriptionType subscriptionType : SubscriptionType.values()) {
+            GraphQLRequest<PublicAndOwner> originalRequest = createRequest(PublicAndOwner.class, subscriptionType);
+            GraphQLRequest<PublicAndOwner> modifiedRequest = decorator.decorate(originalRequest, mode);
+            assertNull(getOwnerField(modifiedRequest));
+        }
+
+        // PublicAndOwnerOidc combines public and owner-based auth with an OIDC claim
+        for (SubscriptionType subscriptionType : SubscriptionType.values()) {
+            GraphQLRequest<PublicAndOwnerOidc> originalRequest =
+                createRequest(PublicAndOwnerOidc.class, subscriptionType);
+            GraphQLRequest<PublicAndOwnerOidc> modifiedRequest = decorator.decorate(originalRequest, mode);
+            assertNull(getOwnerField(modifiedRequest));
+        }
+    }
+
+    /**
+     * Verify owner argument is added if model contains both owner-based and public-key
+     * authorization and the auth mode is cognito.
+     * @throws AmplifyException if a ModelSchema can't be derived from the Model class.
+     */
+    @Test
+    public void addsOwnerWhenMultiAuthWithCognito() throws AmplifyException {
+        final AuthorizationType mode = AuthorizationType.AMAZON_COGNITO_USER_POOLS;
+        final String expectedOwner = FakeCognitoAuthProvider.USERNAME;
+
+        for (SubscriptionType subscriptionType : SubscriptionType.values()) {
+            GraphQLRequest<PublicAndOwner> originalRequest = createRequest(PublicAndOwner.class, subscriptionType);
+            GraphQLRequest<PublicAndOwner> modifiedRequest = decorator.decorate(originalRequest, mode);
+            assertEquals(expectedOwner, getOwnerField(modifiedRequest));
+        }
+    }
+
+    /**
+     * Verify owner argument is added if model contains both owner-based and public-key
+     * authorization and the auth mode is oidc.
+     * @throws AmplifyException if a ModelSchema can't be derived from the Model class.
+     */
+    @Test
+    public void addsOwnerWhenMultiAuthWithOidc() throws AmplifyException {
+        final AuthorizationType mode = AuthorizationType.OPENID_CONNECT;
+        final String expectedOwner = FakeOidcAuthProvider.SUB;
+
+        for (SubscriptionType subscriptionType : SubscriptionType.values()) {
+            GraphQLRequest<PublicAndOwnerOidc> originalRequest =
+                createRequest(PublicAndOwnerOidc.class, subscriptionType);
+            GraphQLRequest<PublicAndOwnerOidc> modifiedRequest = decorator.decorate(originalRequest, mode);
+            assertEquals(expectedOwner, getOwnerField(modifiedRequest));
+        }
+    }
+
     private <M extends Model> String getOwnerField(GraphQLRequest<M> request) {
         if (request.getVariables().containsKey("owner")) {
             return (String) request.getVariables().get("owner");
@@ -412,4 +472,16 @@ private abstract static class OwnerInCustomGroup implements Model {}
         )
     })
     private abstract static class OwnerNotInCustomGroup implements Model {}
+
+    @ModelConfig(authRules = {
+        @AuthRule(allow = AuthStrategy.PUBLIC, operations = ModelOperation.READ),
+        @AuthRule(allow = AuthStrategy.OWNER)
+    })
+    private abstract static class PublicAndOwner implements Model {}
+
+    @ModelConfig(authRules = {
+        @AuthRule(allow = AuthStrategy.PUBLIC, operations = ModelOperation.READ),
+        @AuthRule(allow = AuthStrategy.OWNER, identityClaim = "sub")
+    })
+    private abstract static class PublicAndOwnerOidc implements Model {}
 }