From 14aa01d9eaae212a006488cab2103ffa5bfc4fe9 Mon Sep 17 00:00:00 2001 From: John Woo Date: Wed, 30 Sep 2020 15:10:16 -0700 Subject: [PATCH] fix(DataStore): owner based auth, read operations --- .../Model/Decorator/AuthRuleDecorator.swift | 25 +++---------------- .../ModelMultipleOwnerAuthRuleTests.swift | 8 +++--- .../ModelReadUpdateAuthRuleTests.swift | 20 +++++---------- .../GraphQLRequestAuthRuleTests.swift | 20 ++++++++++----- 4 files changed, 28 insertions(+), 45 deletions(-) diff --git a/AmplifyPlugins/Core/AWSPluginsCore/Model/Decorator/AuthRuleDecorator.swift b/AmplifyPlugins/Core/AWSPluginsCore/Model/Decorator/AuthRuleDecorator.swift index 15ec120a33..1bf066142a 100644 --- a/AmplifyPlugins/Core/AWSPluginsCore/Model/Decorator/AuthRuleDecorator.swift +++ b/AmplifyPlugins/Core/AWSPluginsCore/Model/Decorator/AuthRuleDecorator.swift @@ -60,12 +60,11 @@ public struct AuthRuleDecorator: ModelBasedGraphQLDocumentDecorator { let ownerField = authRule.getOwnerFieldOrDefault() selectionSet = appendOwnerFieldToSelectionSetIfNeeded(selectionSet: selectionSet, ownerField: ownerField) - guard case let .subscription(subscriptionType, ownerId) = input else { + guard case let .subscription(_, ownerId) = input else { return document.copy(selectionSet: selectionSet) } - let operations = authRule.getModelOperationsOrDefault() - if isOwnerInputRequiredOnSubscription(subscriptionType, operations: operations) { + if isOwnerInputRequiredOnSubscription(authRule) { var inputs = document.inputs inputs[ownerField] = GraphQLDocumentInput(type: "String!", value: .scalar(ownerId)) return document.copy(inputs: inputs, selectionSet: selectionSet) @@ -73,24 +72,8 @@ public struct AuthRuleDecorator: ModelBasedGraphQLDocumentDecorator { return document.copy(selectionSet: selectionSet) } - private func isOwnerInputRequiredOnSubscription(_ subscriptionType: GraphQLSubscriptionType, - operations: [ModelOperation]) -> Bool { - var isOwnerInputRequired = false - switch subscriptionType { - case .onCreate: - if operations.contains(.create) { - isOwnerInputRequired = true - } - case .onUpdate: - if operations.contains(.update) { - isOwnerInputRequired = true - } - case .onDelete: - if operations.contains(.delete) { - isOwnerInputRequired = true - } - } - return isOwnerInputRequired + private func isOwnerInputRequiredOnSubscription(_ authRule: AuthRule) -> Bool { + return authRule.allow == .owner && authRule.getModelOperationsOrDefault().contains(.read) } /// First finds the first `model` SelectionSet. Then, only append it when the `ownerField` does not exist. diff --git a/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/Decorator/AuthRuleDecorator/ModelMultipleOwnerAuthRuleTests.swift b/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/Decorator/AuthRuleDecorator/ModelMultipleOwnerAuthRuleTests.swift index e4628e968b..b9f04e929d 100644 --- a/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/Decorator/AuthRuleDecorator/ModelMultipleOwnerAuthRuleTests.swift +++ b/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/Decorator/AuthRuleDecorator/ModelMultipleOwnerAuthRuleTests.swift @@ -220,8 +220,8 @@ class ModelMultipleOwnerAuthRuleTests: XCTestCase { documentBuilder.add(decorator: AuthRuleDecorator(.subscription(.onCreate, "111"))) let document = documentBuilder.build() let expectedQueryDocument = """ - subscription OnCreateModelMultipleOwner($owner: String!) { - onCreateModelMultipleOwner(owner: $owner) { + subscription OnCreateModelMultipleOwner($editors: String!, $owner: String!) { + onCreateModelMultipleOwner(editors: $editors, owner: $owner) { id content editors @@ -275,8 +275,8 @@ class ModelMultipleOwnerAuthRuleTests: XCTestCase { documentBuilder.add(decorator: AuthRuleDecorator(.subscription(.onDelete, "111"))) let document = documentBuilder.build() let expectedQueryDocument = """ - subscription OnDeleteModelMultipleOwner($owner: String!) { - onDeleteModelMultipleOwner(owner: $owner) { + subscription OnDeleteModelMultipleOwner($editors: String!, $owner: String!) { + onDeleteModelMultipleOwner(editors: $editors, owner: $owner) { id content editors diff --git a/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/Decorator/AuthRuleDecorator/ModelReadUpdateAuthRuleTests.swift b/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/Decorator/AuthRuleDecorator/ModelReadUpdateAuthRuleTests.swift index 203c2bd766..3e0232911c 100644 --- a/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/Decorator/AuthRuleDecorator/ModelReadUpdateAuthRuleTests.swift +++ b/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/Decorator/AuthRuleDecorator/ModelReadUpdateAuthRuleTests.swift @@ -205,8 +205,8 @@ class ModelReadUpdateAuthRuleTests: XCTestCase { documentBuilder.add(decorator: AuthRuleDecorator(.subscription(.onCreate, "111"))) let document = documentBuilder.build() let expectedQueryDocument = """ - subscription OnCreateModelReadUpdateField($owner: String!) { - onCreateModelReadUpdateField(owner: $owner) { + subscription OnCreateModelReadUpdateField { + onCreateModelReadUpdateField { id content __typename @@ -216,11 +216,7 @@ class ModelReadUpdateAuthRuleTests: XCTestCase { """ XCTAssertEqual(document.name, "onCreateModelReadUpdateField") XCTAssertEqual(document.stringValue, expectedQueryDocument) - guard let variables = document.variables else { - XCTFail("The document doesn't contain variables") - return - } - XCTAssertEqual(variables["owner"] as? String, "111") + XCTAssert(document.variables.isEmpty) } // Others can `.update` this model, which means the update subscription does not require owner input @@ -252,8 +248,8 @@ class ModelReadUpdateAuthRuleTests: XCTestCase { documentBuilder.add(decorator: AuthRuleDecorator(.subscription(.onDelete, "111"))) let document = documentBuilder.build() let expectedQueryDocument = """ - subscription OnDeleteModelReadUpdateField($owner: String!) { - onDeleteModelReadUpdateField(owner: $owner) { + subscription OnDeleteModelReadUpdateField { + onDeleteModelReadUpdateField { id content __typename @@ -263,11 +259,7 @@ class ModelReadUpdateAuthRuleTests: XCTestCase { """ XCTAssertEqual(document.name, "onDeleteModelReadUpdateField") XCTAssertEqual(document.stringValue, expectedQueryDocument) - guard let variables = document.variables else { - XCTFail("The document doesn't contain variables") - return - } - XCTAssertEqual(variables["owner"] as? String, "111") + XCTAssert(document.variables.isEmpty) } } diff --git a/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLRequest/GraphQLRequestAuthRuleTests.swift b/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLRequest/GraphQLRequestAuthRuleTests.swift index 0676a67f32..e88e2a96fc 100644 --- a/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLRequest/GraphQLRequestAuthRuleTests.swift +++ b/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLRequest/GraphQLRequestAuthRuleTests.swift @@ -228,8 +228,8 @@ class GraphQLRequestAuthRuleTests: XCTestCase { documentBuilder.add(decorator: AuthRuleDecorator(.subscription(.onUpdate, "111"))) let document = documentBuilder.build() let documentStringValue = """ - subscription OnUpdateArticle { - onUpdateArticle { + subscription OnUpdateArticle($owner: String!) { + onUpdateArticle(owner: $owner) { id authorNotes content @@ -249,7 +249,11 @@ class GraphQLRequestAuthRuleTests: XCTestCase { XCTAssertEqual(document.stringValue, request.document) XCTAssertEqual(documentStringValue, request.document) XCTAssert(request.responseType == MutationSyncResult.self) - XCTAssertNil(request.variables) + guard let variables = document.variables else { + XCTFail("The document doesn't contain variables") + return + } + XCTAssertEqual(variables["owner"] as? String, "111") } func testOnDeleteSubscriptionGraphQLRequest() throws { @@ -260,8 +264,8 @@ class GraphQLRequestAuthRuleTests: XCTestCase { documentBuilder.add(decorator: AuthRuleDecorator(.subscription(.onDelete, "111"))) let document = documentBuilder.build() let documentStringValue = """ - subscription OnDeleteArticle { - onDeleteArticle { + subscription OnDeleteArticle($owner: String!) { + onDeleteArticle(owner: $owner) { id authorNotes content @@ -281,7 +285,11 @@ class GraphQLRequestAuthRuleTests: XCTestCase { XCTAssertEqual(document.stringValue, request.document) XCTAssertEqual(documentStringValue, request.document) XCTAssert(request.responseType == MutationSyncResult.self) - XCTAssertNil(request.variables) + guard let variables = document.variables else { + XCTFail("The document doesn't contain variables") + return + } + XCTAssertEqual(variables["owner"] as? String, "111") } func testSyncQueryGraphQLRequest() throws {