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

Update authentication logic to handle network issues #5010

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
6570e20
rebase
samgst-amazon Oct 24, 2024
f255e8b
add telemetry changes
samgst-amazon Oct 29, 2024
e23b3d0
add tests
samgst-amazon Oct 30, 2024
44c4173
fix exception throwing in tests
samgst-amazon Oct 30, 2024
00102b7
tests pass together
samgst-amazon Oct 30, 2024
8960fac
detekt
samgst-amazon Oct 30, 2024
d2921f7
redundant
samgst-amazon Oct 30, 2024
a5ba978
Update plugins/core/resources/resources/software/aws/toolkits/resourc…
samgst-amazon Oct 31, 2024
57d63ff
synchronize lock for notification
samgst-amazon Oct 31, 2024
4734331
Merge remote-tracking branch 'origin/main' into samgst/UserFacingNetw…
samgst-amazon Oct 31, 2024
6eda457
Merge branch 'samgst/UserFacingNetworkErrors' of github.com:aws/aws-t…
samgst-amazon Oct 31, 2024
0b8656e
fix tests
samgst-amazon Oct 31, 2024
503982c
remove IDE persistent storage
samgst-amazon Nov 1, 2024
43ad975
Merge branch 'main' into samgst/UserFacingNetworkErrors
samgst-amazon Nov 1, 2024
0c29dde
Merge branch 'main' into samgst/UserFacingNetworkErrors
samgst-amazon Nov 1, 2024
03575f2
atomic boolean
samgst-amazon Nov 1, 2024
1e57186
Merge branch 'main' into samgst/UserFacingNetworkErrors
samgst-amazon Nov 5, 2024
8b5ab3a
exceptions
samgst-amazon Nov 4, 2024
a2a9248
throw exception
samgst-amazon Nov 5, 2024
b7cef7f
change when with subject
samgst-amazon Nov 5, 2024
ee69990
tests pass
samgst-amazon Nov 5, 2024
4352800
same mockk framework
samgst-amazon Nov 5, 2024
fff2ce1
detekt
samgst-amazon Nov 5, 2024
7603bb0
unmockkAll for each test
samgst-amazon Nov 5, 2024
3da5208
generalized exception thrown
samgst-amazon Nov 6, 2024
696b97d
Merge branch 'main' into samgst/UserFacingNetworkErrors
samgst-amazon Nov 6, 2024
382e80e
detekt
samgst-amazon Nov 7, 2024
3614a8d
out of order
samgst-amazon Nov 7, 2024
8f90654
Merge branch 'main' into samgst/UserFacingNetworkErrors
samgst-amazon Nov 7, 2024
27e1801
Merge branch 'main' into samgst/UserFacingNetworkErrors
samgst-amazon Nov 11, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,15 @@
import software.aws.toolkits.jetbrains.core.credentials.sso.bearer.BearerTokenProvider
import software.aws.toolkits.jetbrains.core.credentials.sso.bearer.BearerTokenProviderListener
import software.aws.toolkits.jetbrains.core.credentials.sso.bearer.InteractiveBearerTokenProvider
import software.aws.toolkits.jetbrains.utils.notifyInfo
import software.aws.toolkits.jetbrains.utils.runUnderProgressIfNeeded
import software.aws.toolkits.resources.AwsCoreBundle
import software.aws.toolkits.resources.AwsCoreBundle.message
import software.aws.toolkits.telemetry.AuthTelemetry
import software.aws.toolkits.telemetry.CredentialSourceId
import software.aws.toolkits.telemetry.CredentialType
import software.aws.toolkits.telemetry.Result
import java.net.UnknownHostException
Fixed Show fixed Hide fixed
import java.time.Instant

sealed interface ToolkitConnection {
Expand Down Expand Up @@ -254,7 +257,7 @@
var didReauth = false
maybeReauthProviderIfNeeded(project, reauthSource, tokenProvider) {
didReauth = true
runUnderProgressIfNeeded(project, AwsCoreBundle.message("credentials.pending.title"), true) {

Check warning on line 260 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/credentials/ToolkitAuthManager.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Redundant qualifier name

Redundant qualifier name
try {
tokenProvider.reauthenticate()
if (isReAuth) {
Expand All @@ -272,6 +275,7 @@
source = source,
)
}
hasSeenFirstNetworkError = false
} catch (e: Exception) {
if (isReAuth) {
val result = if (e is ProcessCanceledException) Result.Cancelled else Result.Failed
Expand Down Expand Up @@ -320,16 +324,34 @@
BearerTokenAuthState.NEEDS_REFRESH -> {
try {
getLogger<ToolkitAuthManager>().warn { "Starting token refresh" }
return runUnderProgressIfNeeded(project, AwsCoreBundle.message("credentials.refreshing"), true) {

Check warning on line 327 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/credentials/ToolkitAuthManager.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Redundant qualifier name

Redundant qualifier name
tokenProvider.resolveToken()
BearerTokenProviderListener.notifyCredUpdate(tokenProvider.id)
hasSeenFirstNetworkError = false
return@runUnderProgressIfNeeded false
}
} catch (e: SsoOidcException) {
AuthTelemetry.sourceOfRefresh(authRefreshSource = reauthSource.toString())
getLogger<ToolkitAuthManager>().warn(e) { "Redriving bearer token login flow since token could not be refreshed" }
onReauthRequired(e)
return true
} catch (e: Exception) {
when {
Fixed Show fixed Hide fixed
e is SsoOidcException -> {
AuthTelemetry.sourceOfRefresh(authRefreshSource = reauthSource.toString())
getLogger<ToolkitAuthManager>().warn(e) { "Redriving bearer token login flow since token could not be refreshed" }
onReauthRequired(e)
return true
}
e is UnknownHostException || e.message?.contains("Unable to execute HTTP request") == true -> {
samgst-amazon marked this conversation as resolved.
Show resolved Hide resolved
getLogger<ToolkitAuthManager>().warn(e) { "Failed to refresh token" }
if (!hasSeenFirstNetworkError) {
hasSeenFirstNetworkError = true
notifyInfo(
message("general.auth.network.error"),
message("general.auth.network.error.message"),
project
)
}
return false
}
else -> { return false }
}
}
}

Expand Down Expand Up @@ -410,6 +432,8 @@
}
}

private var hasSeenFirstNetworkError = false
samgst-amazon marked this conversation as resolved.
Show resolved Hide resolved

data class ConnectionMetadata(
val sourceId: String? = null,
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package software.aws.toolkits.jetbrains.core.credentials

import com.intellij.openapi.project.Project
import com.intellij.testFramework.ApplicationExtension
import io.mockk.every
import io.mockk.just
import io.mockk.mockkObject
import io.mockk.mockkStatic
import io.mockk.runs
import io.mockk.verify
import org.junit.jupiter.api.Assertions.*
samgst-amazon marked this conversation as resolved.
Show resolved Hide resolved
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.TestInstance
import org.junit.jupiter.api.extension.ExtendWith
import org.mockito.kotlin.doThrow
import org.mockito.kotlin.mock
import org.mockito.kotlin.reset
import org.mockito.kotlin.whenever
samgst-amazon marked this conversation as resolved.
Show resolved Hide resolved
import software.aws.toolkits.jetbrains.core.credentials.sso.DeviceAuthorizationGrantToken
import software.aws.toolkits.jetbrains.core.credentials.sso.bearer.BearerTokenAuthState
import software.aws.toolkits.jetbrains.core.credentials.sso.bearer.BearerTokenProvider
import software.aws.toolkits.jetbrains.core.credentials.sso.bearer.BearerTokenProviderListener
import software.aws.toolkits.jetbrains.utils.notifyInfo
import software.aws.toolkits.resources.AwsCoreBundle.message
import java.time.Instant
import java.time.temporal.ChronoUnit

@ExtendWith(ApplicationExtension::class)
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
class ToolkitAuthManagerTest {
private lateinit var project: Project
private lateinit var tokenProvider: BearerTokenProvider
private var reauthCallCount = 0

@BeforeEach
fun setUp() {
project = mock()
tokenProvider = mock()
reauthCallCount = 0
val field = Class.forName("software.aws.toolkits.jetbrains.core.credentials.ToolkitAuthManagerKt")
.getDeclaredField("hasSeenFirstNetworkError")
field.isAccessible = true
field.set(null, false)
samgst-amazon marked this conversation as resolved.
Show resolved Hide resolved

mockkObject(BearerTokenProviderListener)
mockkStatic("software.aws.toolkits.jetbrains.utils.NotificationUtilsKt")
every {
notifyInfo(any(), any(), any())
} just runs
every { BearerTokenProviderListener.notifyCredUpdate(any<String>()) } just runs
}

@Test
fun `test NEEDS_REFRESH state with network error - first occurrence`() {
whenever(tokenProvider.state()).thenReturn(BearerTokenAuthState.NEEDS_REFRESH)
doThrow(RuntimeException("Unable to execute HTTP request"))
.`when`(tokenProvider)
samgst-amazon marked this conversation as resolved.
Show resolved Hide resolved
.resolveToken()

val result = maybeReauthProviderIfNeeded(
project,
ReauthSource.TOOLKIT,
tokenProvider
) { _ -> reauthCallCount++ }

assertFalse(result)
assertEquals(0, reauthCallCount)
verify(exactly = 1) {
notifyInfo(
message("general.auth.network.error"),
message("general.auth.network.error.message"),
project
)
}
}

@Test
fun `test NEEDS_REFRESH state with network error - subsequent occurrence`() {
whenever(tokenProvider.state()).thenReturn(BearerTokenAuthState.NEEDS_REFRESH)
doThrow(RuntimeException("Unable to execute HTTP request"))
.`when`(tokenProvider)
.resolveToken()

// First call to set the internal flag
maybeReauthProviderIfNeeded(
project,
ReauthSource.TOOLKIT,
tokenProvider
) { _ -> reauthCallCount++ }

// Second call - should not show notification
val result = maybeReauthProviderIfNeeded(
project,
ReauthSource.TOOLKIT,
tokenProvider
) { _ -> reauthCallCount++ }

assertFalse(result)
assertEquals(0, reauthCallCount)
verify(exactly = 1) {
notifyInfo(
message("general.auth.network.error"),
message("general.auth.network.error.message"),
project
)
}
}

@Test
fun `test successful refresh clears notification flag`() {
whenever(tokenProvider.state()).thenReturn(BearerTokenAuthState.NEEDS_REFRESH)

// First trigger a network error
doThrow(RuntimeException("Unable to execute HTTP request"))
.`when`(tokenProvider)
.resolveToken()

maybeReauthProviderIfNeeded(
project,
ReauthSource.TOOLKIT,
tokenProvider
) { _ -> reauthCallCount++ }

reset(tokenProvider)

// Now simulate successful refresh
whenever(tokenProvider.state()).thenReturn(BearerTokenAuthState.NEEDS_REFRESH)
whenever(tokenProvider.resolveToken()).thenReturn(
DeviceAuthorizationGrantToken(
startUrl = "https://example.com",
region = "us-east-1",
accessToken = "testAccessToken",
refreshToken = "testRefreshToken",
expiresAt = Instant.now().plus(1, ChronoUnit.HOURS),
)
)
maybeReauthProviderIfNeeded(
project,
ReauthSource.TOOLKIT,
tokenProvider
) { _ -> reauthCallCount++ }

reset(tokenProvider)
// Now trigger another network error - should show notification again
whenever(tokenProvider.state()).thenReturn(BearerTokenAuthState.NEEDS_REFRESH)
doThrow(RuntimeException("Unable to execute HTTP request"))
.`when`(tokenProvider)
.resolveToken()
maybeReauthProviderIfNeeded(
project,
ReauthSource.TOOLKIT,
tokenProvider
) { _ -> reauthCallCount++ }

verify(exactly = 2) {
notifyInfo(
message("general.auth.network.error"),
message("general.auth.network.error.message"),
project
)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1240,6 +1240,8 @@ gateway.connection.workflow.step_skipped=Step skipped
gateway.connection.workflow.step_successful=\nStep completed successfully\n
general.add.another=Add another
general.auth.reauthenticate=Reauthenticate
general.auth.network.error=Network Error
general.auth.network.error.message=Failed to update connection due to networking issues
samgst-amazon marked this conversation as resolved.
Show resolved Hide resolved
general.cancel=Cancel
general.close_button=Close
general.configure_button=Configure
Expand Down
Loading