search

hashicorp / terraform-provider-azuread

Terraform provider for Azure Active Directory
https://registry.terraform.io/providers/hashicorp/azuread/latest/docs
Mozilla Public License 2.0
421 stars 289 forks source link

azuread_application.identifier_uris validation disallow schemaless value that supported by Azure Portal #655

Closed hayorov closed 2 years ago

hayorov commented 2 years ago

Community Note

Terraform (and AzureAD Provider) Version

Terraform v1.0.7 on linux_amd64

Affected Resource(s)

Terraform Configuration Files

  ~ resource "azuread_application" "this" {
      ~ identifier_uris                = [
          + "https://XXX.polaris.synopsys.com/alias/XXX",
          - "XXX.polaris.synopsys.com/alias/XXX",
        ]

Debug Output

Error: URI has no host

with azuread_application.this["Polaris RITS DEV SSO"], on main.tf line 29, in resource "azuread_application" "this": 29: identifier_uris = lookup(each.value, "identifier_uris", null)

Expected Behavior

Actual Behavior

Parameter value (azuread_application.identifier_uris) w/o schema (https://) imported from Azure Portal rasies a validation error.

Steps to Reproduce

  1. Create an app in the Azure portal
  2. Define identifier_uris w/o schema eg. XXX.polaris.synopsys.com/alias/XXX
  3. Apply and import azuread_application resource with terraform import XXX YYY
  4. Got validation error Error: URI has no host

Important Factoids

References

Code that introduced the validation 

        // urn scheme not supported with personal account sign-ins
        for _, v := range identifierUris {
            if diags := validate.IsUriFunc([]string{"http", "https", "api", "ms-appx"}, false, false)(v, cty.Path{}); diags.HasError() {
                return fmt.Errorf("`identifier_uris` is invalid. The URN scheme is not supported when `sign_in_audience` is %q or %q",
                    msgraph.SignInAudienceAzureADandPersonalMicrosoftAccount, msgraph.SignInAudiencePersonalMicrosoftAccount)
            }
        }

Azure API response with value w/o schema

Link: <https://developer.microsoft-tst.com/en-us/graph/changes?$filterby=beta,PrivatePreview:RequestSignatureVerification&from=2021-10-01&to=2021-11-01>;rel="deprecation";type="text/html"
Odata-Version: 4.0
Request-Id: d05dc7b6-7608-4596-b6f3-e71e52ac83ba
Strict-Transport-Security: max-age=31536000
Sunset: Tue, 30 Nov 2021 23:59:59 GMT
Vary: Accept-Encoding
X-Ms-Ags-Diagnostic: {"ServerInfo":{"DataCenter":"Southeast Asia","Slice":"E","Ring":"5","ScaleUnit":"001","RoleInstance":"SI2PEPF00000BC1"}}
X-Ms-Resource-Unit: 1

{
    "@odata.context": "https://graph.microsoft.com/beta/$metadata#applications/$entity",
    "id": "YYY",
    "deletedDateTime": null,
    "appId": "ZZZ",
    "applicationTemplateId": null,
    "identifierUris": [
        "XXX.polaris.synopsys.com/alias/XXX"
    ],
    "web": {
        "redirectUris": [
            "https://XXX.polaris.synopsys.com/api/auth/saml/SSO/alias/XXX"
        ],
        "homePageUrl": "https://account.activedirectory.windowsazure.com:444/applications/default.aspx?metadata=customappsso|ISV9.1|primary|z",
        "logoutUrl": "https://XXX.polaris.synopsys.com/api/auth/saml/SingleLogout/alias/XXX",
        "redirectUriSettings": [
            {
                "uri": "https://XXX.polaris.synopsys.com/api/auth/saml/SSO/alias/XXX",
                "index": null
            }
        ],
        "implicitGrantSettings": {
            "enableIdTokenIssuance": true,
            "enableAccessTokenIssuance": false
        }
    },
    "spa": {
        "redirectUris": []
    }
}
============================= End AzureAD Response ============================:
manicminer commented 2 years ago

Hi @hayorov, thanks for raising this! App manifest validation is a complex topic and we'd like to get this right! We do however have to cater for all scenarios and also take into account potential incompatibilities. I was trying to reproduce your particular setup but as I have encountered previously, the portal does not allow identifier URIs without a valid scheme:

Screenshot 2021-11-03 at 09 49 17

Various docs also suggest that the identifier URI must have a valid scheme (and the supported schemes also differ when Microsoft accounts are involved), including the breaking change link you mention.

For most purposes, we must also support the latest validation requirements imposed by Azure AD, regardless of whether it affects existing applications, as not doing so causes Terraform to break in disappointing ways for users.

hayorov commented 2 years ago

@manicminer many thanks for this quick response. Unfortunately, I'm a bit far from Azure AD things, and I have my very personal strong opinion about AAD as a product.

Meanwhile,

Manual onboarding of this app was done using the ā€œEnterprise Applicationā€ blade in azure that usually accepts the Entity ID without https://.

Screenshots (azure portal):

image

image

Also, I've added Azure RM API response with legitimate no-schema value.

I'm fine with schema but in my case, I'm trying to onboard an existing app into the state with around zero object modifications.

manicminer commented 2 years ago

@hayorov Thanks for the reply and clarification. Unfortunately in this instance these fields are not quite the same thing. Whilst it's true that if you update the SAML entity ID in the Enterprise Apps blade, it does also update the App ID URI for the linked app registration when in the same tenant, the reverse is not true. Ultimately it looks like this is a convenience/hack on the part of the Enterprise Apps blade and is something we cannot replicate, since the API for the Single sign-on pane is not public.

hayorov commented 2 years ago

@manicminer should I address it to MS?

TLDR: use of https:// fails with unverified domain 401(0) error based on weird non-backward-compatible changes from MS, ref https://docs.microsoft.com/en-us/azure/active-directory/develop/reference-breaking-changes#appid-uri-in-single-tenant-applications-will-require-use-of-default-scheme-or-verified-domains

am I right that each domain (aka custom must be added as verified in AAD and it seems there is no such functionality in provider or even az cli / rest)

manicminer commented 2 years ago

@hayorov It is possible that the recent "verified domain in app ID URI" breaking change may have incompatibilities with SAML configurations. Unfortunately testing on all counts here is difficult for us due to the lack of API support for SAML configurations.

hayorov commented 2 years ago

@manicminer Ic, i plan to talk with our MS guys about this problem earlier next week and will update this iisue.

hayorov commented 2 years ago

UPD (for MS representatives) Support request ID: 2111100060000473

hayorov commented 2 years ago

UPD: I got an official support acknowledgment that for SAML-based application identifierUris could omit scheme and it's a legit case.

I plan to support such behavior and raise a PR, @manicminer may i seek your advice on this matter...

the tricky thing is that currently saml capabilities are defined via service principal - preferred_single_sign_on_mode and not available during application resource validation so I foresee two options:

manicminer commented 2 years ago

Thanks @hayorov, we plan to support these once there is a publicly available API endpoint for SAML configuration of service principals

hayorov commented 2 years ago

@manicminer Appreciate your quick response.

May I ask "publicly available API endpoint" means MS Graph support? (are we talking about APIs mentioned here) e.g.:

or something different?

It seems that you don't require help/contribution here and already have plans.

Probably I'll make a fork and will use it until your SAML release. May I know rough ETAs?

manicminer commented 2 years ago

@hayorov No problem - any endpoint that exposes the initial SAML configuration for service principals would do. The corresponding portal blade uses an internal API to do this.

hayorov commented 2 years ago

UPD: from the MS ticket I got confirmation that if app is created via API (e.g. tf app resource) and the owner added it will not allow editing SAML attributes - like claims. They recommend creating an app via Portal (UI)...

manicminer commented 2 years ago

@hayorov Thanks, appreciate your feedback from chasing this up. Whilst that may be unsatisfactory for many, unfortunately it does correlate with the lack of API support for this aspect of SAML configuration.

We're already tracking this in https://github.com/hashicorp/terraform-provider-azuread/issues/173, as mentioned here and in that thread we're waiting on this API support before we can implement. We can't support setting-by-proxy of the SAML identifier via the App ID URI property and as such it's unlikely we'll change our validation to allow schemaless values. Accordingly I'm going to close this issue in favor of https://github.com/hashicorp/terraform-provider-azuread/issues/173, and I would encourage you to upvote and subscribe to that issue as this helps us in communicating the need to Microsoft. Thanks!

github-actions[bot] commented 2 years ago

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.