Add logs to notify when SpringDocs/Scalar is enabled because SpringDocs/Scalar is enabled by default (#3090)

[zakaria-shahen]

Add logs to notify when SpringDocs/Scalar is enabled (#3090)

Since the SpringDoc team disagrees with disabling SpringDocs/Scalar by default #3090 (like Scalar does scalar/scalar#6781).
we can use an alternative approach to achieve the same result: notify developers who may be unaware about SpringDocs behavior.

Background:

@bnasslahsen

Making it disabled by default would be better. You could introduce a new property like springdoc.scalar.enabled to allow users to enable or disable it, rather than relying on scalar.enabled.

From previous experience, I've found that people typically disable SpringDoc via springdoc.swagger-ui.enabled only in production profiles, without realizing for several months that their other endpoints remain exposed to the internet through /v3/api-docs.

You might say this is a noob mistake, but in reality, the mistake is made by principal-level developers, and it passes both internal and external penetration testing. No one catches it for months in production, until a curious junior developer (me at one of my previous companies) discovers it by accident.

There may still be other systems/companies where this issue hasn't been discovered yet, so it's better to disable it by default and ensure users familiarize themselves with your library before enabling it.

Note that I sent the same concern to Scalar via email, and they accepted it: scalar/scalar#6781 (Of course, I also sent the same concern to SpringDoc via email)

I agree with you that it may seem overkill, but when you look at it from the user's perspective, it makes sense.

Also, in 2024, the CVE board updated the CNA rules, including the following:

• 4.1.4 Insecure default configuration settings SHOULD be determined to be vulnerabilities

So what do you think?

Thank you for your effort in providing the Spring ecosystem with this library which makes our lives easier.

[lrozenblyum]

@bnasslahsen @zakaria-shahen IMHO #3131 makes deep sense.
The idea of the current issue is to prevent default IMPLICIT insecure configuration to go live without attention.

However if a developer needs this API and enables it EXPLICITLY - the warning becomes just confusing.

[Gugu7264]

+1, these warnings are currently unconditionally printed if the docs are enabled, it doesn't make sense at all to have unfixable warnings.

There should be a way to disable the warnings, and that would be explicitly enabling the docs.

[bnasslahsen]

@Gugu7264/ @zakaria-shahen / @lrozenblyum,

As you can see, it's not easy to satisfy everyone in the open source community!
To accommodate your preference as well, I suggest we introduce an additional property to disable the warnings.
Even, it just adds more configuration...

You're welcome to submit a new PR for this change!

[zakaria-shahen]

Yes, I agree with @bnasslahsen that adding more configuration options can be misleading or confusing to users. Perhaps in the next major version, we could do a similar revamp of the configuration properties like what the Spring team did in Spring Boot 2.0.

For example, we could deprecate the following properties:

• springdoc.api-docs.enabled
• springdoc.swagger-ui.enabled
• scalar.enabled

And replace them with a single property: springdoc.enabled=scalar,swagger-ui,api-docs, with the default value set to none.

[Gugu7264]

@bnasslahsen @zakaria-shahen Instead of adding a new property, couldn't we just display the warnings if the corresponding property (springdoc.[api-docs/swagger-ui].enabled) is not set?

That would make sense to have the warnings in such a case, and also would prevent having yet another property to add as you said.

[bnasslahsen]

@Gugu7264,

I vote for it. I think that could also make sense.
However, it raises the question of whether it would cover all use cases.
There will always be someone asking for a different behavior or level of control, so relying solely on the absence of a property might not be flexible enough for everyone.

[Gugu7264]

@bnasslahsen I see your point, but it would be more confusing to have a property just to disable a warning.
In any case relying on the absence of a property is better than an unconditionally raising a warning.

[bnasslahsen]

Agree, we will go for it for now, and see feedbacks

[mkruisz-dt]

I just stumbled upon this discussion, because I was trying to get rid of the warnings in some projects.
I can confirm that my initial expectation before searching here was to get rid of it by setting the property explicitly, as suggested by @Gugu7264 .