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

adds new script for administrator's reference regarding OAuth2 permission grants in Entra ID #6536

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

adds new script for administrator's reference regarding OAuth2 permission grants in Entra ID #6536

wants to merge 15 commits into from

Conversation

tmaestrini
Copy link

@tmaestrini tmaestrini commented Dec 26, 2024 •
edited
Loading

Closes #6537

@milanholemans
Copy link
Contributor

Thank you @tmaestrini, could you also link the related issue to this PR?

@tmaestrini
Copy link
Author

Hey @milanholemans, thanks for getting back to me! It looks like you're always on the go! 😃 That's so cool! 👍
It is a part of a script that I've used while doing some work-related tasks. There is no related issue – the PR only adds a new script sample.

@milanholemans
Copy link
Contributor

Since we handle the principle of 1 issue = 1PR, could you create an issue for the completeness?

@tmaestrini tmaestrini changed the title new script sample: add new script for administrator's reference regarding OAuth2 permission grants in Entra ID adds new script for administrator's reference regarding OAuth2 permission grants in Entra ID Dec 26, 2024
@tmaestrini
Copy link
Author

Hey @milanholemans, sorry I missed that. See reference to issue in description field above.

@martinlingstuyl martinlingstuyl self-assigned this Jan 8, 2025
martinlingstuyl
martinlingstuyl requested changes Jan 8, 2025
View reviewed changes
Copy link
Contributor

@martinlingstuyl martinlingstuyl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tmaestrini, my swiss, italian friend 🥂 I really love this sample, well done! I do have some comments to dot the t's and cross the i's. (or was that the other way around 😅) But anyway, let's look at some comments before we merge this!

docs/docs/sample-scripts/entra/entra-remove-oauth2-permission-grants/index.mdx Outdated Show resolved Hide resolved
docs/docs/sample-scripts/entra/entra-remove-oauth2-permission-grants/index.mdx Outdated Show resolved Hide resolved
docs/docs/sample-scripts/entra/entra-remove-oauth2-permission-grants/index.mdx Outdated Show resolved Hide resolved
docs/docs/sample-scripts/entra/entra-remove-oauth2-permission-grants/index.mdx Outdated Show resolved Hide resolved
docs/docs/sample-scripts/entra/entra-remove-oauth2-permission-grants/index.mdx Outdated Show resolved Hide resolved
docs/docs/sample-scripts/entra/entra-remove-oauth2-permission-grants/index.mdx Outdated Show resolved Hide resolved
docs/docs/sample-scripts/entra/entra-remove-oauth2-permission-grants/index.mdx Outdated Show resolved Hide resolved
docs/docs/sample-scripts/entra/entra-remove-oauth2-permission-grants/assets/sample.json Outdated Show resolved Hide resolved
docs/docs/sample-scripts/entra/entra-remove-oauth2-permission-grants/assets/sample.json Outdated Show resolved Hide resolved
docs/docs/sample-scripts/entra/entra-remove-oauth2-permission-grants/assets/sample.json Outdated Show resolved Hide resolved
@martinlingstuyl martinlingstuyl marked this pull request as draft January 8, 2025 20:36
@tmaestrini
Copy link
Author

@martinlingstuyl Muchas gracias, amigo! 🙌 Happy New Years 🥂 – and thank you very much for the review. Great balls of fire!
I've implemented all the fixes. Ready for review again!

@tmaestrini tmaestrini marked this pull request as ready for review January 8, 2025 22:25
martinlingstuyl
martinlingstuyl requested changes Jan 9, 2025
View reviewed changes
Copy link
Contributor

@martinlingstuyl martinlingstuyl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK @tmaestrini, some last really small comments from my side. Let's fix those before we merge it. Very nice work!

docs/docs/sample-scripts/entra/entra-remove-oauth2-permission-grants/assets/sample.json Outdated Show resolved Hide resolved
docs/docs/sample-scripts/entra/entra-remove-oauth2-permission-grants/index.mdx Outdated Show resolved Hide resolved
docs/docs/sample-scripts/entra/entra-remove-oauth2-permission-grants/index.mdx Outdated Show resolved Hide resolved
docs/docs/sample-scripts/entra/entra-remove-oauth2-permission-grants/index.mdx Outdated Show resolved Hide resolved
docs/docs/sample-scripts/entra/entra-remove-oauth2-permission-grants/index.mdx Outdated Show resolved Hide resolved
docs/docs/sample-scripts/entra/entra-remove-oauth2-permission-grants/index.mdx Outdated Show resolved Hide resolved
docs/docs/sample-scripts/entra/entra-remove-oauth2-permission-grants/index.mdx Outdated Show resolved Hide resolved
@martinlingstuyl martinlingstuyl marked this pull request as draft January 9, 2025 08:07
@tmaestrini
Copy link
Author

Thanks for your comments, @martinlingstuyl. I've implemented the changes as requested and made some additional and related updates in the function description as well.

Let's check again – ready to review! 😃

@tmaestrini tmaestrini marked this pull request as ready for review January 9, 2025 22:00
@martinlingstuyl
Copy link
Contributor

Will do @tmaestrini!

@tmaestrini
Copy link
Author

tmaestrini commented Jan 9, 2025 •
edited
Loading

You're great, @martinlingstuyl 🚀🚀🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinlingstuyl martinlingstuyl martinlingstuyl requested changes

Assignees

@martinlingstuyl martinlingstuyl

Labels
pr-sample
Projects
None yet
Milestone
No milestone
3 participants
@tmaestrini @milanholemans @martinlingstuyl