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

Jump to bottom

Migrate ADGraph to MSGraph #1325

Merged
merged 38 commits into from
Nov 13, 2023
Merged

Migrate ADGraph to MSGraph #1325

merged 38 commits into from
Nov 13, 2023

Conversation

xuzhang3
Copy link
Collaborator

@xuzhang3 xuzhang3 commented Nov 9, 2023

SUMMARY
ISSUE TYPE
  • Bugfix Pull Request
  • Docs Pull Request
  • Feature Pull Request
  • New Module Pull Request
COMPONENT NAME
ADDITIONAL INFORMATION

xuzhang3 and others added 30 commits September 13, 2023 17:29
@xuzhang3
migrate to msgraph
03e5e41
@xuzhang3
migrate to msgraph
602173c
@xuzhang3
migrate to msgraph
1c95ce0
@xuzhang3
migrate to ms graph API
068dc8e
@xuzhang3
ad group info
c97b054
@xuzhang3
merge dev
f485ff4
@xuzhang3
app info and group info
0543e81
@xuzhang3
migrate to msgraph
a90df86
@xuzhang3
spn msgraph
34d33e7
@xuzhang3
migrate AD to MS graph
145299a
@neil-yechenwei
Migrate AAD API to MSGraph API for ServicePrincipal, User and Password (
6c88ad0 
#1280)

* migrate sp and fix application

* migrate azure_rm_adserviceprincipal.py

* migrate azure_rm_aduser_info.py

* migrate azure_rm_adpassword_info.py

* migrate azure_rm_aduser.py

* Migrate azure_rm_adpassword.py and fix azure_rm_adpassword_info.py

* update azure_rm_adserviceprincipal.py

* update azure_rm_adserviceprincipal_info.py

* update azure_rm_aduser.py

* update azure_rm_aduser_info.py

* update azure_rm_adpassword.py

* update azure_rm_adpassword_info.py

* update azure_rm_aduser.py

* update azure_rm_adpassword.py

* update azure_rm_adpassword_info.py

* update azure_rm_adpassword.py to use add_password and remove_password

* update azure_rm_aduser_info.py to add select

* update azure_rm_aduser.py to add select

* update code

* update code

* update code

* update code

* update code

* update code

---------

Co-authored-by: neil-yechenwei <v-cheye@microsoft.com>
@xuzhang3
cli auth
5721adf
@xuzhang3
auth with CLI
b58ec8f
@neil-yechenwei
Get the existing user after create (#1286)
7d82076 
Co-authored-by: neil-yechenwei <v-cheye@microsoft.com>
@neil-yechenwei
Add advanced query filter to get the existing user by list (#1287)
0449baf 
Co-authored-by: neil-yechenwei <v-cheye@microsoft.com>
@xuzhang3
get app by app id
43b4163
@xuzhang3
fix lint
3a09918
@xuzhang3
fix sanity
ada96c1
@xuzhang3
fix sanity
67ee47a
@xuzhang3
fix sanity
914a0c2
@xuzhang3
fix sanity
295ca14
@xuzhang3
deprecate tenant ID
fed0bba
@neil-yechenwei
Mark the property for deprecating and renaming property (#1294)
7e8f680 
* Mark the property for deprecating and renaming property

* update code

* update code

---------

Co-authored-by: neil-yechenwei <v-cheye@microsoft.com>
@xuzhang3
deprecate tenant ID
5f67f32
@xuzhang3
merge upstream
28eaf00
@xuzhang3
msgraph sdk v1.0.0
29ccc71
@xuzhang3
deprecate tenant ID
d11956a
@xuzhang3
update doc
c0dbd52
@xuzhang3
fix lint
2e45c4b
@xuzhang3
fix lint
b38b1aa
@xuzhang3 xuzhang3 added the ready_for_review The PR has been modified and can be reviewed and merged label Nov 9, 2023
@xuzhang3 xuzhang3 merged commit 21f6e15 into dev Nov 13, 2023
This was referenced Nov 30, 2023
Closed
Closed
Closed
@Fred-sun Fred-sun mentioned this pull request Jan 24, 2024
Closed
@l3ender
Copy link
Contributor

l3ender commented Mar 13, 2024

We just updated to the 2.x version of the collection and started seeing the following errors with our Ansible service principal (app registration):

APIError
Code: 403
message: None
error: MainError(additional_data={}, code='Authorization_RequestDenied', details=None, inner_error=InnerError(additional_data={'date': DateTime(2024, 3, 13, 18, 50, 25, tzinfo=Timezone('UTC'))}, client_request_id='957e2f10-ffb6-4de3-b84b-eba7263ef1e3', date=None, odata_type=None, request_id='8102b970-a9c8-4b3b-be26-b7c5d53aa5b7'), message='Insufficient privileges to complete the operation.', target=None)

This was due to the change done to migrate from AD Graph to MS Graph (this PR).

To resolve, we had to add the necessary permission to our service principal (Entra ID > App Registrations > API Permissions). In our case, we are only using the azure_rm_adgroup_info module and so only need read access to directories.

image

The first entry is the legacy one, the second is the updated. After we fully migrate we will be removing the first entry.

I suggest the changelog/release notes be updated to clearly specify action is required for this change and that the graph update is a breaking change. As the notes currently stand, it wasn't quite clear (to me) that this was part of the breaking change. We believe this should have been more clearly mentioned as a breaking change, as mentioned back in 2021 - #573 (comment).

Hopefully this helps someone else. Thanks!

@xuzhang3 xuzhang3 deleted the f/msgraph_beta branch May 21, 2024 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
ready_for_review The PR has been modified and can be reviewed and merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@xuzhang3 @l3ender @neil-yechenwei