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.

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

allow reading oauth2 permissions from aad application #79

Conversation

janschumann
Copy link
Contributor

@janschumann janschumann commented May 14, 2019

Adds the oauth2Permissions manifest attribute to the exported attributes of a azuread_application resource and data resource.

https://docs.microsoft.com/en-us/azure/active-directory/develop/reference-app-manifest

@dominik-lekse
Copy link

@janschumann Thanks for doing the "boring" stuff. I am looking forward for this being merged into the provider soon.

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Thanks for the enhancement @janschumann,

I've left a few comments inline

azuread/resource_application.go Outdated Show resolved Hide resolved
website/docs/r/application.html.markdown Outdated Show resolved Hide resolved
website/docs/d/application.html.markdown Show resolved Hide resolved
azuread/resource_application.go Outdated Show resolved Hide resolved
@janschumann janschumann force-pushed the feature/allow_reading_oauth2_permissions_from_aad_application branch from 0bf7646 to 1611566 Compare May 15, 2019 05:47
@jfcoz
Copy link

jfcoz commented May 21, 2019

@katbyte, is the review ok now ?

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Thanks for the revision @janschumann, aside from one minor comment this LGTM 👍 hope you don't mind but i am going to push a change to resolve the one outstanding issue so i can merge this today 🙂

"oauth2_permissions": {
Type: schema.TypeList,
Optional: true,
Computed: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

As this block is only exported/computed this can be removed

Suggested change
Computed: true,

@katbyte katbyte merged commit c8aaa2a into hashicorp:master May 23, 2019
katbyte added a commit that referenced this pull request May 23, 2019
@bluemalkin
Copy link

Hi, is there an ETA for a release with this fix please ?

@dbourcet
Copy link

dbourcet commented Jun 7, 2019

I am curious about the syntax to use. If I have an application like so :

resource "azuread_application" "server" {
    name      = "server"
    reply_urls = ["http://server"]

what is the syntax to access the id of an oauth2_permissions block?
${azuread_application.server.oauth2_permissions[0]["id"] ?
${azuread_application.server.oauth2_permissions["id"] ?
I can't find how to use it.

@dbourcet
Copy link

dbourcet commented Jun 7, 2019

I manage to do it this way: ${lookup(azuread_application.server.oauth2_permissions[0], "id")}.

@ghost
Copy link

ghost commented Jun 23, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Jun 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants