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

feat: FederatedAuthConnectionPlugin #741

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

aaronchung-bitquill
Copy link
Contributor

@aaronchung-bitquill aaronchung-bitquill commented Nov 17, 2023

Summary

Description

Implement a new connection plugin for Federated Authentication. Currently, supports SAML authentication through ADFS. and authorization by assuming an AWS IAM role.

Additional Reviewers

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@aaronchung-bitquill aaronchung-bitquill force-pushed the feature/federated-auth-plugin branch 4 times, most recently from a2ac9cd to bc89e93 Compare November 17, 2023 12:25
@karenc-bq karenc-bq changed the title Feature/FederatedAuthConnectionPlugin feat: FederatedAuthConnectionPlugin Nov 17, 2023
@aaronchung-bitquill aaronchung-bitquill force-pushed the feature/federated-auth-plugin branch 9 times, most recently from a0c2a31 to 6b73c62 Compare November 21, 2023 01:33
@aaronchung-bitquill aaronchung-bitquill marked this pull request as ready for review November 21, 2023 01:43
@aaronchung-bitquill aaronchung-bitquill force-pushed the feature/federated-auth-plugin branch 5 times, most recently from 2419f9e to 15a5ba6 Compare November 23, 2023 00:36
@@ -92,7 +93,8 @@ public class ConnectionPluginChainBuilder {
put(HostMonitoringConnectionPluginFactory.class, 800);
put(IamAuthConnectionPluginFactory.class, 900);
put(AwsSecretsManagerConnectionPluginFactory.class, 1000);
put(LogQueryConnectionPluginFactory.class, 1100);
put(FederatedAuthPluginFactory.class, 1100);
Copy link
Contributor

Choose a reason for hiding this comment

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

What would happen if users provide both IAM and FederatedAuth in their plugin list? Should federated auth be put before IAM?

@@ -147,6 +157,17 @@ Failover.failedToUpdateCurrentHostspecAvailability=Failed to update current host
Failover.noOperationsAfterConnectionClosed=No operations allowed after connection closed.
Failover.invalidHostListProvider=Incorrect type of host list provider found, please ensure the correct host list provider is specified. The host list provider in use is: ''{0}'', the plugin is expected a cluster-aware host list provider such as the AuroraHostListProvider.

# Federated Authentication Connection Plugin
FederatedAuthPlugin.generatedNewIamToken=Generated new IAM token = ''{0}''
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need two sets of the same messages for iam auth and federated auth plugin

} else if (html.startsWith("<", i)) {
sb.append('<');
i += 4;
} else if (html.startsWith("&gt;", i)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use replaceAll to simplify this logic?


private String getSignInPageBody(final CloseableHttpClient httpClient, final String uri) throws IOException {
LOGGER.finest(Messages.get("AdfsCredentialsProviderFactory.signOnPageUrl", new Object[] {uri}));
validateURL(uri);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: other method names use "Url", so should be validateUrl()

wrapper/build.gradle.kts Outdated Show resolved Hide resolved
@aaronchung-bitquill aaronchung-bitquill force-pushed the feature/federated-auth-plugin branch 3 times, most recently from c3ef0ad to e13a560 Compare December 13, 2023 20:06
Comment on lines +269 to +284
final Optional<Region> regionOptional = Region.regions().stream()
.filter(r -> r.id().equalsIgnoreCase(rdsRegion))
.findFirst();

if (!regionOptional.isPresent()) {
final String exceptionMessage = Messages.get(
"AwsSdk.unsupportedRegion",
new Object[] {rdsRegion});

LOGGER.fine(exceptionMessage);
throw new SQLException(exceptionMessage);
}

return regionOptional.get();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move this validation to RdsUtils? Duplicated code as the IAM plugin.

}
}

private void updateAuthenticationToken(HostSpec hostSpec, Properties props, Region region, String cacheKey)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move these IAM specific methods to IamAuthUtils to reduce duplicated code?

Comment on lines 107 to 109
FederatedAuthPlugin plugin =
new FederatedAuthPlugin(mockPluginService, mockCredentialsProviderFactory);
FederatedAuthPlugin spyPlugin = Mockito.spy(plugin);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
FederatedAuthPlugin plugin =
new FederatedAuthPlugin(mockPluginService, mockCredentialsProviderFactory);
FederatedAuthPlugin spyPlugin = Mockito.spy(plugin);
FederatedAuthPlugin spyPlugin = Mockito.spy(new FederatedAuthPlugin(mockPluginService, mockCredentialsProviderFactory));

new FederatedAuthPlugin(mockPluginService, mockCredentialsProviderFactory);

String key = "us-east-2:pg.testdb.us-east-2.rds.amazonaws.com:" + DEFAULT_PORT + ":iamUser";
TokenInfo tokenInfo = new TokenInfo(
Copy link
Contributor

Choose a reason for hiding this comment

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

can we make tokenInfo a private static final variable?

Copy link
Contributor

@karenc-bq karenc-bq left a comment

Choose a reason for hiding this comment

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

LGTM, some nits

@aaronchung-bitquill aaronchung-bitquill force-pushed the feature/federated-auth-plugin branch 3 times, most recently from ea01672 to 820dbef Compare December 13, 2023 22:48
@@ -14,6 +14,15 @@
# limitations under the License.
#

# ADFS Credentials Provider Getter
AdfsCredentialsProviderFactory.failedLogin=Failed login. Could not obtain SAML Assertion from ADFS SignOn Page POST response: \n''{0}''
AdfsCredentialsProviderFactory.getSamlAssertionFailed=Failed to get Saml Assertion due to exception: ''{0}''
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
AdfsCredentialsProviderFactory.getSamlAssertionFailed=Failed to get Saml Assertion due to exception: ''{0}''
AdfsCredentialsProviderFactory.getSamlAssertionFailed=Failed to get SAML Assertion due to exception: ''{0}''

@karenc-bq karenc-bq enabled auto-merge (squash) December 14, 2023 01:13
@karenc-bq karenc-bq merged commit 5fcf8f0 into aws:main Dec 14, 2023
5 checks passed
sergiyv-improving added a commit to Bit-Quill/aws-advanced-jdbc-wrapper that referenced this pull request Jan 11, 2024
* Configuration Profiles (aws#711)

Co-authored-by: sergiyvamz <[email protected]>

* chore(deps): bump org.testcontainers:postgresql from 1.19.1 to 1.19.2 (aws#743)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump io.vertx:vertx-stack-depchain from 4.4.6 to 4.5.0 (aws#745)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump org.testcontainers:junit-jupiter from 1.19.1 to 1.19.2 (aws#747)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump io.opentelemetry:opentelemetry-api from 1.31.0 to 1.32.0 (aws#746)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump com.fasterxml.jackson.core:jackson-databind from 2.15.3 to 2.16.0 (aws#744)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Disable failing integration test for PG driver (aws#742)

* Configuration Profiles documentation (aws#738)

* feat: Autoregister a target driver (aws#748)

* chore: reduce log level for intentionally ignored exceptions (aws#751)

* chore(deps): bump org.testcontainers:mariadb from 1.19.1 to 1.19.3 (aws#756)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump software.amazon.awssdk:secretsmanager from 2.21.21 to 2.21.31 (aws#762)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump io.opentelemetry:opentelemetry-sdk from 1.31.0 to 1.32.0 (aws#758)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump org.testcontainers:postgresql from 1.19.2 to 1.19.3 (aws#757)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump org.testcontainers:junit-jupiter from 1.19.2 to 1.19.3 (aws#759)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feat: node fastest response time strategy (aws#755)

* chore: update changelog and versioning for version 2.3.1 (aws#754)

* chore(deps): bump org.testcontainers:testcontainers from 1.19.1 to 1.19.3 (aws#771)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump org.mariadb.jdbc:mariadb-java-client from 3.3.0 to 3.3.1 (aws#767)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump org.apache.poi:poi-ooxml from 5.2.4 to 5.2.5 (aws#769)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump software.amazon.awssdk:secretsmanager from 2.21.31 to 2.21.38 (aws#772)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump software.amazon.awssdk:rds from 2.21.11 to 2.21.38 (aws#773)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump software.amazon.awssdk:rds from 2.21.38 to 2.21.42 (aws#776)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump org.postgresql:postgresql from 42.6.0 to 42.7.1 (aws#778)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump software.amazon.awssdk:secretsmanager from 2.21.38 to 2.21.43 (aws#781)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump io.opentelemetry:opentelemetry-exporter-otlp from 1.32.0 to 1.33.0 (aws#777)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix: use existing entries to update the round robin cache (aws#739)

* set hostId in HostSpec (aws#782)

* docs: update HikariCP example to include configuring the datasource with a JDBC URL (aws#749)

* Enhanced host monitoring plugin ver.2 (aws#764)

* Fix: expose AuroraInitialConnectionStrategyPlugin with a plugin code (aws#784)

* feat: FederatedAuthConnectionPlugin (aws#741)

* chore: replace synchronized with locks in AwsCredentialsManager (aws#785)

* docs: FederatedAuthPlugin (aws#787)

Co-authored-by: Karen <[email protected]>

* chore(deps): bump io.opentelemetry:opentelemetry-sdk-metrics from 1.32.0 to 1.33.0 (aws#792)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump software.amazon.awssdk:ec2 from 2.21.12 to 2.22.1 (aws#795)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump io.opentelemetry:opentelemetry-api from 1.32.0 to 1.33.0 (aws#794)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump org.junit.jupiter:junit-jupiter-params from 5.10.0 to 5.10.1 (aws#793)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Improve efm2 failure detection timing (aws#797)

* chore: update versioning and changelog (aws#791)

* fix: SqlMethodAnalyzer to handle empty SQL query and not throw IndexOutOfBoundsException (aws#798)

* Add documentation for read/write splitting Spring limitations (aws#800)

* Add example code for Read/Write Splitting sample (aws#765)

* fix: restructuring try blocks in dialects for exception handling (aws#799)

* chore(deps): bump software.amazon.awssdk:secretsmanager from 2.21.43 to 2.22.5 (aws#802)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump io.vertx:vertx-stack-depchain from 4.5.0 to 4.5.1 (aws#803)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump com.amazonaws:aws-xray-recorder-sdk-core from 2.14.0 to 2.15.0 (aws#804)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix: add missing log message (aws#801)

Co-authored-by: Bruno Paiva Lima da Silva <[email protected]>

* fix: making a variable volatile in RdsHostListProvider (aws#806)

* chore(deps): bump software.amazon.awssdk:ec2 from 2.22.1 to 2.22.9 (aws#808)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump io.opentelemetry:opentelemetry-sdk from 1.32.0 to 1.33.0 (aws#809)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump org.postgresql:postgresql from 42.6.0 to 42.7.1 (aws#810)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump tj-actions/changed-files from 37 to 41 in /.github/workflows (aws#811)

* transfer session state during failover (aws#814)

* feat: Session state transfer redesign (aws#821)

* chore(deps): bump software.amazon.awssdk:rds from 2.21.42 to 2.22.13 (aws#822)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump software.amazon.awssdk:sts from 2.21.42 to 2.22.13 (aws#823)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump com.fasterxml.jackson.core:jackson-databind from 2.16.0 to 2.16.1 (aws#818)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump com.github.spotbugs from 5.2.+ to 6.0.6 (aws#820)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Improve Multi-AZ cluster detection (aws#824)

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Bruno Paiva Lima da Silva <[email protected]>
Co-authored-by: sergiyvamz <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: sergiyvamz <[email protected]>
Co-authored-by: Karen <[email protected]>
Co-authored-by: crystall-bitquill <[email protected]>
Co-authored-by: aaronchung-bitquill <[email protected]>
Co-authored-by: congoamz <[email protected]>
@aaronchung-bitquill aaronchung-bitquill deleted the feature/federated-auth-plugin branch January 12, 2024 01:36
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 this pull request may close these issues.

5 participants