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

Support reply_url type Spa on azuread_application #244

Closed
jorgecarleitao opened this issue Apr 19, 2020 · 10 comments · Fixed by #474
Closed

Support reply_url type Spa on azuread_application #244

jorgecarleitao opened this issue Apr 19, 2020 · 10 comments · Fixed by #474

Comments

@jorgecarleitao
Copy link
Contributor

jorgecarleitao commented Apr 19, 2020

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

Description

This is a feature request to extend the reply_url on azuread_application to allow a brand new type of reply urls implemented in Azure AD called "Spa".

They differ from the traditional "Web" reply urls in that they support OAuth2's PKCE, see here on how to change it in the manifest, and here for the discussion on a client. Essentially, reply urls will have two types, "Web" (current) and "Spa" (new addition).

Currently, terraform only supports an array of strings, which represent reply urls of type "Web". We need to extend it to support either an array of strings (for backward compatibility) or an array of maps containing two keys, e.g. "reply_url" and "type", where "type" can only be "Web" or "Spa".

FYI I have validated that changing the type does indeed make PKCE work.

New or Affected Resource(s)

  • azuread_application
@katbyte
Copy link
Collaborator

katbyte commented May 15, 2020

hey @jorgecarleitao. It doesn't appear support for that is in the SDK at the moment, it only accepts an array of strings.

@dominik-lekse
Copy link

I have a use case which also depends on the new Spa type.

References:

@dominik-lekse
Copy link

I have created an upstream issue to track this: Azure/azure-rest-api-specs#9617

@manicminer
Copy link
Member

I doubt this will be supported in AAD Graph as new features are being added to MS Graph.

I've scoured the object references for applications and service principals, and I could only find support for the current/legacy replyUrls property,. I don't doubt it will be added at some point.

The provider is not currently able to leverage MS Graph API, however we do plan on moving to it. Hopefully this property will be supported when that time comes.

@manicminer
Copy link
Member

Supported in MS Graph: https://docs.microsoft.com/en-us/graph/api/resources/spaapplication?view=graph-rest-beta

@yene
Copy link

yene commented Mar 26, 2021

@manicminer What would be the right approach if I wanted to generate App registrations with SPA from config. Should I script against the MS graph manually, or is there an abstraction?

@manicminer
Copy link
Member

@yene Right now you'll need to script this, but we'll have support for SPA configs in the near future.

@CaptainStealthy
Copy link
Contributor

CaptainStealthy commented Mar 31, 2021

@yene I use a post-apply Azure CLI script to add the SPA config. I can probably find the code and post a snippet if you need it

@scott-doyland-burrows
Copy link

Hi,

I use this bash script, where the variables are set like this:

TF_OUT_APPREG_UI_REPLY_URLS="https://url1,https://url2"
TF_OUT_APPREG_UI_OBJECT_ID="00000000-0000-0000-0000-000000000000""

ui_reply_urls=`echo "["\"$TF_OUT_APPREG_UI_REPLY_URLS"\"]" | sed  's/,/\",\"/g'`
spa_patch=$(printf '%s\n' "$ui_reply_urls" | jq -c '{"spa":{"redirectUris":.}}')
az rest --method patch --uri "https://graph.microsoft.com/v1.0/applications/$TF_OUT_APPREG_UI_OBJECT_ID" --headers 'Content-Type=application/json' --body="$spa_patch"

@manicminer manicminer modified the milestones: Blocked, v2.0.0 May 27, 2021
@manicminer manicminer linked a pull request Jun 30, 2021 that will close this issue
@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 Aug 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants