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
Fix AWS SSO connection with RDS using IAM #4278
Conversation
@@ -168,7 +208,8 @@ class IamAuthTest { | |||
hasCredentials: Boolean = true, | |||
hasBadHost: Boolean = false, | |||
hasSslConfig: Boolean = true, | |||
dbmsType: Dbms = Dbms.POSTGRES | |||
dbmsType: Dbms = Dbms.POSTGRES, | |||
credentialId: String = this.credentialId |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new parameter is not actually used anywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is being used on line number 227. It is used to store the profile name.
@@ -49,6 +58,17 @@ class IamAuth : DatabaseAuthProviderCompatabilityAdapter { | |||
|
|||
override fun createWidget(project: Project?, credentials: DatabaseCredentials, dataSource: LocalDataSource): AuthWidget? = IamAuthWidget() | |||
|
|||
inner class SsoNoTokenFix(val project: Project, val connection: ProtoConnection) : ErrorInfo.Fix { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this really need to be an inner class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are using getAuthInformation
which is not accessible without using inner class.
val simpleErrorInfo = SimpleErrorInfo( | ||
message("rds.validation.iam_sso_connection.error_info"), | ||
e, | ||
listOf(SsoNoTokenFix(project, connection)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the error and fix are always the same so why do we split up the declarations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as discussed, we'll refactor this a bit more for the other cases -- redshift/IAM and secrets manager
.../toolkit/jetbrains-ultimate/src/software/aws/toolkits/jetbrains/services/rds/auth/IamAuth.kt
Show resolved
Hide resolved
.../toolkit/jetbrains-ultimate/src/software/aws/toolkits/jetbrains/services/rds/auth/IamAuth.kt
Outdated
Show resolved
Hide resolved
fun handleSsoAuthentication(project: Project, connection: ProtoConnection): ProtoConnection { | ||
val authInformation = getAuthInformation(connection) | ||
val profileCredentials = | ||
CredentialManager.getInstance().getCredentialIdentifierById(authInformation.connectionSettings.credentials.id) as ProfileCredentialsIdentifierSso | ||
|
||
val session = CredentialManager.getInstance() | ||
.getSsoSessionIdentifiers() | ||
.first { it.id == profileCredentials.sessionIdentifier } | ||
val ssoConnection = ToolkitAuthManager.getInstance().getOrCreateSsoConnection( | ||
UserConfigSsoSessionProfile( | ||
configSessionName = profileCredentials.ssoSessionName, | ||
ssoRegion = session.ssoRegion, | ||
startUrl = session.startUrl, | ||
scopes = session.scopes.toList() | ||
) | ||
) | ||
|
||
reauthConnectionIfNeeded(project, ssoConnection) | ||
return connection | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how can we reuse the existing logic instead of copy pasting it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ProfileCredentialsIdentifierSso
returns a user action that performs the authentication. We are returning/throwing an error with a fix to authenticate here. Only session and ssoConnection part is common. We will have to create a separate method for that.
...lkit/jetbrains-ultimate/tst/software/aws/toolkits/jetbrains/services/rds/auth/IamAuthTest.kt
Outdated
Show resolved
Hide resolved
...lkit/jetbrains-ultimate/tst/software/aws/toolkits/jetbrains/services/rds/auth/IamAuthTest.kt
Outdated
Show resolved
Hide resolved
.changes/next-release/bugfix-7d06545e-eb51-4a84-9333-96a7add7e78c.json
Outdated
Show resolved
Hide resolved
plugins/toolkit/resources/resources/software/aws/toolkits/resources/MessagesBundle.properties
Outdated
Show resolved
Hide resolved
val simpleErrorInfo = SimpleErrorInfo( | ||
message("rds.validation.iam_sso_connection.error_info"), | ||
e, | ||
listOf(SsoNoTokenFix(project, connection)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as discussed, we'll refactor this a bit more for the other cases -- redshift/IAM and secrets manager
Quality Gate passedIssues Measures |
Types of changes
Description
When attempting to establish a connection using a new SSO profile from the config file, the absence of a generated token leads to connection problems. Implementing a fix where users will be prompted to go through the browser login flow again. This will generate the necessary token and establish the connection with the SSO profile seamlessly.
Checklist
License
I confirm that my contribution is made under the terms of the Apache 2.0 license.