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.

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

Jump to bottom

x/oauth2: Config.Exchange goes into a runaway for loop hammering my OAuth2 server's /token enpoint #64607

Closed
ghstahl opened this issue Dec 7, 2023 · 7 comments
Closed

x/oauth2: Config.Exchange goes into a runaway for loop hammering my OAuth2 server's /token enpoint #64607

ghstahl opened this issue Dec 7, 2023 · 7 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Unreleased

Comments

@ghstahl
Copy link

ghstahl commented Dec 7, 2023 •
edited
Loading

Go version

go version go1.21.5 windows/amd64

What operating system and processor architecture are you using (go env)?

set GO111MODULE=on
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\ghsta\AppData\Local\go-build
set GOENV=C:\Users\ghsta\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\work\github\pkg\mod
set GONOPROXY=github.com/fluffy-bunny/fluffy-kafka,go.mapped.dev
set GONOSUMDB=github.com/fluffy-bunny/fluffy-kafka,go.mapped.dev
set GOOS=windows
set GOPATH=C:\work\github
set GOPRIVATE=github.com/fluffy-bunny/fluffy-kafka,go.mapped.dev
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.21.5
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\work\github\mapped\ccc\oidc-orchestrator\go.mod
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\ghsta\AppData\Local\Temp\go-build3311421769=/tmp/go-build -gno-record-gcc-switches

What did you do?

I have this in a private repo that I can share.
I am using a slight modified version of this app

I am running a simple echo app where I exposed a REST POST /token endpoint.
My handler doesn't do much, other than make an outbound REST call to get some info and then returns.

During the Exchange the client hammers my /token endpoint over and over again. I have seen this on 2 separate machines and can produce it pretty reliably.

The problem seems to be deep in the http library where there is a for loop in play. So probably not the oauth2 code.

I can repo it is running the 2 apps separately and using vscode where I debug both.

I have tracked it down to the r, err := ContextClient(ctx).Do(req.WithContext(ctx))

func doTokenRoundTrip(ctx context.Context, req *http.Request) (*Token, error) {
	r, err := ContextClient(ctx).Do(req.WithContext(ctx))
	if err != nil {
		return nil, err
	}
...
}

net/http/transport.go:604
resp, err = pconn.roundTrip(treq)

btw: Sometimes it works.

I will happily share my repo with someone that can go deeper on this than I have been able to investigate so far.

What did you expect to see?

a single call to the /token endpoint that either succeeds for fails.

What did you see instead?

My external services /token endpoint is getting hammered.
@gopherbot gopherbot added this to the Unreleased milestone Dec 7, 2023
@mauri870
Copy link
Member

mauri870 commented Dec 7, 2023

Can you share a trimmed down version of your code that triggers this, so we can reproduce? Thanks

@mauri870 mauri870 added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Dec 7, 2023
@ghstahl
Copy link
Author

ghstahl commented Dec 7, 2023 •
edited
Loading

Made Public
use the following branch

  1. Run the server
  2. Run the ClientApp

browse to login
It will take you to a google login.
login.
At this point you will see that the server output is getting hammered.

@mauri870

This comment was marked as off-topic.

Sign in to view
@ghstahl
Copy link
Author

ghstahl commented Dec 7, 2023 •
edited
Loading

made it public
It's about as simple as I can make it.
Easy enough to run and see the errors.

@ghstahl
Copy link
Author

ghstahl commented Dec 8, 2023 •
edited
Loading

more clues:
My server's /token endpoint makes an outbound HTTP POST call. I have passed the echo.Request.Context as well as context.Background() with no better results when making the downstream call.

 
sequenceDiagram
    ClientApp->>Server: POST /token
    Server->>DownstreamServer: POST /token
    DownstreamServer-->>Server: TokenResponse
    Server->>Server: Mint New Tokens
    Server-->>ClientApp: TokenResponse

Doing this makes the client app go into a runaway loop.

I made the following changes and no runaway loop. My downstream server is now just returning a mocked reponse.
Loading 
sequenceDiagram
    ClientApp->>Server: POST /token
    Server->>MockDownstreamServer: POST /token
    MockDownstreamServer-->>Server: TokenResponse
    Server->>Server: Mint New Tokens
    Server-->>ClientApp: TokenResponse

Let's assume I am doing bad stuff in my server. It shouldn't make a client react by hammering me.

@seankhliao
Copy link
Member

This doesn't look like a problem with go oauth libs, and inserting traces into the client doesn't show it making repeated requests.
Instead it appears to be errors within your server application that is causing retries.

Unlike many projects, the Go project does not use GitHub Issues for general discussion or asking questions. GitHub Issues are used for tracking bugs and proposals only.

For questions please refer to https://github.com/golang/go/wiki/Questions

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Dec 8, 2023
@ghstahl
Copy link
Author

ghstahl commented Dec 8, 2023

Confirmed I had a recursion problem on the server side. Thanks for the slap in the head @seankhliao :)

@golang golang locked and limited conversation to collaborators Dec 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@ghstahl @gopherbot @mauri870 @seankhliao