Skip to content

Commit

Permalink
Workaround for corrupted or missing @odata.id for directory objects
Browse files Browse the repository at this point in the history 
We use the `@odata.id` field (`DirectoryObject.ODataId`) to reference
objects for `@odata.bind` fields, e.g. when creating applications,
groups, service principals, or posting new members for groups or
directory roles. Two breaking changes were recently introduced in the
API:

1. `@odata.id` stopped being returned for requests with
   `Accept: odata.metadata=minimal`. Hamilton has a fix for this in
   v0.32.0.
2. The format of the `@odata.id` value has changed for some tenants.
   Previously it was always the URI of the object, now in many cases
   despite the odata.json spec suggesting this should be a URI, it
   takes the form `directoryObject('0000...')`. This form isn't
   recognised by other API endpoints like applications, groups etc.

To work around (2), we're manually constructing the OData ID to look
like a URI. This is intended to be a temporary fix until the API
stabilizes.

See #588
  • Loading branch information
@manicminer
manicminer committed Oct 6, 2021
1 parent 34fa432 commit 933b4a6
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 57 deletions.
42 changes: 30 additions & 12 deletions internal/services/applications/application_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,13 @@ func applicationResourceCreate(ctx context.Context, d *schema.ResourceData, meta
if callerObject == nil {
return tf.ErrorDiagF(errors.New("returned callerObject was nil"), "Could not retrieve calling principal object %q", callerId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if callerObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve calling principal object %q", callerId)
//}
callerObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, callerId)))

ownersFirst20 := msgraph.Owners{*callerObject}
var ownersExtra msgraph.Owners

Expand All @@ -927,21 +934,25 @@ func applicationResourceCreate(ctx context.Context, d *schema.ResourceData, meta
// Retrieve and set the initial owners, which can be up to 20 in total when creating the application
if v, ok := d.GetOk("owners"); ok {
ownerCount := 0
for _, id := range v.(*schema.Set).List() {
if strings.EqualFold(id.(string), callerId) {
for _, ownerId := range v.(*schema.Set).List() {
if strings.EqualFold(ownerId.(string), callerId) {
removeCallerOwner = false
continue
}
ownerObject, _, err := directoryObjectsClient.Get(ctx, id.(string), odata.Query{})
ownerObject, _, err := directoryObjectsClient.Get(ctx, ownerId.(string), odata.Query{})
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", id)
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", ownerId)
}
if ownerObject == nil {
return tf.ErrorDiagF(errors.New("ownerObject was nil"), "Could not retrieve owner principal object %q", id)
}
if ownerObject.ODataId == nil {
return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", id)
return tf.ErrorDiagF(errors.New("ownerObject was nil"), "Could not retrieve owner principal object %q", ownerId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if ownerObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", ownerId)
//}
ownerObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, ownerId)))

if ownerCount < 19 {
ownersFirst20 = append(ownersFirst20, *ownerObject)
} else {
Expand Down Expand Up @@ -1077,14 +1088,21 @@ func applicationResourceUpdate(ctx context.Context, d *schema.ResourceData, meta

if len(ownersToAdd) > 0 {
newOwners := make(msgraph.Owners, 0)
for _, m := range ownersToAdd {
ownerObject, _, err := directoryObjectsClient.Get(ctx, m, odata.Query{})
for _, ownerId := range ownersToAdd {
ownerObject, _, err := directoryObjectsClient.Get(ctx, ownerId, odata.Query{})
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", m)
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", ownerId)
}
if ownerObject == nil {
return tf.ErrorDiagF(errors.New("returned ownerObject was nil"), "Could not retrieve owner principal object %q", m)
return tf.ErrorDiagF(errors.New("returned ownerObject was nil"), "Could not retrieve owner principal object %q", ownerId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if ownerObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", ownerId)
//}
ownerObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, ownerId)))

newOwners = append(newOwners, *ownerObject)
}

Expand Down
11 changes: 8 additions & 3 deletions internal/services/directoryroles/directory_role_member_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/hashicorp/terraform-provider-azuread/internal/clients"
"github.com/hashicorp/terraform-provider-azuread/internal/services/directoryroles/parse"
"github.com/hashicorp/terraform-provider-azuread/internal/tf"
"github.com/hashicorp/terraform-provider-azuread/internal/utils"
"github.com/hashicorp/terraform-provider-azuread/internal/validate"
)

Expand Down Expand Up @@ -90,9 +91,13 @@ func directoryRoleMemberResourceCreate(ctx context.Context, d *schema.ResourceDa
if memberObject == nil {
return tf.ErrorDiagF(errors.New("returned memberObject was nil"), "Could not retrieve member principal object %q", id.MemberId)
}
if memberObject.ODataId == nil {
return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve member principal object %q", id.MemberId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if memberObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve member principal object %q", id.MemberId)
//}
memberObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, id.MemberId)))

role.Members = &msgraph.Members{*memberObject}

if _, err := client.AddMembers(ctx, role); err != nil {
Expand Down
12 changes: 9 additions & 3 deletions internal/services/groups/group_member_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package groups
import (
"context"
"errors"
"fmt"
"log"
"net/http"
"strings"
Expand All @@ -16,6 +17,7 @@ import (
"github.com/hashicorp/terraform-provider-azuread/internal/clients"
"github.com/hashicorp/terraform-provider-azuread/internal/services/groups/parse"
"github.com/hashicorp/terraform-provider-azuread/internal/tf"
"github.com/hashicorp/terraform-provider-azuread/internal/utils"
"github.com/hashicorp/terraform-provider-azuread/internal/validate"
)

Expand Down Expand Up @@ -95,9 +97,13 @@ func groupMemberResourceCreate(ctx context.Context, d *schema.ResourceData, meta
if memberObject == nil {
return tf.ErrorDiagF(errors.New("returned memberObject was nil"), "Could not retrieve member principal object %q", memberId)
}
if memberObject.ODataId == nil {
return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve member principal object %q", memberId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if memberObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve member principal object %q", memberId)
//}
memberObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, memberId)))

group.Members = &msgraph.Members{*memberObject}

if _, err := client.AddMembers(ctx, group); err != nil {
Expand Down
73 changes: 46 additions & 27 deletions internal/services/groups/group_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,9 +418,13 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter
if ownerObject.ID == nil {
return nil, errors.New("ownerObject ID was nil")
}
if ownerObject.ODataId == nil {
return nil, errors.New("ODataId was nil")
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if ownerObject.ODataId == nil {
// return nil, errors.New("ODataId was nil")
//}
ownerObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, id)))

if ownerObject.ODataType == nil {
return nil, errors.New("ownerObject ODataType was nil")
}
Expand All @@ -436,15 +440,12 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter

// First look for the calling principal in the specified owners; it should always be included in the initial
// owners to avoid orphaning a group when the caller doesn't have the Groups.ReadWrite.All scope.
for _, id := range owners {
ownerObject, err := getOwnerObject(ctx, id.(string))
for _, ownerId := range owners {
ownerObject, err := getOwnerObject(ctx, ownerId.(string))
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", id)
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", ownerId)
}
if strings.EqualFold(*ownerObject.ID, callerId) {
if ownerObject.ODataId == nil {
return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", id)
}
if ownerCount < 20 {
ownersFirst20 = append(ownersFirst20, *ownerObject)
} else {
Expand All @@ -456,10 +457,10 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter

// Then look for users, and finally service principals
for _, t := range []odata.Type{odata.TypeUser, odata.TypeServicePrincipal} {
for _, id := range owners {
ownerObject, err := getOwnerObject(ctx, id.(string))
for _, ownerId := range owners {
ownerObject, err := getOwnerObject(ctx, ownerId.(string))
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", id)
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", ownerId)
}
if *ownerObject.ODataType == t && !strings.EqualFold(*ownerObject.ID, callerId) {
if ownerCount < 20 {
Expand Down Expand Up @@ -508,17 +509,21 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter
// Add members after the group is created
members := make(msgraph.Members, 0)
if v, ok := d.GetOk("members"); ok {
for _, id := range v.(*schema.Set).List() {
memberObject, _, err := directoryObjectsClient.Get(ctx, id.(string), odata.Query{})
for _, memberId := range v.(*schema.Set).List() {
memberObject, _, err := directoryObjectsClient.Get(ctx, memberId.(string), odata.Query{})
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve member principal object %q", id)
return tf.ErrorDiagF(err, "Could not retrieve member principal object %q", memberId)
}
if memberObject == nil {
return tf.ErrorDiagF(errors.New("memberObject was nil"), "Could not retrieve member principal object %q", id)
}
if memberObject.ODataId == nil {
return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve member principal object %q", id)
return tf.ErrorDiagF(errors.New("memberObject was nil"), "Could not retrieve member principal object %q", memberId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if memberObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve member principal object %q", memberId)
//}
memberObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, memberId)))

members = append(members, *memberObject)
}
}
Expand Down Expand Up @@ -603,14 +608,21 @@ func groupResourceUpdate(ctx context.Context, d *schema.ResourceData, meta inter

if len(membersToAdd) > 0 {
newMembers := make(msgraph.Members, 0)
for _, m := range membersToAdd {
memberObject, _, err := directoryObjectsClient.Get(ctx, m, odata.Query{})
for _, memberId := range membersToAdd {
memberObject, _, err := directoryObjectsClient.Get(ctx, memberId, odata.Query{})
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve principal object %q", m)
return tf.ErrorDiagF(err, "Could not retrieve principal object %q", memberId)
}
if memberObject == nil {
return tf.ErrorDiagF(errors.New("returned memberObject was nil"), "Could not retrieve member principal object %q", m)
return tf.ErrorDiagF(errors.New("returned memberObject was nil"), "Could not retrieve member principal object %q", memberId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if ownerObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", memberId)
//}
memberObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, memberId)))

newMembers = append(newMembers, *memberObject)
}

Expand Down Expand Up @@ -641,14 +653,21 @@ func groupResourceUpdate(ctx context.Context, d *schema.ResourceData, meta inter

if len(ownersToAdd) > 0 {
newOwners := make(msgraph.Owners, 0)
for _, m := range ownersToAdd {
ownerObject, _, err := directoryObjectsClient.Get(ctx, m, odata.Query{})
for _, ownerId := range ownersToAdd {
ownerObject, _, err := directoryObjectsClient.Get(ctx, ownerId, odata.Query{})
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", m)
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", ownerId)
}
if ownerObject == nil {
return tf.ErrorDiagF(errors.New("returned ownerObject was nil"), "Could not retrieve owner principal object %q", m)
return tf.ErrorDiagF(errors.New("returned ownerObject was nil"), "Could not retrieve owner principal object %q", ownerId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if ownerObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", ownerId)
//}
ownerObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, ownerId)))

newOwners = append(newOwners, *ownerObject)
}

Expand Down
41 changes: 29 additions & 12 deletions internal/services/serviceprincipals/service_principal_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,13 @@ func servicePrincipalResourceCreate(ctx context.Context, d *schema.ResourceData,
if callerObject == nil {
return tf.ErrorDiagF(errors.New("returned callerObject was nil"), "Could not retrieve calling principal object %q", callerId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if callerObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve calling principal object %q", callerId)
//}
callerObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, callerId)))

ownersFirst20 := msgraph.Owners{*callerObject}
var ownersExtra msgraph.Owners

Expand All @@ -385,21 +392,25 @@ func servicePrincipalResourceCreate(ctx context.Context, d *schema.ResourceData,
// Retrieve and set the initial owners, which can be up to 20 in total when creating the service principal
if v, ok := d.GetOk("owners"); ok {
ownerCount := 0
for _, id := range v.(*schema.Set).List() {
if strings.EqualFold(id.(string), callerId) {
for _, ownerId := range v.(*schema.Set).List() {
if strings.EqualFold(ownerId.(string), callerId) {
removeCallerOwner = false
continue
}
ownerObject, _, err := directoryObjectsClient.Get(ctx, id.(string), odata.Query{})
ownerObject, _, err := directoryObjectsClient.Get(ctx, ownerId.(string), odata.Query{})
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", id)
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", ownerId)
}
if ownerObject == nil {
return tf.ErrorDiagF(errors.New("ownerObject was nil"), "Could not retrieve owner principal object %q", id)
}
if ownerObject.ODataId == nil {
return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", id)
return tf.ErrorDiagF(errors.New("ownerObject was nil"), "Could not retrieve owner principal object %q", ownerId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if ownerObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", ownerId)
//}
ownerObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, ownerId)))

if ownerCount < 19 {
ownersFirst20 = append(ownersFirst20, *ownerObject)
} else {
Expand Down Expand Up @@ -485,14 +496,20 @@ func servicePrincipalResourceUpdate(ctx context.Context, d *schema.ResourceData,

if len(ownersToAdd) > 0 {
newOwners := make(msgraph.Owners, 0)
for _, m := range ownersToAdd {
ownerObject, _, err := directoryObjectsClient.Get(ctx, m, odata.Query{})
for _, ownerId := range ownersToAdd {
ownerObject, _, err := directoryObjectsClient.Get(ctx, ownerId, odata.Query{})
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", m)
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", ownerId)
}
if ownerObject == nil {
return tf.ErrorDiagF(errors.New("returned ownerObject was nil"), "Could not retrieve owner principal object %q", m)
return tf.ErrorDiagF(errors.New("returned ownerObject was nil"), "Could not retrieve owner principal object %q", ownerId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if ownerObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", ownerId)
//}
ownerObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, ownerId)))
newOwners = append(newOwners, *ownerObject)
}

Expand Down

0 comments on commit 933b4a6

Please sign in to comment.