Handle email password obfuscation (#23925)
The frontend receives obfuscated values for settings marked sensitive:
```clojure
(defsetting email-smtp-password
(deferred-tru "SMTP password.")
:sensitive? true)
```
and over the wire:
```javascript
{
"key": "email-smtp-password",
"value": "**********ig",
"is_env_setting": false,
"env_name": "MB_EMAIL_SMTP_PASSWORD",
"description": "SMTP password.",
"default": null
},
```
So the frontend form never has a valid password in it. When you edit
email settings, we check if those are valid, and if so, commit the
settings. But with the wrong password email settings are almost always
wrong. You have to re-type the email password just to change other
settings, like the friendly name, reply-to email address, etc.
So we recognize that we've received the obfuscated value, swap in the
real value for the email connection test, and then obfuscate the
password in the response. If we do not receive an obfuscated value, just
leave it alone and return the value anyways (they typed it in, so seems
safe to send it back).
Note that there is a function in models.setting called
`obfuscated-value?` that checks strings against a regex of
`#"^\*{10}.{2}$"`. This is used to never set settings to an obfuscated
value. But its a bit less sensitive than our purposes need. We have a
real value and an obfuscated value so we can check if the obfuscated
value is based on the real value. (obfuscate-value "password") ->
"**********rd" . Whereas we might recognize "**********AA" as the
obfuscated value if we reused that helper function.
NOTE this could be weird if anyone's password changes from "password" to
"**********rd" but the chances of that happening are miniscule.
Had thought to have the FE not send a value at all if it is unchanged
but this had some far-reaching implications that we don't want to tackle
so close to the release. There's an associated issue for LDAP settings
that is largely the same as this:
https://github.com/metabase/metabase/issues/16708 But it is not clear to
me if these are the only two places or if there could be others. Until
that research is done we can just accept this patch as is and come up
with a systematic approach in the future.
But it does seem only three settings are sensitive:
```clojure
setting=> (->> (vals @registered-settings)
(filter :sensitive?)
(map :name))
(:saml-keystore-password :email-smtp-password :ldap-password)
```
The direct setting apis won't write setting values which appear as
obfuscated, but we have other endpoints that take collections of
settings as a unit (for instance, the endpoint /api/email that takes all
of the email settings here to test if they work together).
Please register or sign in to comment