Provide @ConditionalOn… annotations for all concrete conditions

[OrangeDog]

As I'm partially customising the SAML configuration, it would be very useful to be able to make my config also conditional on org.springframework.boot.autoconfigure.security.saml2.RegistrationConfiguredCondition, rather than having to duplicate it.

[wilkinsona]

We wouldn't make the condition itself public, but provide a public annotation that references the condition. That said, without significant demand, I'm not sure that it's something that we'd want to do and copy-pasting the code may be your best option. To help us to decide, can you please describe in a bit more detail what you're trying to do and the role that the condition would perform in that.

[OrangeDog]

I'm trying to have service(s) that can optionally have SAML enabled via application properties.

I am building my own RelyingPartyRegistrationRepository, so I cannot use @ConditionalOnBean. There's only a single properly that triggers SAML configuration, but it's a map hence why you have a RegistrationConfiguredCondition in the first place instead of being able to use @ConditionalOnProperty.

What I have done is define my own saml.enabled boolean property to use, but that's not the best because Boot's autoconfiguration ignores it.

[philwebb]

Thanks for the additional information @OrangeDog. I'm afraid I don't think we should make RegistrationConfiguredCondition public in our own code. I'd rather keep that code internal.

Give that the class is only about 60 lines I would recommend taking the copy/paste approach that @wilkinsona suggested.

[OrangeDog]

Fine I guess, but I still have to check and re-copy it every time you change it.

[philwebb]

If it's any consolation, the class hasn't had any functional changes since it was written 5 years ago.

[gbaso]

I found this issue looking for the same problem. While I understand the decision to keep the condition class, it's not really satisfying since the corresponding oauth2 condition is indeed public: ClientsConfiguredCondition.

Would it be possible to reconsider the decision? Or, alternatively, add public annotations that reference the conditions?

[philwebb]

ClientsConfiguredCondition is public because it's used by other packages so we couldn't make it package-private. Having said that, the inconsistency is a bit annoying. I'm going to reopen this one to discuss further if we should make the class public or add an annotation.

[philwebb]

We discussed this today and we'd like to hide the ClientsConfiguredCondition behind a @ConditionalOnClientsConfigured annotation which we'll make public. We'd actually like to go further and make sure that all *Condition classes are package-private and have an annotation.

[wilkinsona]

A first pass at this can be found in this branch. I'd like to get some feedback from the team on the names of the new annotations:

• @ConditionalOnEnabledDevTools
• @ConditionalOnIssuerLocationJwtDecoder
• @ConditionalOnOAuth2ClientRegistrationConfigured
• @ConditionalOnPublicKeyJwtDecoder

We should also consider if we want to list the new annotations in the relevant section of the docs.

[OrangeDog]

My 2¢ are that the words Enabled and Configured are probably not needed in the names.

[wilkinsona]

We're going to keep @ConditionalOnEnabledDevTools as we feel that the enabled part if important to distinguish between DevTools just being on the classpath. We're going to rename @ConditionalOnOAuth2ClientRegistrationConfigured to @ConditionalOnOAuth2ClientRegistrationProperties to make it clear that it's properties that have to be set for the condition to match.