Description
Every JDBC connector in Presto is expected to support per-session credential
passthrough via the standard user-credential-name / password-credential-name
config properties on BaseJdbcConfig, wired through automatically by
DriverConnectionFactory(Driver, BaseJdbcConfig) (see the constructor in
presto-base-jdbc/.../DriverConnectionFactory.java, and its usage of
JdbcIdentity.getExtraCredentials() in openConnection()).
The Oracle connector doesn't get this for free because
OracleClientModule.connectionFactory() needs to add Oracle-specific
connection properties (TLS truststore, oracle.jdbc.includeSynonyms), so it
builds its own DriverConnectionFactory directly instead of using the
BaseJdbcConfig-only constructor:
return new DriverConnectionFactory(
new OracleDriver(),
config.getConnectionUrl(),
Optional.empty(),
Optional.empty(),
connectionProperties);
The two Optional.empty() arguments are userCredentialName and
passwordCredentialName - by hardcoding them to empty instead of reading
config.getUserCredentialName() / config.getPasswordCredentialName(), this
silently drops support for per-session extraCredentials for Oracle only.
Every other JDBC connector that relies on the default wiring (e.g. MySQL)
supports this correctly.
Impact
Any deployment using per-session Oracle credentials (extraCredentials set
per query/session rather than one static connection-user/connection-password
for the whole catalog) silently falls back to the static config credentials,
with no error - this can be a correctness/security surprise for multi-tenant
setups.
Proposed fix
Pass config.getUserCredentialName() / config.getPasswordCredentialName()
through instead of Optional.empty(), matching the pattern already used by
the DriverConnectionFactory(Driver, BaseJdbcConfig) convenience constructor.
Happy to submit a PR for this.
Description
Every JDBC connector in Presto is expected to support per-session credential
passthrough via the standard
user-credential-name/password-credential-nameconfig properties on
BaseJdbcConfig, wired through automatically byDriverConnectionFactory(Driver, BaseJdbcConfig)(see the constructor inpresto-base-jdbc/.../DriverConnectionFactory.java, and its usage ofJdbcIdentity.getExtraCredentials()inopenConnection()).The Oracle connector doesn't get this for free because
OracleClientModule.connectionFactory()needs to add Oracle-specificconnection properties (TLS truststore,
oracle.jdbc.includeSynonyms), so itbuilds its own
DriverConnectionFactorydirectly instead of using theBaseJdbcConfig-only constructor:The two
Optional.empty()arguments areuserCredentialNameandpasswordCredentialName- by hardcoding them to empty instead of readingconfig.getUserCredentialName()/config.getPasswordCredentialName(), thissilently drops support for per-session
extraCredentialsfor Oracle only.Every other JDBC connector that relies on the default wiring (e.g. MySQL)
supports this correctly.
Impact
Any deployment using per-session Oracle credentials (
extraCredentialssetper query/session rather than one static
connection-user/connection-passwordfor the whole catalog) silently falls back to the static config credentials,
with no error - this can be a correctness/security surprise for multi-tenant
setups.
Proposed fix
Pass
config.getUserCredentialName()/config.getPasswordCredentialName()through instead of
Optional.empty(), matching the pattern already used bythe
DriverConnectionFactory(Driver, BaseJdbcConfig)convenience constructor.Happy to submit a PR for this.