Converts the CryptoExceptions into session messages.

[VangelisP]

Overview

Converts the CryptoExceptions into session messages. Please see https://lab.civicrm.org/dev/core/-/issues/4308 for more information

Before

If you change the CIVICRM_CRED_KEYS variable (or clone the site), access to /civicrm/admin/setting/smtp?reset=1 is producing a WSOD due to the key change

After

Gain access to /civicrm/admin/setting/smtp?reset=1 but also warn the user that his/her key is invalid and needs to re-enter it.

[civibot]

(Standard links)

• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[totten]

For the purposes of /civicrm/admin/setting/smtp?reset=1, I agree this looks better. I like that the message even gets translated here. For this screen, it is fair to show a UI message and fill smtpPassword with some placeholder (like null or "" or the raw ciphertext).

But I'm skeptical about straight substitution of CryptoException with setStatus(). The essential issue is that there are multiple paths to that line, and I think it's going to trade one problem for another. Some examples:

• This status check uses the exception to detect whether SIGN key is working. Without the exception, the status-message won't be reported properly. (EDIT: I haven't r-run with the patch. But in HEAD, it looks to be this exact exception that it relies on.)
• The error-condition (bad-key; due to prod<=>staging; or due to mangled key-rotation; or due to other misconfiguration) would also affect background-operations, such as email-blasts or membership-reminders. There is no "session" where it can display the notice, so the notice would be dropped. Execution would proceed with some dummy value -- and then fail in some arbitrary/chaotic way. It seems better IMHO for these to fail with the exception (because that's easier to debug).

Would it make sense to you if the setting form caught the exception and then passed it to the UI (e.g. setStatus($e->getMessage()))?

[VangelisP]

Hi @totten , I was almost certain that there were more things happening in the backend but i didn't knew how many and where. Your suggestion is way more optimized and efficient and does the job (just tested it) equally because I do get what I want to get (a notification to the admin(s) about the invalid key).

I will rework my patch to do that instead.

Many thanks!

[totten]

Thank you, @VangelisP! I tried it out, and it seems to work well.

All tests passed. Merging.