oracle-r2dbc icon indicating copy to clipboard operation
oracle-r2dbc copied to clipboard

Cannot change password of connection factory after creation

Open calebcodesgud opened this issue 2 years ago • 8 comments

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?

calebcodesgud avatar Aug 12 '22 20:08 calebcodesgud

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?

Michael-A-McMahon avatar Aug 12 '22 21:08 Michael-A-McMahon

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

calebcodesgud avatar Aug 12 '22 21:08 calebcodesgud

Awesome. Thanks for bringing this use case up.

Michael-A-McMahon avatar Aug 12 '22 21:08 Michael-A-McMahon

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.

mp911de avatar Aug 15 '22 06:08 mp911de

@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?

calebcodesgud avatar Aug 15 '22 17:08 calebcodesgud

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.

Michael-A-McMahon avatar Aug 15 '22 20:08 Michael-A-McMahon

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

mp911de avatar Aug 16 '22 07:08 mp911de

Thanks! @mp911de

calebcodesgud avatar Aug 16 '22 11:08 calebcodesgud

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

Michael-A-McMahon avatar Nov 10 '22 19:11 Michael-A-McMahon

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.

Michael-A-McMahon avatar Oct 25 '23 18:10 Michael-A-McMahon