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

[Feature request] Enable property azure.activedirectory.redirect-uri-template #21116

Closed
blacelle opened this issue May 3, 2021 · 3 comments · Fixed by #21249
Closed

[Feature request] Enable property azure.activedirectory.redirect-uri-template #21116

blacelle opened this issue May 3, 2021 · 3 comments · Fixed by #21249
Assignees
Labels
azure-spring All azure-spring related issues azure-spring-aad Spring active directory related issues. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. feature-request This issue requires a new behavior in the product in order be resolved.
Milestone

Comments

@blacelle
Copy link

blacelle commented May 3, 2021

Query/Question
Why is com.azure.spring.autoconfigure.aad.AADAuthenticationProperties.redirectUriTemplate being deprecated?

The code states ():

    /**
     * @deprecated Now the redirect-url-template is not configurable.
     * <p>
     * Redirect URI always equal to "{baseUrl}/login/oauth2/code/".
     * </p>
     * <p>
     * User should set "Redirect URI" to "{baseUrl}/login/oauth2/code/" in Azure Portal.
     * </p>
     *
     * @see <a href="https://github.com/Azure/azure-sdk-for-java/tree/c27ee4421309cec8598462b419e035cf091429da/sdk/spring/azure-spring-boot-starter-active-directory#accessing-a-web-application">aad-starter readme.</a>
     * @see com.azure.spring.aad.webapp.AADWebAppConfiguration#clientRegistrationRepository()
     */
    @Deprecated
    private String redirectUriTemplate;

which looks quite arbitrary.

In our case, we rely on this parameter to change the redirecUriTemplate to:

azure.activedirectory:
  redirect-uri-template: '{baseUrl}/admin/login/oauth2/code/{registrationId}'
spring.security.oauth2.client.registration.azure:
  redirect-uri: '{baseUrl}/admin/login/oauth2/code/{registrationId}'

as we want to hide all these routes behind a /admin/** prefix.

As we already hack over AADWebAppConfiguration for other reasons (#19816), we had to also hack the following in this class:

	private ClientRegistration.Builder createClientBuilder(String id) {
		ClientRegistration.Builder result = ClientRegistration.withRegistrationId(id);
		result.authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE);

		// HACK MiTrust
		// Unclear why it is deprecated in com.azure.spring.autoconfigure.aad.AADAuthenticationProperties.redirectUriTemplate
		if (Strings.isNullOrEmpty(properties.getRedirectUriTemplate())) {
			result.redirectUriTemplate("{baseUrl}/login/oauth2/code/");
		} else {
			result.redirectUriTemplate(properties.getRedirectUriTemplate());
		}

                 ...
@ghost ghost added needs-triage This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels May 3, 2021
@joshfree joshfree added azure-spring All azure-spring related issues azure-spring-aad Spring active directory related issues. Client This issue points to a problem in the data-plane of the library. labels May 6, 2021
@ghost ghost removed the needs-triage This is a new issue that needs to be triaged to the appropriate team. label May 6, 2021
@joshfree
Copy link
Member

joshfree commented May 6, 2021

@stliu could you follow up with @blacelle 's question?

@chenrujun
Copy link
Member

@blacelle , thanks for reaching out.

Originally, we thought that this property does not need to be configured in normal case.
Since you have this feature request, we will implement it.
And @han-gao will create PR soon.

@chenrujun chenrujun added this to To do in Spring Cloud Azure via automation May 7, 2021
@chenrujun chenrujun added this to the [2021] June milestone May 7, 2021
@chenrujun chenrujun changed the title Why is com.azure.spring.autoconfigure.aad.AADAuthenticationProperties.redirectUriTemplate deprecated? [Feature request] Enable property azure.activedirectory.redirect-uri-template May 7, 2021
@chenrujun chenrujun added feature-request This issue requires a new behavior in the product in order be resolved. and removed question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels May 7, 2021
@chenrujun chenrujun linked a pull request May 10, 2021 that will close this issue
@han-gao
Copy link
Contributor

han-gao commented May 10, 2021

Hi, @blacelle and @chenrujun

I have created a pr to support it in aad stater, and add the feature description in the aad stater README file.

Spring Cloud Azure automation moved this from To do to Done May 18, 2021
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-java that referenced this issue Oct 17, 2022
Fix RoundTrip error (Azure#21116)

* Fix RoundTrip error

* Fix model validation error

* Fix missing definition ManagedIntegrationRuntime

* Fix Invalid_Type error

* Revert "Fix Invalid_Type error"

This reverts commit 3216774d9c40b33a4837f579b3b6aeef5fa35fa3.
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
azure-spring All azure-spring related issues azure-spring-aad Spring active directory related issues. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. feature-request This issue requires a new behavior in the product in order be resolved.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants