Skip to content

Commit

Permalink
Merge pull request #102 from manicminer/bugfix/nil-checks
Browse files Browse the repository at this point in the history 
Application update consistency check & nil checks
  • Loading branch information
@manicminer
manicminer authored Sep 14, 2021
2 parents ad1f383 + 9f68a1d commit 2eb455d
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 38 deletions.
19 changes: 16 additions & 3 deletions msgraph/applications.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -428,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
Expand Down Expand Up @@ -478,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
Expand Down
10 changes: 3 additions & 7 deletions msgraph/directory_roles.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down
25 changes: 10 additions & 15 deletions msgraph/groups.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down
15 changes: 7 additions & 8 deletions msgraph/serviceprincipals.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions odata/odata.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 2eb455d

Please sign in to comment.