fix: re-authorize during retries #562
Conversation
| // Always rewind the request body when non-nil. | ||
| if bodyReader != nil { | ||
| req.Body = bodyReader() | ||
| if bOff.retryCount > 0 && (c.cfg.Okta.Client.AuthorizationMode == "PrivateKey" || c.cfg.Okta.Client.AuthorizationMode == "JWT") { |
There was a problem hiding this comment.
Difference from https://github.com/okta/terraform-provider-okta/pull/2421/files, we only clear the token when retryCount > 0 otherwise the token is generated on each initial request which is wasteful I think
| headerParams := make(map[string]string) | ||
| queryParams := req.URL.Query() | ||
| req.URL.RawQuery = "" | ||
| auth, err := c.prepareRequest(ctx, req.URL.String(), req.Method, nil, headerParams, queryParams, url.Values{}, []formFile{}) |
There was a problem hiding this comment.
I had to use the full URL here without the query otherwise I got an error that the htu claim is invalid
| } | ||
| operation := func() (*http.Response, error) { | ||
| // Always rewind the request body when non-nil. | ||
| if bodyReader != nil { |
There was a problem hiding this comment.
Another difference from https://github.com/okta/terraform-provider-okta/pull/2421/files, this is already done inside the if and else so I removed the top level req.Body = bodyReader()
| req.Body = bodyReader() | ||
| if bOff.retryCount > 0 && (c.cfg.Okta.Client.AuthorizationMode == "PrivateKey" || c.cfg.Okta.Client.AuthorizationMode == "JWT") { | ||
| // Clear the token cache to force fresh authorization | ||
| // This will get a new access token and potentially a new nonce |
There was a problem hiding this comment.
I'm not sure how this works with concurrent API requests, don't they share the cache so we can have race conditions here?
There was a problem hiding this comment.
I think the impact is that tokens can be generated more than needed:
- Request 1 retries, clears cache, calls
prepareRequestwhich generates a new token - Concurrently request 2 retires, clears cache, generates new token but it could have used the token from ⬆️
Regardless it seems the token cache is mostly best effort as multiple concurrent requests can override the cache at anytime if a cache entry had expired
1 participant
I already signed the CLA a while back for https://github.com/okta/okta-sdk-golang/pulls?q=is%3Apr+author%3Aerezrokah+is%3Aclosed. I can send it again if you don't have it
Summary
Fixes #554
Instead of #555 per #555 (comment)
Type of PR
Test Information
Go Version:
go version go1.25.4 darwin/arm64Os Version: MacOS 26.1
OpenAPI Spec Version:
Signoff
make fmton my code