oracle / oracle-r2dbc

R2DBC Driver for Oracle Database
https://oracle.com
Other
194 stars 40 forks source link

Cannot change password of connection factory after creation #84

Closed calebcodesgud closed 8 months ago

calebcodesgud commented 1 year ago

I'm using this driver in a r2dbc connection pool which is meant to have variable size. The password for the database I'm accessing rotates hourly... I was hoping to extend the classes

https://github.com/oracle/oracle-r2dbc/blob/main/src/main/java/oracle/r2dbc/impl/OracleConnectionFactoryImpl.java https://github.com/oracle/oracle-r2dbc/blob/main/src/main/java/oracle/r2dbc/impl/OracleReactiveJdbcAdapter.java

to use an extended version of OracleDataSource where the getConnection() functions can call out to retrieve updated passwords.

However, it seems nearly everything in this lib is a final or protected class and cannot be extended...

Is there a better solution to this? Do I have to just destroy my connection pool every time it fails to create a new connection and recreate it entirely?

Would it be possible to make these classes public and non-final? Or even allow a supplier like Mono\<String> for the password?

Michael-A-McMahon commented 1 year ago

I think it's great you've employed a secure practice of rotating your password on the regular. Oracle R2DBC should definitely add support for this.

I like the idea of a publisher that emits the password. We could define an extended option to configure it:

public final class OracleR2dbcOptions {
...
    /**
     * Configures a publisher that emits a database password. Oracle R2DBC
     * subscribes to this publisher and requests a password each time it opens a
     * connection. After consuming the password, Oracle R2DBC writes the value 
     * of zero to all positions of the emitted char array.
     */
    private static final Option<Publisher<char[]>> PASSWORD_PUBLISHER =
      Option.valueOf("oracle.r2dbc.passwordPublisher");
...

And then application code can configure a publisher like this:

    ConnectionFactoryOptions.builder()
        .option(
          OracleR2dbcOptions.PASSWORD_PUBLISHER,
          Mono.fromSupplier(() -> getPassword()))

Just formulating ideas at this point. Do you think this would work?

calebcodesgud commented 1 year ago

@Michael-A-McMahon that would be perfect honestly.

Michael-A-McMahon commented 1 year ago

Awesome. Thanks for bringing this use case up.

mp911de commented 1 year ago

FWIW, we have seen similar requests across the board of drivers. It would likely make sense to have a credentials abstraction so that full credential pairs (username and password) can be rotated. HashiCorp's Vault generates new usernames and passwords upon credential rotation, so something Publisher<Credential> instead of Publisher<char[]> seems useful.

calebcodesgud commented 1 year ago

@mp911de I think the idea of credential supplier where username and password are both returned is also good. Upon reading through the codebase, username and password are both accessed simultaneously often to create new connections, so I don't see the change being too much more painful relative to just a password supplier... Probably worth doing if it can help more people.

@Michael-A-McMahon without committing to anything, can you provide any estimate of when you might get this implemented?

Michael-A-McMahon commented 1 year ago

Really appreciate the discussion here. If this functionality is needed across drivers, then an SPI enhancement seems like the right thing versus an Oracle specific extension.

I'm dealing with some unexpected life events this week, and will be unable to focus much on this issue. I think it's great to promote secure coding, and so I'm looking forward to rejoining the discussion more next week.

mp911de commented 1 year ago

I filed https://github.com/r2dbc/r2dbc-spi/issues/273

calebcodesgud commented 1 year ago

Thanks! @mp911de

Michael-A-McMahon commented 1 year ago

Closing this issue. Better to congregate around the SPI issue.

Michael-A-McMahon commented 9 months ago

As discussed in the r2dbc-spi thread, I'll add support for Options having a value emitted by a Supplier or Publisher. At the least, I'll support USERNAME and PASSWORD options. If possible, I'll get this to work for all options.