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

Spring Authorization Server now defaults multipleIssuersAllowed to false and it cannot be easily re-enabled #41355

Closed
wants to merge 1 commit into from

Conversation

opcooc
Copy link
Contributor

@opcooc opcooc commented Jul 9, 2024

No description provided.

…verProperties to AuthorizationServerSettings
@pivotal-cla
Copy link

@opcooc Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@opcooc opcooc closed this Jul 9, 2024
@opcooc opcooc reopened this Jul 9, 2024
@pivotal-cla
Copy link

@opcooc Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 9, 2024
@opcooc opcooc changed the title Add the multipliceIssuesAllowed parameter from OAuth2AuthorizationSer… Add the multipliceIssuesAllowed parameter from OAuth2AuthorizationServerProperties to AuthorizationServerSettings Jul 9, 2024
@wilkinsona wilkinsona changed the title Add the multipliceIssuesAllowed parameter from OAuth2AuthorizationServerProperties to AuthorizationServerSettings Spring Authorization Server now defaults multipleIssuersAllowed to false and it cannot be easily re-enabled Jul 9, 2024
@wilkinsona
Copy link
Member

Thanks for the PR, @opcooc.

It looks like the default changed late in the 1.3 development cycle. Without this change, if a Boot users wants to flip the default back to its previous value, they would have to stop using the spring.security.oauth2.authorizationserver.* properties and define their own AuthorizationServerSettings bean. That feels rather clunky to me so I am tempted to treat this is a bug and to add a property to make it easier in Spring Boot 3.3.x.

@jgrandja, what's you take on this please? Does the above sound reasonable or is allowing multiple issuers sufficiently rare that would could introduce the property in Boot 3.4 and require an AuthorizationServerSettings bean in 3.3.x?

@wilkinsona wilkinsona added the status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team label Jul 9, 2024
@jgrandja
Copy link

@wilkinsona Yes, the default for AuthorizationServerSettings.multipleIssuersAllowed was reverted to false just before we released 1.3 because a default of true did not automatically make the authorization server multi-tenant capable as there are a a few custom configurations required by the application as indicated in the How-to: Implement Multitenancy guide.

We should add a Boot property for AuthorizationServerSettings.multipleIssuersAllowed to be consistent with the existing properties available for the AuthorizationServerSettings. I think merging this PR in 3.3.x is totally fine but I'll leave it up to you. And apologies, I should have raised a ticket for this so it made it into 3.3.0.

@wilkinsona wilkinsona added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team labels Jul 17, 2024
@wilkinsona wilkinsona self-assigned this Jul 17, 2024
@wilkinsona wilkinsona added this to the 3.3.x milestone Jul 17, 2024
@wilkinsona
Copy link
Member

Thanks, @jgrandja. In that case, we'll consider this to be a bug of omission and add the property in 3.3.x.

@wilkinsona
Copy link
Member

Thank you, @opcooc. I polished your changes as part of merging them. Please see 1a6760e if you're interested.

@opcooc
Copy link
Contributor Author

opcooc commented Jul 17, 2024

Polish "Add configuration property to allow multiple issuers"

Thank you for helping me modify

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants