Skip to content

Commit

Permalink
fix(api) owner with readonly access should be able to sub to on_delet…
Browse files Browse the repository at this point in the history 
…e, on_create, on_update (#807)

Co-authored-by: Salton Arthur Massally <salton.massally@gmail.com>
  • Loading branch information
@richardmcclellan @saltonmassally
richardmcclellan and saltonmassally authored Sep 8, 2020
1 parent 55077a7 commit 821871e
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 55 deletions.
23 changes: 4 additions & 19 deletions aws-api/src/main/java/com/amplifyframework/api/aws/AWSApiPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@
import com.amplifyframework.api.graphql.GraphQLOperation;
import com.amplifyframework.api.graphql.GraphQLRequest;
import com.amplifyframework.api.graphql.GraphQLResponse;
import com.amplifyframework.api.graphql.Operation;
import com.amplifyframework.api.graphql.SubscriptionType;
import com.amplifyframework.api.rest.HttpMethod;
import com.amplifyframework.api.rest.RestOperation;
import com.amplifyframework.api.rest.RestOperationRequest;
Expand All @@ -56,7 +54,6 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
Expand Down Expand Up @@ -288,7 +285,7 @@ public <R> GraphQLOperation<R> subscribe(
try {
AppSyncGraphQLRequest<R> appSyncRequest = (AppSyncGraphQLRequest<R>) request;
for (AuthRule authRule : appSyncRequest.getModelSchema().getAuthRules()) {
if (isOwnerArgumentRequired(authRule, appSyncRequest.getOperation())) {
if (isOwnerArgumentRequired(authRule)) {
request = appSyncRequest.newBuilder()
.variable(authRule.getOwnerFieldOrDefault(), "String!", getUsername())
.build();
Expand All @@ -315,21 +312,9 @@ public <R> GraphQLOperation<R> subscribe(
return operation;
}

private boolean isOwnerArgumentRequired(AuthRule authRule, Operation operation) {
if (!AuthStrategy.OWNER.equals(authRule.getAuthStrategy())) {
return false;
}
List<ModelOperation> operations = authRule.getOperationsOrDefault();
if (SubscriptionType.ON_CREATE.equals(operation) && operations.contains(ModelOperation.CREATE)) {
return true;
}
if (SubscriptionType.ON_UPDATE.equals(operation) && operations.contains(ModelOperation.UPDATE)) {
return true;
}
if (SubscriptionType.ON_DELETE.equals(operation) && operations.contains(ModelOperation.DELETE)) {
return true;
}
return false;
private boolean isOwnerArgumentRequired(AuthRule authRule) {
return AuthStrategy.OWNER.equals(authRule.getAuthStrategy())
&& authRule.getOperationsOrDefault().contains(ModelOperation.READ);
}

private String getUsername() throws ApiException {
Expand Down
46 changes: 10 additions & 36 deletions aws-api/src/test/java/com/amplifyframework/api/aws/AWSApiPluginTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,8 @@
import com.amplifyframework.api.graphql.GraphQLOperation;
import com.amplifyframework.api.graphql.GraphQLRequest;
import com.amplifyframework.api.graphql.GraphQLResponse;
import com.amplifyframework.api.graphql.MutationType;
import com.amplifyframework.api.graphql.Operation;
import com.amplifyframework.api.graphql.PaginatedResult;
import com.amplifyframework.api.graphql.QueryType;
import com.amplifyframework.api.graphql.SubscriptionType;
import com.amplifyframework.api.graphql.model.ModelMutation;
import com.amplifyframework.api.graphql.model.ModelPagination;
Expand Down Expand Up @@ -294,33 +292,19 @@ public void ownerArgumentIsAddedAndSerializedInRequest() throws JSONException {
}

/**
* Verify that owner argument is required for ON_CREATE subscription if ModelOperation.CREATE is specified.
* Verify that owner argument is required for all subscriptions if ModelOperation.READ is specified.
* @throws AmplifyException if a ModelSchema can't be derived from the Model class.
*/
@Test
public void ownerArgumentAddedForOnCreate() throws AmplifyException {
assertTrue(isOwnerArgumentAdded(Owner.class, SubscriptionType.ON_CREATE));
assertTrue(isOwnerArgumentAdded(OwnerCreate.class, SubscriptionType.ON_CREATE));
}

/**
* Verify that owner argument is required for ON_UPDATE subscription if ModelOperation.UPDATE is specified.
* @throws AmplifyException if a ModelSchema can't be derived from the Model class.
*/
@Test
public void ownerArgumentAddedForOnUpdate() throws AmplifyException {
public void ownerArgumentAddedForRestrictedRead() throws AmplifyException {
assertTrue(isOwnerArgumentAdded(Owner.class, SubscriptionType.ON_UPDATE));
assertTrue(isOwnerArgumentAdded(OwnerUpdate.class, SubscriptionType.ON_UPDATE));
}
assertTrue(isOwnerArgumentAdded(OwnerRead.class, SubscriptionType.ON_UPDATE));

/**
* Verify that owner argument is required for ON_DELETE subscription if ModelOperation.DELETE is specified.
* @throws AmplifyException if a ModelSchema can't be derived from the Model class.
*/
@Test
public void ownerArgumentAddedForOnDelete() throws AmplifyException {
assertTrue(isOwnerArgumentAdded(Owner.class, SubscriptionType.ON_DELETE));
assertTrue(isOwnerArgumentAdded(OwnerDelete.class, SubscriptionType.ON_DELETE));
assertTrue(isOwnerArgumentAdded(OwnerRead.class, SubscriptionType.ON_DELETE));

assertTrue(isOwnerArgumentAdded(Owner.class, SubscriptionType.ON_CREATE));
assertTrue(isOwnerArgumentAdded(OwnerRead.class, SubscriptionType.ON_CREATE));
}

/**
Expand All @@ -330,14 +314,14 @@ public void ownerArgumentAddedForOnDelete() throws AmplifyException {
@Test
public void ownerArgumentNotAddedIfOperationNotRestricted() throws AmplifyException {
assertFalse(isOwnerArgumentAdded(OwnerCreate.class, SubscriptionType.ON_UPDATE));
assertFalse(isOwnerArgumentAdded(OwnerRead.class, SubscriptionType.ON_UPDATE));
assertFalse(isOwnerArgumentAdded(OwnerUpdate.class, SubscriptionType.ON_UPDATE));
assertFalse(isOwnerArgumentAdded(OwnerDelete.class, SubscriptionType.ON_UPDATE));

assertFalse(isOwnerArgumentAdded(OwnerCreate.class, SubscriptionType.ON_DELETE));
assertFalse(isOwnerArgumentAdded(OwnerRead.class, SubscriptionType.ON_DELETE));
assertFalse(isOwnerArgumentAdded(OwnerUpdate.class, SubscriptionType.ON_DELETE));
assertFalse(isOwnerArgumentAdded(OwnerDelete.class, SubscriptionType.ON_DELETE));

assertFalse(isOwnerArgumentAdded(OwnerRead.class, SubscriptionType.ON_CREATE));
assertFalse(isOwnerArgumentAdded(OwnerCreate.class, SubscriptionType.ON_CREATE));
assertFalse(isOwnerArgumentAdded(OwnerUpdate.class, SubscriptionType.ON_CREATE));
assertFalse(isOwnerArgumentAdded(OwnerDelete.class, SubscriptionType.ON_CREATE));
}
Expand All @@ -351,16 +335,6 @@ public void ownerArgumentNotAddedIfNotOwnerStrategy() throws AmplifyException {
assertFalse(isOwnerArgumentAdded(Group.class, SubscriptionType.ON_CREATE));
}

/**
* Verify owner argument NOT added for Query or Mutation operations.
* @throws AmplifyException if a ModelSchema can't be derived from the Model class.
*/
@Test
public void verifyOwnerArgumentNotAddedIfNotSubscriptionOperation() throws AmplifyException {
assertFalse(isOwnerArgumentAdded(Owner.class, QueryType.GET));
assertFalse(isOwnerArgumentAdded(Owner.class, MutationType.CREATE));
}

private boolean isOwnerArgumentAdded(Class<? extends Model> clazz, Operation operation)
throws AmplifyException {
AppSyncGraphQLRequest<Model> request = AppSyncGraphQLRequest.builder()
Expand Down

0 comments on commit 821871e

Please sign in to comment.