fix: Separated the logic between Certificate Expiry Notification and Ignore TLS/SSL errors for HTTPS websites

[savtheo]

Summary

In this pull request, the following changes are made:

• When a user selects the Ignore TLS/SSL errors for HTTPS websites option, the Certificate Expiry Notification option is not disabled.
• Instead Expiry Notifications work as should (triggers when the Certificate Expiry Notification option is selected and not when the option is not selected)
• All other functionality remains as is (warnings regarding TLS/SSL errors).

Please follow this checklist to avoid unnecessary back and forth (click to expand)

• [ X ] ⚠️ If there are Breaking change (a fix or feature that alters existing functionality in a way that could cause issues) I have called them out
• [ X ] 🧠 I have disclosed any use of LLMs/AI in this contribution and reviewed all generated content.
  I understand that I am responsible for and able to explain every line of code I submit.
• [ X ] 🔍 Any UI changes adhere to visual style of this project.
• [ X ] 🛠️ I have self-reviewed and self-tested my code to ensure it works as expected.
• [ - ] 📝 I have commented my code, especially in hard-to-understand areas (e.g., using JSDoc for methods).
• [ - ] 🤖 I added or updated automated tests where appropriate.
• [ - ] 📄 Documentation updates are included (if applicable).
• [ - ] 🧰 Dependency updates are listed and explained.
• [ - ] ⚠️ CI passes and is green.

Screenshots for Visual Changes

Before:

After:

Event	Before	After
Ignore TLS/SSL errors for HTTPS webistes

[github-actions]

Hello and thanks for lending a paw to Uptime Kuma! 🐻👋
As this is your first contribution, please be sure to check out our Pull Request guidelines.
In particular: - Mark your PR as Draft while you’re still making changes - Mark it as Ready for review once it’s fully ready
If you have any design or process questions, feel free to ask them right here in this pull request - unclear documentation is a bug too.

[CommanderStorm]

I don't think the way these options would link after this PR works well.
What does this mean or why do you want this to behave this way?

We specifically disabled this because it is a footgun otherwise.

[savtheo]

I don't think the way these options would link after this PR works well. What does this mean or why do you want this to behave this way?

We specifically disabled this because it is a footgun otherwise.

I can't understand why this is a footgun. For whatever reasons I might want to monitor a webserver which is using a self-signed certificate and for which I would like to reissue the certificate when approaches it's end of life. Why a notification of the expiration day is a problem? If I don't want it I can still disable it. Also all the other functionality regarding the TLS/SSL checks remain the same.

[louislam]

Just check our history and git blame to find out why, like #4029.

It may be working for your case, but there are a lot of cases you have to test and handle.

[savtheo]

Just check our history and git blame to find out why, like #4029.

It may be working for your case, but there are a lot of cases you have to test and handle.

Yes exactly! The problem reported in #4029 is what this commit resolves. It will still consider and process the expiry notifications even if the Ignore TLS/SSL error for HTTPS websites option is selected.

The changes are minor and not mess the existing functionality. It just adds the option to get Notifications even if you decide to Ignore the TLS/SSL errors (obviously expired certificate is an error). If you don't want the notifications you just don't select the specific option.

[louislam]

I guess you misunderstood #4029, I think #4029 (PR: #5156) was introduced the limitation.

In other words, this pr reverts the change. So we may need a reason why #5156 have to be reverted.

[savtheo]

Sorry, but I have read and re-read the #4029 many times. In Describe your problem of that issue the author says:
In summary the Certificate alert notifications are not happening. Also when the author replied back on Apr 7, 2024 we can see that he has enable both options, so he wants to get expiry notifications but those are not working and on May 14, 2024 he states that he is using self-signed certificates.

The solution proposed and implemented in #5156 didn't resolve the issue in terms of enabling the notifications but it just removed this functionality for self-signed certificates because it was not working.

A similar issue is reported in #6464, where the author of this mentions the separation of the CA as a possible solution.

The proposed solution in this request is based on separation of functionality. The logic for "Certificate Expiry Notification" is separated from the logic of the "Ignore TLS/SSL errors for https websites". It's been now handled as something different but functional no matter what the other options are.

All the error functionality still works (including the certificate expiration) but also the Certificate Expiry Notification works even if there are errors in the TLS/SSL certificate.

[CommanderStorm]

If you ignore errors anyhow, certificate expirations does not make sense to me.
The proper solution for self-signed certificates is to enrol them into your trust store via the respective environment variables, or to just use let's encrypt.
Certificate expiration notifications without certificate warnings is just a bit weird.

I think there is a better system to be had (not two checkboxes), but just re-adding the checkbox does not resolve the issue.
I think where the difference of these checkboxes is is likely not clear to users.
Maybe a combobox?