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

New Resource Request: app permissions/Microsoft Graph Permissions #33

Closed
stenio123 opened this issue Dec 5, 2018 · 21 comments
Closed

Comments

@stenio123
Copy link

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Add a new Terraform resource to add/grant app permissions related to Microsoft Graph.

New or Affected Resource(s)

  • azurerm_app_permission_definition
  • azurerm_app_permission_assignment

Potential Terraform Configuration

# Create app (existing resource)
resource "azurerm_azuread_application" "my-app" {
  name                       = "my-app"
}
# Create service principal (existing resource)
resource "azurerm_azuread_service_principal" "app-service-principal" {
  application_id = "${azurerm_azuread_application.my-app.application_id}"
}
# Create app_permission (requested resource)
resource "azurerm_app_permission_definition" "test" {
  name               = "my-app-permission-definition"
  permissions = ["Application.ReadWrite.All", "Directory.ReadWrite.All"] 
}
# Grant app permission (requested resource)
resource "azurerm_app_permission_assignment" "test" {
  app_permission_definition_id = "${azurerm_role_definition.test.id}"
  app_id       = "${azurerm_azuread_service_principal.id}"
}

References

@katbyte katbyte transferred this issue from hashicorp/terraform-provider-azurerm Jan 28, 2019
@katbyte
Copy link
Collaborator

katbyte commented Jan 28, 2019

As we are moving all azure ad resources to the separate AzureAd pr i have moved this issue there

@steve-hawkins
Copy link
Contributor

@katbyte I'm going to start looking into this as thanks to #79 it appears we now have the ability to read the application OAuth2 permissions, so would make sense to be able to manage them as well

just wanted to check if anyone else has already started?

@katbyte
Copy link
Collaborator

katbyte commented May 28, 2019 via email

@jorgecarleitao
Copy link
Contributor

Isn't this already available through the required_resource_access? E.g.

  # Microsoft Graph
  required_resource_access {
    resource_app_id = "00000003-0000-0000-c000-000000000000"

    # Access directory as the signed in user
    resource_access {
      id = "0e263e50-5827-48a4-b97c-d940288653c7"
      type = "Scope"
    }

    # Read directory data
    resource_access {
      id = "06da0dbc-49e2-44d2-8312-53f166ab848a"
      type = "Scope"
    }
  }

@steve-hawkins
Copy link
Contributor

Isn't this already available through the required_resource_access? E.g.

@jorgecarleitao that creates the applications required permissions, but some of those permissions then need to be granted by an admin

Here is an example from the Portal of permissions that have and have not been granted consent from an admin:-

image

@janlunddk
Copy link

Unless I missed something in the proposal here, I think that the azurerm_app_permission_definition would need an additional property, to tell which API these permissions belong to, as a lot of these permissions are present in multiple APIs (e.g. "Application.ReadWrite.All", "Directory.ReadWrite.All")
I learned the hard way by trial and error, that these permissions are not interchangeable between e.g. Azure AD API and MS Graph API, even though the Azure AD API is now marked as legacy.

This is especially important for the AzureAD provider resources as they seem to need the legacy permissions and will not work using the MS Graph permissions.

image

@drdamour
Copy link

drdamour commented Jan 7, 2020

@jorgecarleitao where'd u get the guids from, all i'm used to is URIs like https://graph.microsoft.com/User.Read.All

@drdamour
Copy link

anyone with the same question i had, i found the answer by using

az ad sp list --display-name "Microsoft Graph" | select-string <URI or DISPLAY NAME> -cont 10

@manfred-nilsson-wcar
Copy link

This seams very messy I am not sure what is the correct implementation but I found this post helpful (https://nedinthecloud.com/2019/07/16/demystifying-azure-ad-service-principals/).

And i would question if the right place for this is the Azure AD provider ?

@drdamour
Copy link

drdamour commented Jan 23, 2020

This is the ad provider repo so it's in the right place

@drdamour
Copy link

Fwiw everything in the description is already achievable with the current required_resource_access block as of this writing.

But using the guids only is pretty tough. Be nice if it could be guid OR uri for both the required_resource_access and resource_access blocks since those are what show up in portal.

Someone mentioned u cant grant admin consent...and that would not make sense on an aplication. You can only grant consent for a service principal in a tenant (though the portal shows in the app reg the current tenants admin status)...in fact consent may be for the whole tenant for that app...im not really sure if you can have two SPs in one tenant for a single app and if they can have distinct grants...never tried.

IMO as written this ticket could be closed.

@manicminer
Copy link
Contributor

In terms of the original feature request, I believe API Permissions for an application can be managed with the required_resource_access block of the azuread_application resource. I don't think it makes sense for us to work with the API and permissions friendly names, as Microsoft makes APIs available via their published UUIDs and you can look these up in the portal to store as variables if you wish.

I'm looking to merge a change in #252 that will enable setting published scopes for an application (aka "Expose an API" in the portal), and I believe this covers all the cases discussed in this issue so I am going to mark as resolved.

Please feel free to comment if I have missed something!

@drdamour
Copy link

@manicminer theres not actually an easy way to find the uuid’s in the portal :(

@manicminer
Copy link
Contributor

@drdamour the way I usually do it, is to grant an API scope and then poke through the application manifest. Admittedly this doesn't exactly qualify as easy but it's workable. If there's a way to list them via an API i'm all ears :)

@drdamour
Copy link

@manicminer the command i mention at #33 (comment) is certainly api backed, but not sure if thats something in the go sdk

@raphaelschnaitl
Copy link

Fwiw everything in the description is already achievable with the current required_resource_access block as of this writing.

I do not understand, how is it currently possible to Grant Admin consent with the current required_resource_access?

@manicminer
Copy link
Contributor

I do not understand, how is it currently possible to Grant Admin consent with the current required_resource_access?

@Console32 This particular action is not supported by the provider. Admin Consent for API scopes is implemented as a user-only operation, presumably to enforce oversight, as such there's no supported way of granting consent unless you are signed in as a real user. It's possible this may be changing with some recent additions to the MS Graph API, but that's speculation at this point and we are not currently able to use that API anyway. So, at least for the time being, this is a manual step that tenant administrators must accomplish.

@janlunddk
Copy link

@manicminer Consent can be given using oauth2PermissionGrants although still in Beta.
Sam Cogan wrote a nice article about this topic:
https://samcogan.com/provide-admin-consent-fora-azure-ad-applications-programmatically/

But be warned - it is beta, and I personally had an App Registration that failed badly after playing around with it 😄
Actually it failed so bad that it was causing an internal error in Azure
image

@drdamour
Copy link

Fwiw everything in the description is already achievable with the current required_resource_access block as of this writing.

I do not understand, how is it currently possible to Grant Admin consent with the current required_resource_access?

@Console32 in my interpretation admin consent was not requested in the original description, but i can see how it could be interpreted the other way.

@manicminer
Copy link
Contributor

We'll possibly have to wait for this to make it out of beta :)

@ghost
Copy link

ghost commented Jun 17, 2020

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 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants