Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot change password of connection factory after creation #84

Closed
calebcodesgud opened this issue Aug 12, 2022 · 10 comments · Fixed by #137
Closed

Cannot change password of connection factory after creation #84

calebcodesgud opened this issue Aug 12, 2022 · 10 comments · Fixed by #137

Comments

@calebcodesgud
Copy link

calebcodesgud commented Aug 12, 2022

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
Copy link
Member

Michael-A-McMahon commented Aug 12, 2022

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
Copy link
Author

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

@Michael-A-McMahon
Copy link
Member

Awesome. Thanks for bringing this use case up.

@mp911de
Copy link

mp911de commented Aug 15, 2022

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
Copy link
Author

@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
Copy link
Member

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
Copy link

mp911de commented Aug 16, 2022

I filed r2dbc/r2dbc-spi#273

@calebcodesgud
Copy link
Author

Thanks! @mp911de

@Michael-A-McMahon
Copy link
Member

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

@Michael-A-McMahon Michael-A-McMahon closed this as not planned Won't fix, can't repro, duplicate, stale Nov 10, 2022
@Michael-A-McMahon
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants