Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(DataStore): Owner and Group Combined @auth #817

Merged
merged 3 commits into from
Oct 15, 2020
Merged

fix(DataStore): Owner and Group Combined @auth #817

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6add65e
fix(DataStore): Owner and Group Combined @auth
wooj2 Oct 10, 2020
02191cc
adding unit tests
wooj2 Oct 12, 2020
8f8eea6
updating per pr comments
wooj2 Oct 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions Amplify.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,11 @@
6BBECD7123ADA7E100C8DFBE /* AmplifyAWSServiceConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6BBECD7023ADA7E100C8DFBE /* AmplifyAWSServiceConfiguration.swift */; };
6BBECD7423ADA9D100C8DFBE /* AmplifyAWSServiceConfigurationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6BBECD7323ADA9D100C8DFBE /* AmplifyAWSServiceConfigurationTests.swift */; };
6BEE0817253114FD00133961 /* AWSAuthServiceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6BEE0816253114FD00133961 /* AWSAuthServiceTests.swift */; };
6BEE08192533CAA600133961 /* GraphQLRequestOwnerAndGroupTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6BEE08182533CAA600133961 /* GraphQLRequestOwnerAndGroupTests.swift */; };
6BEE081C2533CCFA00133961 /* OGCScenarioBPost+Schema.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6BEE081A2533CCFA00133961 /* OGCScenarioBPost+Schema.swift */; };
6BEE081D2533CCFA00133961 /* OGCScenarioBPost.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6BEE081B2533CCFA00133961 /* OGCScenarioBPost.swift */; };
6BEE08242533D30800133961 /* OGCScenarioBMGroupPost.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6BEE08222533D30800133961 /* OGCScenarioBMGroupPost.swift */; };
6BEE08252533D30800133961 /* OGCScenarioBMGroupPost+Schema.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6BEE08232533D30800133961 /* OGCScenarioBMGroupPost+Schema.swift */; };
7D5ED6C78E25246DDAF2F2EC /* Pods_Amplify.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 84F3A76FB68CEFA45F4BB1BB /* Pods_Amplify.framework */; platformFilter = ios; };
7F27B1DCE59C1E674172CCD6 /* Pods_Amplify_AmplifyTestConfigs_AmplifyTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 976D972EC2BBCAAD023694EB /* Pods_Amplify_AmplifyTestConfigs_AmplifyTests.framework */; };
881246F5DCC59436DC932469 /* Pods_Amplify_AWSPluginsCore.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 35D92182B8445C8F9B0FAE94 /* Pods_Amplify_AWSPluginsCore.framework */; };
Expand Down Expand Up @@ -859,6 +864,11 @@
6BBECD7023ADA7E100C8DFBE /* AmplifyAWSServiceConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AmplifyAWSServiceConfiguration.swift; sourceTree = "<group>"; };
6BBECD7323ADA9D100C8DFBE /* AmplifyAWSServiceConfigurationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AmplifyAWSServiceConfigurationTests.swift; sourceTree = "<group>"; };
6BEE0816253114FD00133961 /* AWSAuthServiceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AWSAuthServiceTests.swift; sourceTree = "<group>"; };
6BEE08182533CAA600133961 /* GraphQLRequestOwnerAndGroupTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GraphQLRequestOwnerAndGroupTests.swift; sourceTree = "<group>"; };
6BEE081A2533CCFA00133961 /* OGCScenarioBPost+Schema.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "OGCScenarioBPost+Schema.swift"; sourceTree = "<group>"; };
6BEE081B2533CCFA00133961 /* OGCScenarioBPost.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OGCScenarioBPost.swift; sourceTree = "<group>"; };
6BEE08222533D30800133961 /* OGCScenarioBMGroupPost.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OGCScenarioBMGroupPost.swift; sourceTree = "<group>"; };
6BEE08232533D30800133961 /* OGCScenarioBMGroupPost+Schema.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "OGCScenarioBMGroupPost+Schema.swift"; sourceTree = "<group>"; };
6C41D3730B7ED4FD62A43E40 /* Pods-Amplify-AmplifyAWSPlugins-AWSAPICategoryPlugin-AWSAPICategoryPluginTests.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Amplify-AmplifyAWSPlugins-AWSAPICategoryPlugin-AWSAPICategoryPluginTests.debug.xcconfig"; path = "Target Support Files/Pods-Amplify-AmplifyAWSPlugins-AWSAPICategoryPlugin-AWSAPICategoryPluginTests/Pods-Amplify-AmplifyAWSPlugins-AWSAPICategoryPlugin-AWSAPICategoryPluginTests.debug.xcconfig"; sourceTree = "<group>"; };
6D51240C78418B733FFA6829 /* Pods-Amplify-AWSPluginsCore-AWSPluginsTestConfigs-AWSDataStoreCategoryPluginTests.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Amplify-AWSPluginsCore-AWSPluginsTestConfigs-AWSDataStoreCategoryPluginTests.debug.xcconfig"; path = "Target Support Files/Pods-Amplify-AWSPluginsCore-AWSPluginsTestConfigs-AWSDataStoreCategoryPluginTests/Pods-Amplify-AWSPluginsCore-AWSPluginsTestConfigs-AWSDataStoreCategoryPluginTests.debug.xcconfig"; sourceTree = "<group>"; };
6D62C9C57736C3BEADEB1E30 /* Pods-AWSPinpointAnalyticsPlugin.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-AWSPinpointAnalyticsPlugin.debug.xcconfig"; path = "Target Support Files/Pods-AWSPinpointAnalyticsPlugin/Pods-AWSPinpointAnalyticsPlugin.debug.xcconfig"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1588,6 +1598,7 @@
2129BE322394828B006363A1 /* GraphQLRequestModelTests.swift */,
216E45F0248E971E0035E3CE /* GraphQLRequestNonModelTests.swift */,
214F497D2486DA5000DA616C /* GraphQLRequestOptionalAssociationTests.swift */,
6BEE08182533CAA600133961 /* GraphQLRequestOwnerAndGroupTests.swift */,
6B9F7C5125267E1500F1F71C /* MockAWSAuthUser.swift */,
);
path = GraphQLRequest;
Expand Down Expand Up @@ -2327,6 +2338,10 @@
B9521831237E21B900F53237 /* Post+Schema.swift */,
2129BE002394627B006363A1 /* PostCommentModelRegistration.swift */,
B9AA09F02473CA29000E6FBB /* PostStatus.swift */,
6BEE08222533D30800133961 /* OGCScenarioBMGroupPost.swift */,
6BEE08232533D30800133961 /* OGCScenarioBMGroupPost+Schema.swift */,
6BEE081B2533CCFA00133961 /* OGCScenarioBPost.swift */,
6BEE081A2533CCFA00133961 /* OGCScenarioBPost+Schema.swift */,
B952182F237E21B900F53237 /* schema.graphql */,
6B9F7C542526864800F1F71C /* ScenarioATest6Post.swift */,
6B9F7C532526864800F1F71C /* ScenarioATest6Post+Schema.swift */,
Expand Down Expand Up @@ -4132,6 +4147,7 @@
21AD425A249C0D910016FE95 /* AnyModelTester.swift in Sources */,
2129BE3C2394828B006363A1 /* GraphQLRequestModelTests.swift in Sources */,
2183A56523EA4A8400232880 /* GraphQLListQueryTests.swift in Sources */,
6BEE08192533CAA600133961 /* GraphQLRequestOwnerAndGroupTests.swift in Sources */,
21AD425B249C0DBE0016FE95 /* AnyModelTests.swift in Sources */,
D83C5160248964780091548E /* ModelGraphQLTests.swift in Sources */,
);
Expand Down Expand Up @@ -4582,10 +4598,12 @@
B9521833237E21BA00F53237 /* Comment+Schema.swift in Sources */,
FA176ED7238503C200C5C5F9 /* HubListenerTestUtilities.swift in Sources */,
216E45EE248E914F0035E3CE /* Todo.swift in Sources */,
6BEE081D2533CCFA00133961 /* OGCScenarioBPost.swift in Sources */,
21F40A3A23A294770074678E /* TestConfigHelper.swift in Sources */,
B4BD6B3723708C6700A1F0A7 /* MockPredictionsCategoryPlugin.swift in Sources */,
FACA361C2327FC7D000E74F6 /* MockHubCategoryPlugin.swift in Sources */,
FAD3937F23820DAE00463F5E /* MockDataStoreCategoryPlugin.swift in Sources */,
6BEE08252533D30800133961 /* OGCScenarioBMGroupPost+Schema.swift in Sources */,
FACA361A2327FC69000E74F6 /* MockStorageCategoryPlugin.swift in Sources */,
B9521837237E21BA00F53237 /* Post.swift in Sources */,
B9FAA11A23879AC8009414B4 /* BookAuthor.swift in Sources */,
Expand All @@ -4610,6 +4628,7 @@
FACA361D2327FC84000E74F6 /* MockAPICategoryPlugin.swift in Sources */,
B9FAA11023878C5E009414B4 /* UserProfile.swift in Sources */,
B9AA09F12473CA29000E6FBB /* PostStatus.swift in Sources */,
6BEE081C2533CCFA00133961 /* OGCScenarioBPost+Schema.swift in Sources */,
214F497B2486D8A200DA616C /* User+Schema.swift in Sources */,
6B9F7C562526864800F1F71C /* ScenarioATest6Post.swift in Sources */,
B9FAA12023879BD0009414B4 /* BookAuthor+Schema.swift in Sources */,
Expand All @@ -4623,6 +4642,7 @@
B9FAA10E23878BF3009414B4 /* UserAccount.swift in Sources */,
214F49782486D8A200DA616C /* UserFollowing.swift in Sources */,
21F40A3C23A2952C0074678E /* AuthHelper.swift in Sources */,
6BEE08242533D30800133961 /* OGCScenarioBMGroupPost.swift in Sources */,
B4F3E9FA24314ECC00F23296 /* MockAuthCategoryPlugin.swift in Sources */,
214F49CE24898E8500DA616C /* Article+Schema.swift in Sources */,
216E460A249183230035E3CE /* Section.swift in Sources */,
Expand Down
63 changes: 57 additions & 6 deletions AmplifyPlugins/Core/AWSPluginsCore/Model/Decorator/AuthRuleDecorator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import Foundation
import Amplify

public typealias IdentityClaimsDictionary = [String: String]
public typealias IdentityClaimsDictionary = [String: AnyObject]

public enum AuthRuleDecoratorInput {
case subscription(GraphQLSubscriptionType, IdentityClaimsDictionary)
Expand Down Expand Up @@ -42,14 +42,46 @@ public struct AuthRuleDecorator: ModelBasedGraphQLDocumentDecorator {
return document
}
var decorateDocument = document
let readRestrictingOwnerRules = authRules.filter { isReadRestrictingOwner($0) }
if readRestrictingOwnerRules.count > 1 {
log.error("""
Detected multiple owner type auth rules \
with a READ operation. We currently do not support this use case. Please \
limit your type to just one owner auth rule with a READ operation restriction.
""")
return decorateDocument
}

let readAuthorizedGroups = collectReadAuthorizedGroups(authRules: authRules)

authRules.forEach { authRule in
decorateDocument = decorateIfOwnerAuthStrategy(document: decorateDocument, authRule: authRule)
decorateDocument = decorateAuthStrategy(document: decorateDocument,
authRule: authRule,
readAuthorizedGroups: readAuthorizedGroups)
}
return decorateDocument
}

func decorateIfOwnerAuthStrategy(document: SingleDirectiveGraphQLDocument,
authRule: AuthRule) -> SingleDirectiveGraphQLDocument {
private func collectReadAuthorizedGroups(authRules: AuthRules) -> Set<String> {
var readAuthorizedGroups = Set<String>()
let readRestrictingGroupRules = authRules.filter { isReadRestrictingStaticGroup($0) }
for groupRules in readRestrictingGroupRules {
groupRules.groups.forEach { group in
readAuthorizedGroups.insert(group)
}
}
return readAuthorizedGroups
}

private func isReadRestrictingStaticGroup(_ authRule: AuthRule) -> Bool {
return authRule.allow == .groups &&
!authRule.groups.isEmpty &&
authRule.getModelOperationsOrDefault().contains(.read)
}

private func decorateAuthStrategy(document: SingleDirectiveGraphQLDocument,
authRule: AuthRule,
readAuthorizedGroups: Set<String>) -> SingleDirectiveGraphQLDocument {
guard authRule.allow == .owner else {
return document
}
Expand All @@ -65,7 +97,8 @@ public struct AuthRuleDecorator: ModelBasedGraphQLDocumentDecorator {
return document.copy(selectionSet: selectionSet)
}

if isOwnerInputRequiredOnSubscription(authRule) {
if isReadRestrictingOwner(authRule) && isNotInReadAuthorizedGroup(readAuthorizedGroups,
cognitoGroupsFrom(claims: claims)) {
var inputs = document.inputs
let identityClaimValue = resolveIdentityClaimValue(identityClaim: authRule.identityClaimOrDefault(),
claims: claims)
Expand All @@ -77,10 +110,28 @@ public struct AuthRuleDecorator: ModelBasedGraphQLDocumentDecorator {
return document.copy(selectionSet: selectionSet)
}

private func isOwnerInputRequiredOnSubscription(_ authRule: AuthRule) -> Bool {
private func isReadRestrictingOwner(_ authRule: AuthRule) -> Bool {
return authRule.allow == .owner && authRule.getModelOperationsOrDefault().contains(.read)
}

private func isNotInReadAuthorizedGroup(_ readAuthorizedGroups: Set<String>,
_ cognitoGroupsFromClaims: Set<String>) -> Bool {
return (readAuthorizedGroups.isEmpty ||
readAuthorizedGroups.isDisjoint(with: cognitoGroupsFromClaims))
}

private func cognitoGroupsFrom(claims: IdentityClaimsDictionary) -> Set<String> {
var groupSet = Set<String>()
if let groups = (claims["cognito:groups"] as? NSArray) as Array? {
for group in groups {
if let groupString = group as? String {
groupSet.insert(groupString)
}
}
}
return groupSet
}

private func resolveIdentityClaimValue(identityClaim: String, claims: IdentityClaimsDictionary) -> String? {
guard let identityValue = claims[identityClaim] as? String else {
log.error("""
Expand Down
45 changes: 39 additions & 6 deletions ...SPluginsCoreTests/Model/Decorator/AuthRuleDecorator/ModelMultipleOwnerAuthRuleTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ public struct ModelMultipleOwner: Model {
}
}

/*
* AppSync service currently supports only one owner rule with a single read at this time
* therefore, we will have a single test which test that we do not support this.
*
* When we do support this feature, we will delete test case "testUnsupportedModelMultipleOwner_CreateMutation",
* and uncomment the other tests.
*/

class ModelMultipleOwnerAuthRuleTests: XCTestCase {

override func setUp() {
Expand All @@ -63,7 +71,31 @@ class ModelMultipleOwnerAuthRuleTests: XCTestCase {
override func tearDown() {
ModelRegistry.reset()
}
// This is a test case to demostrate if we attempt to use a model with multiple auth rules
// with a read operation, we effectively create a subscription without decorating it with auth.
// We should delete this use case when the AppSync service supports this use case.
func testUnsupportedModelMultipleOwner_CreateMutation() {
var documentBuilder = ModelBasedGraphQLDocumentBuilder(modelType: ModelMultipleOwner.self,
operationType: .mutation)
documentBuilder.add(decorator: DirectiveNameDecorator(type: .create))
documentBuilder.add(decorator: AuthRuleDecorator(.mutation))
let document = documentBuilder.build()
let expectedQueryDocument = """
mutation CreateModelMultipleOwner {
createModelMultipleOwner {
id
content
editors
__typename
}
}
"""
XCTAssertEqual(document.name, "createModelMultipleOwner")
XCTAssertEqual(document.stringValue, expectedQueryDocument)
XCTAssertTrue(document.variables.isEmpty)
}

/*
// Ensure that the `owner` field is added to the model fields
func testModelMultipleOwner_CreateMutation() {
var documentBuilder = ModelBasedGraphQLDocumentBuilder(modelType: ModelMultipleOwner.self,
Expand Down Expand Up @@ -214,8 +246,8 @@ class ModelMultipleOwnerAuthRuleTests: XCTestCase {

// Only the 'owner' inherently has `.create` operation, requiring the subscription operation to contain the input
func testModelMultipleOwner_OnCreateSubscription() {
let claims = ["username": "user1",
"sub": "123e4567-dead-beef-a456-426614174000"]
let claims = ["username": "user1" as AnyObject,
"sub": "123e4567-dead-beef-a456-426614174000" as AnyObject]
var documentBuilder = ModelBasedGraphQLDocumentBuilder(modelType: ModelMultipleOwner.self,
operationType: .subscription)
documentBuilder.add(decorator: DirectiveNameDecorator(type: .onCreate))
Expand Down Expand Up @@ -243,8 +275,8 @@ class ModelMultipleOwnerAuthRuleTests: XCTestCase {

// Each owner with `.update` operation requires the ownerField on the corresponding subscription operation
func testModelMultipleOwner_OnUpdateSubscription() {
let claims = ["username": "user1",
"sub": "123e4567-dead-beef-a456-426614174000"]
let claims = ["username": "user1" as AnyObject,
"sub": "123e4567-dead-beef-a456-426614174000" as AnyObject]
var documentBuilder = ModelBasedGraphQLDocumentBuilder(modelType: ModelMultipleOwner.self,
operationType: .subscription)
documentBuilder.add(decorator: DirectiveNameDecorator(type: .onUpdate))
Expand Down Expand Up @@ -273,8 +305,8 @@ class ModelMultipleOwnerAuthRuleTests: XCTestCase {

// Only the 'owner' inherently has `.delete` operation, requiring the subscription operation to contain the input
func testModelMultipleOwner_OnDeleteSubscription() {
let claims = ["username": "user1",
"sub": "123e4567-dead-beef-a456-426614174000"]
let claims = ["username": "user1" as AnyObject,
"sub": "123e4567-dead-beef-a456-426614174000" as AnyObject]
var documentBuilder = ModelBasedGraphQLDocumentBuilder(modelType: ModelMultipleOwner.self,
operationType: .subscription)
documentBuilder.add(decorator: DirectiveNameDecorator(type: .onDelete))
Expand All @@ -299,4 +331,5 @@ class ModelMultipleOwnerAuthRuleTests: XCTestCase {
}
XCTAssertEqual(variables["owner"] as? String, "user1")
}
*/
}
12 changes: 6 additions & 6 deletions .../AWSPluginsCoreTests/Model/Decorator/AuthRuleDecorator/ModelReadUpdateAuthRuleTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,8 @@ class ModelReadUpdateAuthRuleTests: XCTestCase {

// The owner auth rule contains `.create` operation, requiring the subscription operation to contain the input
func testModelReadUpdateField_OnCreateSubscription() {
let claims = ["username": "user1",
"sub": "123e4567-dead-beef-a456-426614174000"]
let claims = ["username": "user1" as AnyObject,
"sub": "123e4567-dead-beef-a456-426614174000" as AnyObject]
var documentBuilder = ModelBasedGraphQLDocumentBuilder(modelType: ModelReadUpdateField.self,
operationType: .subscription)
documentBuilder.add(decorator: DirectiveNameDecorator(type: .onCreate))
Expand All @@ -223,8 +223,8 @@ class ModelReadUpdateAuthRuleTests: XCTestCase {

// Others can `.update` this model, which means the update subscription does not require owner input
func testModelReadUpdateField_OnUpdateSubscription() {
let claims = ["username": "user1",
"sub": "123e4567-dead-beef-a456-426614174000"]
let claims = ["username": "user1" as AnyObject,
"sub": "123e4567-dead-beef-a456-426614174000" as AnyObject]
var documentBuilder = ModelBasedGraphQLDocumentBuilder(modelType: ModelReadUpdateField.self,
operationType: .subscription)
documentBuilder.add(decorator: DirectiveNameDecorator(type: .onUpdate))
Expand All @@ -246,8 +246,8 @@ class ModelReadUpdateAuthRuleTests: XCTestCase {

// The owner auth rule contains `.delete` operation, requiring the subscription operation to contain the input
func testModelReadUpdateField_OnDeleteSubscription() {
let claims = ["username": "user1",
"sub": "123e4567-dead-beef-a456-426614174000"]
let claims = ["username": "user1" as AnyObject,
"sub": "123e4567-dead-beef-a456-426614174000" as AnyObject]
var documentBuilder = ModelBasedGraphQLDocumentBuilder(modelType: ModelReadUpdateField.self,
operationType: .subscription)
documentBuilder.add(decorator: DirectiveNameDecorator(type: .onDelete))
Expand Down
Loading