From 33a9c3abc93747fae43fbc27f026245caef5bb0e Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Tue, 14 Sep 2021 10:19:18 +0100 Subject: [PATCH 1/2] Consistency check for applications, roles/scopes sometimes don't get disabled quickly enough --- msgraph/applications.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/msgraph/applications.go b/msgraph/applications.go index a7effa11..61a0ff77 100644 --- a/msgraph/applications.go +++ b/msgraph/applications.go @@ -161,9 +161,22 @@ func (c *ApplicationsClient) Update(ctx context.Context, application Application return status, fmt.Errorf("json.Marshal(): %v", err) } + checkApplicationConsistency := func(resp *http.Response, o *odata.OData) bool { + if resp == nil { + return false + } + if resp.StatusCode == http.StatusNotFound { + return true + } + if resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { + return o.Error.Match(odata.ErrorCannotDeleteOrUpdateEnabledEntitlement) + } + return false + } + _, status, _, err = c.BaseClient.Patch(ctx, PatchHttpRequestInput{ Body: body, - ConsistencyFailureFunc: RetryOn404ConsistencyFailureFunc, + ConsistencyFailureFunc: checkApplicationConsistency, ValidStatusCodes: []int{http.StatusNoContent}, Uri: Uri{ Entity: fmt.Sprintf("/applications/%s", *application.ID), From 9f68a1d632c80252e31eb9012412916c73ab566a Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Tue, 14 Sep 2021 10:19:33 +0100 Subject: [PATCH 2/2] Nil checks --- msgraph/applications.go | 4 ++-- msgraph/directory_roles.go | 10 +++------- msgraph/groups.go | 25 ++++++++++--------------- msgraph/serviceprincipals.go | 15 +++++++-------- odata/odata.go | 11 ++++++----- 5 files changed, 28 insertions(+), 37 deletions(-) diff --git a/msgraph/applications.go b/msgraph/applications.go index 61a0ff77..0dd4a63c 100644 --- a/msgraph/applications.go +++ b/msgraph/applications.go @@ -441,7 +441,7 @@ func (c *ApplicationsClient) AddOwners(ctx context.Context, application *Applica for _, owner := range *application.Owners { // don't fail if an owner already exists checkOwnerAlreadyExists := func(resp *http.Response, o *odata.OData) bool { - if resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { + if resp != nil && resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { return o.Error.Match(odata.ErrorAddedObjectReferencesAlreadyExist) } return false @@ -491,7 +491,7 @@ func (c *ApplicationsClient) RemoveOwners(ctx context.Context, applicationId str // despite the above check, sometimes owners are just gone checkOwnerGone := func(resp *http.Response, o *odata.OData) bool { - if resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { + if resp != nil && resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { return o.Error.Match(odata.ErrorRemovedObjectReferencesDoNotExist) } return false diff --git a/msgraph/directory_roles.go b/msgraph/directory_roles.go index 5612da1d..886661ae 100644 --- a/msgraph/directory_roles.go +++ b/msgraph/directory_roles.go @@ -133,17 +133,13 @@ func (c *DirectoryRolesClient) AddMembers(ctx context.Context, directoryRole *Di for _, member := range *directoryRole.Members { // don't fail if a member already exists checkMemberAlreadyExists := func(resp *http.Response, o *odata.OData) bool { - if resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { + if resp != nil && resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { return o.Error.Match(odata.ErrorAddedObjectReferencesAlreadyExist) } return false } - body, err := json.Marshal(struct { - Member odata.Id `json:"@odata.id"` - }{ - Member: *member.ODataId, - }) + body, err := json.Marshal(DirectoryObject{ODataId: member.ODataId}) if err != nil { return status, fmt.Errorf("json.Marshal(): %v", err) } @@ -242,7 +238,7 @@ func (c *DirectoryRolesClient) Activate(ctx context.Context, roleTemplateID stri // don't fail if a role is already activated checkRoleAlreadyActivated := func(resp *http.Response, o *odata.OData) bool { - if resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { + if resp != nil && resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { return o.Error.Match(odata.ErrorConflictingObjectPresentInDirectory) } return false diff --git a/msgraph/groups.go b/msgraph/groups.go index 816ee9f1..43e3de3a 100644 --- a/msgraph/groups.go +++ b/msgraph/groups.go @@ -63,7 +63,10 @@ func (c *GroupsClient) Create(ctx context.Context, group Group) (*Group, int, er } ownersNotReplicated := func(resp *http.Response, o *odata.OData) bool { - return o != nil && o.Error != nil && o.Error.Match(odata.ErrorResourceDoesNotExist) + if resp != nil && resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { + return o.Error.Match(odata.ErrorResourceDoesNotExist) + } + return false } resp, status, _, err := c.BaseClient.Post(ctx, PostHttpRequestInput{ @@ -398,17 +401,13 @@ func (c *GroupsClient) AddMembers(ctx context.Context, group *Group) (int, error for _, member := range *group.Members { // don't fail if an member already exists checkMemberAlreadyExists := func(resp *http.Response, o *odata.OData) bool { - if resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { + if resp != nil && resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { return o.Error.Match(odata.ErrorAddedObjectReferencesAlreadyExist) } return false } - body, err := json.Marshal(struct { - Member odata.Id `json:"@odata.id"` - }{ - Member: *member.ODataId, - }) + body, err := json.Marshal(DirectoryObject{ODataId: member.ODataId}) if err != nil { return status, fmt.Errorf("json.Marshal(): %v", err) } @@ -452,7 +451,7 @@ func (c *GroupsClient) RemoveMembers(ctx context.Context, id string, memberIds * // despite the above check, sometimes members are just gone checkMemberGone := func(resp *http.Response, o *odata.OData) bool { - if resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { + if resp != nil && resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { return o.Error.Match(odata.ErrorRemovedObjectReferencesDoNotExist) } return false @@ -564,17 +563,13 @@ func (c *GroupsClient) AddOwners(ctx context.Context, group *Group) (int, error) for _, owner := range *group.Owners { // don't fail if an owner already exists checkOwnerAlreadyExists := func(resp *http.Response, o *odata.OData) bool { - if resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { + if resp != nil && resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { return o.Error.Match(odata.ErrorAddedObjectReferencesAlreadyExist) } return false } - body, err := json.Marshal(struct { - Owner odata.Id `json:"@odata.id"` - }{ - Owner: *owner.ODataId, - }) + body, err := json.Marshal(DirectoryObject{ODataId: owner.ODataId}) if err != nil { return status, fmt.Errorf("json.Marshal(): %v", err) } @@ -618,7 +613,7 @@ func (c *GroupsClient) RemoveOwners(ctx context.Context, id string, ownerIds *[] // despite the above check, sometimes owners are just gone checkOwnerGone := func(resp *http.Response, o *odata.OData) bool { - if resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { + if resp != nil && resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { return o.Error.Match(odata.ErrorRemovedObjectReferencesDoNotExist) } return false diff --git a/msgraph/serviceprincipals.go b/msgraph/serviceprincipals.go index 8aefea25..8f10de72 100644 --- a/msgraph/serviceprincipals.go +++ b/msgraph/serviceprincipals.go @@ -64,7 +64,10 @@ func (c *ServicePrincipalsClient) Create(ctx context.Context, servicePrincipal S } appNotReplicated := func(resp *http.Response, o *odata.OData) bool { - return o != nil && o.Error != nil && o.Error.Match(odata.ErrorServicePrincipalInvalidAppId) + if resp != nil && resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { + return o.Error.Match(odata.ErrorServicePrincipalInvalidAppId) + } + return false } resp, status, _, err := c.BaseClient.Post(ctx, PostHttpRequestInput{ @@ -260,17 +263,13 @@ func (c *ServicePrincipalsClient) AddOwners(ctx context.Context, servicePrincipa for _, owner := range *servicePrincipal.Owners { // don't fail if an owner already exists checkOwnerAlreadyExists := func(resp *http.Response, o *odata.OData) bool { - if resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { + if resp != nil && resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { return o.Error.Match(odata.ErrorAddedObjectReferencesAlreadyExist) } return false } - body, err := json.Marshal(struct { - Owner odata.Id `json:"@odata.id"` - }{ - Owner: *owner.ODataId, - }) + body, err := json.Marshal(DirectoryObject{ODataId: owner.ODataId}) if err != nil { return status, fmt.Errorf("json.Marshal(): %v", err) } @@ -314,7 +313,7 @@ func (c *ServicePrincipalsClient) RemoveOwners(ctx context.Context, servicePrinc // despite the above check, sometimes owners are just gone checkOwnerGone := func(resp *http.Response, o *odata.OData) bool { - if resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { + if resp != nil && resp.StatusCode == http.StatusBadRequest && o != nil && o.Error != nil { return o.Error.Match(odata.ErrorRemovedObjectReferencesDoNotExist) } return false diff --git a/odata/odata.go b/odata/odata.go index 6c7f5293..83d7f2fc 100644 --- a/odata/odata.go +++ b/odata/odata.go @@ -8,11 +8,12 @@ import ( ) const ( - ErrorAddedObjectReferencesAlreadyExist = "One or more added object references already exist" - ErrorConflictingObjectPresentInDirectory = "A conflicting object with one or more of the specified property values is present in the directory" - ErrorResourceDoesNotExist = "Resource '.+' does not exist or one of its queried reference-property objects are not present" - ErrorRemovedObjectReferencesDoNotExist = "One or more removed object references do not exist" - ErrorServicePrincipalInvalidAppId = "The appId '.+' of the service principal does not reference a valid application object" + ErrorAddedObjectReferencesAlreadyExist = "One or more added object references already exist" + ErrorCannotDeleteOrUpdateEnabledEntitlement = "Permission (scope or role) cannot be deleted or updated unless disabled first" + ErrorConflictingObjectPresentInDirectory = "A conflicting object with one or more of the specified property values is present in the directory" + ErrorResourceDoesNotExist = "Resource '.+' does not exist or one of its queried reference-property objects are not present" + ErrorRemovedObjectReferencesDoNotExist = "One or more removed object references do not exist" + ErrorServicePrincipalInvalidAppId = "The appId '.+' of the service principal does not reference a valid application object" ) type Id string