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

Don't require secret id in user if already given in jdbc url #17

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,5 @@ The secret being used should be in the JSON format we use for our rotation lambd
...
}
```

Alternatively, you can pass the secret ID as the jdbc uri and omit user. The JDBC connection details such as host, port, dbname will be obtained from your secrets manager secret.
Original file line number Diff line number Diff line change
Expand Up @@ -354,16 +354,29 @@ public Connection connect(String url, Properties info) throws SQLException {
return null;
}

String unwrappedUrl = "";
if (url.startsWith(SCHEME)) { // If this is a URL in the correct scheme, unwrap it
unwrappedUrl = unwrapUrl(url);
String unwrappedUrl = unwrapUrl(url);

if (info != null && info.getProperty("user") != null) {
String credentialsSecretId = info.getProperty("user");
try {
return connectWithSecret(unwrappedUrl, info, credentialsSecretId);
} catch (InterruptedException e) {
// User driven exception. Throw a runtime exception.
throw new RuntimeException(e);
}
} else {
return getWrappedDriver().connect(unwrappedUrl, info);
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is existing code that was previously after this if. It only applies if you are using a full jdbc url with the scheme, so I've moved it up here.


} else { // Else, assume this is a secret ID and try to retrieve it
String secretString = secretCache.getSecretString(url);
if (StringUtils.isNullOrEmpty(secretString)) {
throw new IllegalArgumentException("URL " + url + " is not a valid URL starting with scheme " +
SCHEME + " or a valid retrievable secret ID ");
}

String unwrappedUrl = "";
try {
JsonNode jsonObject = mapper.readTree(secretString);
String endpoint = jsonObject.get("host").asText();
Expand All @@ -376,18 +389,14 @@ public Connection connect(String url, Properties info) throws SQLException {
// Most likely to occur in the event that the data is not JSON. This is more of a user error.
throw new RuntimeException(INVALID_SECRET_STRING_JSON);
}
}

if (info != null && info.getProperty("user") != null) {
String credentialsSecretId = info.getProperty("user");
try {
return connectWithSecret(unwrappedUrl, info, credentialsSecretId);
return connectWithSecret(unwrappedUrl, info, url);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And in the case a user is using a secret as the jdbc url, just use that value rather than looking in user.

} catch (InterruptedException e) {
// User driven exception. Throw a runtime exception.
throw new RuntimeException(e);
}
} else {
return getWrappedDriver().connect(unwrappedUrl, info);

}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,8 @@ public void test_connect_jdbc_returnsNull() throws SQLException {
}

@Test
public void test_connect_works_secretId() {
public void test_connect_works_secretId_in_url() {
Properties props = new Properties();
props.setProperty("user", "user");
assertNotThrows(() -> sut.connect("someSecretId", props));
assertEquals(1, DummyDriver.connectCallCount);
}
Expand Down