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

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

Closed
hayorov opened this issue Nov 3, 2021 · 14 comments

Comments

@hayorov
Copy link

hayorov commented Nov 3, 2021

Community Note

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

Terraform (and AzureAD Provider) Version

Terraform v1.0.7
on linux_amd64

  • provider registry.terraform.io/hashicorp/azuread v2.8.0
  • provider registry.terraform.io/hashicorp/null v3.1.0
  • provider registry.terraform.io/hashicorp/time v0.7.2
  • provider registry.terraform.io/hashicorp/vault v2.24.1

Affected Resource(s)

  • azuread_application

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

  • Provider supports attribute values imported from existing resources created in Azure Portal

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
Copy link
Contributor

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
Copy link
Author

hayorov commented Nov 3, 2021

@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
Copy link
Contributor

@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
Copy link
Author

hayorov commented Nov 3, 2021

@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
Copy link
Contributor

manicminer commented Nov 3, 2021

@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
Copy link
Author

hayorov commented Nov 5, 2021

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

@hayorov
Copy link
Author

hayorov commented Nov 10, 2021

UPD (for MS representatives) Support request ID: 2111100060000473

@hayorov
Copy link
Author

hayorov commented Nov 13, 2021

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:

  • introduce a new tf resource (application_saml...) that incorporates SAML specifics (e.g. this identifierUris, signing certificate management, attributes, and claims)
  • add some saml virtual attribute in the application (maybe copy preferred_single_sign_on_mode from SP) resource that can reflect such capability

@manicminer
Copy link
Contributor

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

@hayorov
Copy link
Author

hayorov commented Nov 15, 2021

@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
Copy link
Contributor

@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
Copy link
Author

hayorov commented Nov 18, 2021

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
Copy link
Contributor

@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 #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 #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
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 19, 2021
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

2 participants