matebenyovszky commented on code in PR #45788:
URL: https://github.com/apache/superset/pull/45788#discussion_r4389839031
##########
superset/extensions/storage/persistent_dao.py:
##########
@@ -47,6 +50,42 @@
logger = logging.getLogger(__name__)
+class EncryptedBytesType(EncryptedType):
+ """EncryptedType that round-trips arbitrary bytes.
+
+ The sqlalchemy_utils engines encrypt text. Given ``bytes``, the parent
+ class encrypts ``repr(value)`` and then fails to rebuild ``bytes`` from
+ the decrypted string ("string argument without an encoding"), so nothing
+ written with ``encrypt=True`` could be read back. The payload is
+ base64-encoded before encryption and decoded after decryption instead.
+ """
+
+ cache_ok = True
+
+ def process_bind_param(self, value: bytes | None, dialect: Any) -> Any:
+ if value is None:
+ return None
+ return super().process_bind_param(
+ base64.b64encode(value).decode("ascii"), dialect
Review Comment:
Right that `SecretsMigrator` can't re-encrypt these rows, but that predates
this PR and isn't a regression. Before it, the conditional-table pass already
failed on every encrypted `extension_storage` row: the same `bytes(str)` error
on read. It also uses the plain `SECRET_KEY` for rows that are encrypted with a
per-user derived key. Fixing it needs the migrator to know about that key
derivation, so I'd keep it as the separate change described under "Additional
context" in #45787 rather than half-fix it here.
I added tests for the remaining branches (NULL in both directions, text
column values, unknown plaintext). The new class is now fully covered.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
