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
@aaronchung-bitquill aaronchung-bitquill force-pushed the feature/federated-auth-plugin branch from bc89e93 to 84018c3 Compare November 17, 2023 19:24
@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
@aaronchung-bitquill aaronchung-bitquill force-pushed the feature/federated-auth-plugin branch from d37c8be to 6bcfc68 Compare December 1, 2023 17:56
@@ -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
@aaronchung-bitquill aaronchung-bitquill force-pushed the feature/federated-auth-plugin branch from 820dbef to 02cdd53 Compare December 14, 2023 00:35
@@ -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 <svvoshch@amazon.com>

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

Signed-off-by: dependabot[bot] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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 <64801825+karenc-bq@users.noreply.github.com>

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

Signed-off-by: dependabot[bot] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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 <64107800+brunos-bq@users.noreply.github.com>

* 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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

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

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Bruno Paiva Lima da Silva <64107800+brunos-bq@users.noreply.github.com>
Co-authored-by: sergiyvamz <svvoshch@amazon.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: sergiyvamz <75754709+sergiyvamz@users.noreply.github.com>
Co-authored-by: Karen <64801825+karenc-bq@users.noreply.github.com>
Co-authored-by: crystall-bitquill <97126568+crystall-bitquill@users.noreply.github.com>
Co-authored-by: aaronchung-bitquill <118320132+aaronchung-bitquill@users.noreply.github.com>
Co-authored-by: congoamz <75754763+congoamz@users.noreply.github.com>
@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