[DERCBOT-1691] Support GSA Impersonation #1950
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -31,7 +31,9 @@ import com.google.api.client.json.jackson2.JacksonFactory | |||||
| import com.google.api.services.chat.v1.HangoutsChat | ||||||
| import com.google.auth.http.HttpCredentialsAdapter | ||||||
| import com.google.auth.oauth2.GoogleCredentials | ||||||
| import com.google.auth.oauth2.ImpersonatedCredentials | ||||||
| import com.google.auth.oauth2.ServiceAccountCredentials | ||||||
| import mu.KotlinLogging | ||||||
| import java.io.ByteArrayInputStream | ||||||
| import java.io.InputStream | ||||||
| import kotlin.reflect.KClass | ||||||
|
|
@@ -41,23 +43,36 @@ private const val SERVICE_CREDENTIAL_PATH_PARAMETER = "serviceCredentialPath" | |||||
| private const val SERVICE_CREDENTIAL_CONTENT_PARAMETER = "serviceCredentialContent" | ||||||
| private const val BOT_PROJECT_NUMBER_PARAMETER = "botProjectNumber" | ||||||
| private const val CONDENSED_FOOTNOTES_PARAMETER = "useCondensedFootnotes" | ||||||
| private const val GSA_TO_IMPERSONATE_PARAMETER = "gsaToImpersonate" | ||||||
|
|
||||||
| internal object GoogleChatConnectorProvider : ConnectorProvider { | ||||||
|
|
||||||
| private val logger = KotlinLogging.logger {} | ||||||
|
|
||||||
| override val connectorType: ConnectorType get() = googleChatConnectorType | ||||||
|
|
||||||
| override fun connector(connectorConfiguration: ConnectorConfiguration): Connector { | ||||||
| with(connectorConfiguration) { | ||||||
|
|
||||||
| val gsaToImpersonate = connectorConfiguration.parameters[GSA_TO_IMPERSONATE_PARAMETER] | ||||||
|
|
||||||
| val credentials: GoogleCredentials = if (gsaToImpersonate.isNullOrBlank()) { | ||||||
| logger.info { "Using classic authentication mode with JSON credentials" } | ||||||
| val credentialInputStream = getCredentialInputStream(connectorConfiguration) | ||||||
| loadCredentials(credentialInputStream) | ||||||
| } else { | ||||||
| logger.info { "Using impersonation mode with GSA: $gsaToImpersonate" } | ||||||
|
There was a problem hiding this comment. Logging the service account email to impersonate could expose sensitive configuration information. Consider using a more generic log message or redacting part of the email (e.g., showing only the domain). For example:
Suggested change
|
||||||
| createImpersonatedCredentials(connectorConfiguration, gsaToImpersonate) | ||||||
| } | ||||||
|
|
||||||
| try { | ||||||
| val token = credentials.refreshAccessToken() | ||||||
| logger.info { "Google credentials OK, access token valid until ${token.expirationTime}" } | ||||||
| } catch (e: Exception) { | ||||||
| logger.error(e) { "Unable to obtain access token for Google Chat API" } | ||||||
| } | ||||||
|
|
||||||
| val requestInitializer: HttpRequestInitializer = HttpCredentialsAdapter(credentials) | ||||||
|
|
||||||
| val useCondensedFootnotes = | ||||||
| connectorConfiguration.parameters[CONDENSED_FOOTNOTES_PARAMETER] == "1" | ||||||
|
|
@@ -85,6 +100,54 @@ internal object GoogleChatConnectorProvider : ConnectorProvider { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| private fun createImpersonatedCredentials( | ||||||
| connectorConfiguration: ConnectorConfiguration, | ||||||
| targetServiceAccount: String | ||||||
| ): GoogleCredentials { | ||||||
|
|
||||||
| val sourceCredentials = getSourceCredentials(connectorConfiguration) | ||||||
|
|
||||||
| logger.info { "Source credentials: ${(sourceCredentials as? ServiceAccountCredentials)?.clientEmail}" } | ||||||
|
There was a problem hiding this comment. Logging the source service account email could expose sensitive configuration information. Consider removing this log statement or redacting the email address.
Suggested change
|
||||||
| logger.info { "Impersonating target GSA = $targetServiceAccount with scopes = $CHAT_SCOPE" } | ||||||
|
There was a problem hiding this comment. Logging the target service account email could expose sensitive configuration information. Consider removing this log statement or redacting the email address.
Suggested change
|
||||||
|
|
||||||
| return ImpersonatedCredentials.create( | ||||||
| sourceCredentials, | ||||||
| targetServiceAccount, | ||||||
| null, | ||||||
| listOf(CHAT_SCOPE), | ||||||
| 3600, | ||||||
|
There was a problem hiding this comment. The hardcoded lifetime value of 3600 seconds (1 hour) should be extracted to a named constant for better maintainability. Consider adding
Contributor
There was a problem hiding this comment. You can also use a property to make it configurable |
||||||
| null | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| private fun getSourceCredentials(connectorConfiguration: ConnectorConfiguration): GoogleCredentials { | ||||||
| return try { | ||||||
| val credentialInputStream = getCredentialInputStream(connectorConfiguration) | ||||||
| val creds = ServiceAccountCredentials.fromStream(credentialInputStream) | ||||||
| .createScoped("https://www.googleapis.com/auth/cloud-platform") | ||||||
|
|
||||||
| logger.info { "Loaded explicit service account: ${(creds as ServiceAccountCredentials).clientEmail}" } | ||||||
|
There was a problem hiding this comment. Logging the service account email could expose sensitive configuration information. Consider removing this log statement or redacting the email address.
Suggested change
|
||||||
|
|
||||||
| creds | ||||||
| } catch (e: Exception) { | ||||||
| logger.info { "No explicit credentials found, using Application Default Credentials" } | ||||||
| GoogleCredentials.getApplicationDefault() | ||||||
| .createScoped("https://www.googleapis.com/auth/cloud-platform") | ||||||
| } | ||||||
|
Comment on lines
+132
to
+136
There was a problem hiding this comment. The catch block handles all exceptions generically with |
||||||
| } | ||||||
|
|
||||||
| private fun getCredentialInputStream(connectorConfiguration: ConnectorConfiguration): InputStream { | ||||||
| return connectorConfiguration.parameters[SERVICE_CREDENTIAL_PATH_PARAMETER] | ||||||
| ?.let { resourceAsStream(it) } | ||||||
| ?: connectorConfiguration.parameters[SERVICE_CREDENTIAL_CONTENT_PARAMETER] | ||||||
| ?.let { ByteArrayInputStream(it.toByteArray()) } | ||||||
| ?: error( | ||||||
| "Service credential missing: either " + | ||||||
| "$SERVICE_CREDENTIAL_PATH_PARAMETER or " + | ||||||
| "$SERVICE_CREDENTIAL_CONTENT_PARAMETER must be provided" | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| private fun loadCredentials(inputStream: InputStream): GoogleCredentials = | ||||||
| ServiceAccountCredentials | ||||||
| .fromStream(inputStream) | ||||||
|
|
@@ -99,6 +162,11 @@ internal object GoogleChatConnectorProvider : ConnectorProvider { | |||||
| BOT_PROJECT_NUMBER_PARAMETER, | ||||||
| true | ||||||
| ), | ||||||
| ConnectorTypeConfigurationField( | ||||||
| "Service account email to impersonate (if provided, priority over JSON credentials)", | ||||||
|
There was a problem hiding this comment. The description "Service account email to impersonate (if provided, priority over JSON credentials)" is misleading. Impersonation doesn't replace JSON credentials; it uses them (or ADC) as the source for impersonation. Consider revising to: "Service account email to impersonate (optional, uses JSON credentials or ADC as source)" to clarify that both can work together.
Suggested change
|
||||||
| GSA_TO_IMPERSONATE_PARAMETER, | ||||||
| false | ||||||
| ), | ||||||
| ConnectorTypeConfigurationField( | ||||||
| "Service account credential file path (default : /service-account-{connectorId}.json)", | ||||||
| SERVICE_CREDENTIAL_PATH_PARAMETER, | ||||||
|
|
@@ -122,4 +190,4 @@ internal object GoogleChatConnectorProvider : ConnectorProvider { | |||||
| setOf(GoogleChatConnectorTextMessageOut::class) | ||||||
| } | ||||||
|
|
||||||
| internal class GoogleChatConnectorProviderService : ConnectorProvider by GoogleChatConnectorProvider | ||||||
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.
Logging the complete message content with
message=${message.toGoogleMessage()}could inadvertently log sensitive user data or PII. Consider logging only metadata (space, thread) or use a sanitized representation of the message.