Skip to content

fix 404 not found page with PKCE flow by closing previous netty channel #4621

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.

Sign up for GitHub

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

Closed
wants to merge 7 commits into from
Closed

fix 404 not found page with PKCE flow by closing previous netty channel #4621

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e8af292
try fix 404 with hacky approach to manually close netty channel
Will-ShaoHua Jul 24, 2024
33f3d77
Merge remote-tracking branch 'upstream/main' into network
Will-ShaoHua Jul 24, 2024
145bb57
Merge remote-tracking branch 'upstream/main' into network
Will-ShaoHua Jul 26, 2024
d35ec92
revert
Will-ShaoHua Jul 26, 2024
29e0574
fix 404
Will-ShaoHua Jul 27, 2024
970d6d3
Merge remote-tracking branch 'upstream/main' into network
Will-ShaoHua Jul 27, 2024
679cafb
add comment
Will-ShaoHua Jul 29, 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
23 changes: 20 additions & 3 deletions ...nity/src/software/aws/toolkits/jetbrains/core/credentials/sso/pkce/ToolkitOAuthService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ import com.intellij.openapi.wm.IdeFocusManager
import com.intellij.util.Url
import com.intellij.util.Urls.newFromEncoded
import com.intellij.util.io.DigestUtil
import com.jetbrains.rd.util.AtomicReference
import io.netty.buffer.Unpooled
import io.netty.channel.Channel
import io.netty.channel.ChannelHandlerContext
import io.netty.handler.codec.http.FullHttpRequest
import io.netty.handler.codec.http.QueryStringDecoder
Expand All @@ -38,13 +40,22 @@ import java.util.concurrent.CompletableFuture

const val PKCE_CLIENT_NAME = "AWS IDE Plugins for JetBrains"

// TODO: naive short term way to mitigate 404 not found (tcp connection is kept alive after success/failure/cancellation)
private var channel: AtomicReference<Channel?> = AtomicReference(null)

@Service
class ToolkitOAuthService : OAuthServiceBase<AccessToken>() {
override val name: String = "aws/toolkit"

fun hasPendingRequest() = currentRequest.get() != null

fun authorize(registration: PKCEClientRegistration): CompletableFuture<AccessToken> {
channel.get()?.let {
if (it.isOpen) {
it.close().sync()
}
}

val currentRequest = currentRequest.get()
val toolkitRequest = currentRequest?.request as? ToolkitOAuthRequest

Expand Down Expand Up @@ -183,10 +194,15 @@ internal class ToolkitOAuthCallbackHandler : OAuthCallbackHandlerBase() {
}

override fun isSupported(request: FullHttpRequest): Boolean {
/**
* comment this out because if we return false early here the TCP connection will be still kept alive by the IDE. In this state, users
* following login attempts will keep seeing 404 not found page, which could be only resolved by (1) re-start the IDE (2) wait until the TCP connection
* between browsers and IDE is closed
*/
// only handle if we're actively waiting on a redirect
if (!oauthService().hasPendingRequest()) {
return false
}
// if (!oauthService().hasPendingRequest()) {
// return false
// }
Copy link
Contributor Author

@Will-ShaoHua Will-ShaoHua Jul 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

confirmed that tcp connection will indeed be closed if cancel button is clicked (Auth completableFuture will be closed)

Screen.Recording.2024-07-29.at.3.17.14.AM.mov


// only handle the /oauth/callback endpoint
return request.uri().trim('/').startsWith("oauth/callback")
Expand All @@ -195,6 +211,7 @@ internal class ToolkitOAuthCallbackHandler : OAuthCallbackHandlerBase() {

internal class ToolkitOAuthCallbackResultService : RestService() {
override fun execute(urlDecoder: QueryStringDecoder, request: FullHttpRequest, context: ChannelHandlerContext): String? {
channel.getAndSet(context.channel())
val path = urlDecoder.path().substringAfter(getServiceName()).trim('/')
val type = when {
path.endsWith(".css") -> "text/css"
Expand Down