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

Panic on requests with private key auth when DPoP is disabled #461

Closed
az-zemr opened this issue May 28, 2024 · 6 comments · Fixed by #463
Closed

Panic on requests with private key auth when DPoP is disabled #461

az-zemr opened this issue May 28, 2024 · 6 comments · Fixed by #463
Labels
bug Something isn't working Triaged waiting-response

Comments

@az-zemr
Copy link

az-zemr commented May 28, 2024

Describe the bug?

With SDK version 4.1.0, every second call to API results in panic.

The root cause: https://github.com/okta/okta-sdk-golang/pull/454/files#diff-9f03089e1a25ed798b9db4fd587bb8a9a4f94eca8df0234aef9ac2b6f298b02cR197

in Authorize, nil value for key is received from getAccessTokenForPrivateKey (I assume, that only happens when dpop is disabled, which is my case). This value is then stored in cache, just like the empty nonce.

On next call, nil privateKey value is retrieved from cache and provided to generateDpopJWT, which causes code to panic.

So, there are two issues actually:

  • SDK should be aware that dpop is optional and not panic
  • Cache should check empty/nil values (or they simply shouldn't be stored there at least)

What is expected to happen?

Subsequent calls to API methods work, just like in previous version

What is the actual behavior?

panic: runtime error: invalid memory address or nil pointer dereference

[signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x1016886d8]

goroutine 57 [running]:
github.com/okta/okta-sdk-golang/v4/okta.generateDpopJWT(0x0, {0x1030071c0, 0x3}, {0x1400026e180, 0x25}, {0x0, 0x0}, {0x14000c86387, 0x34e})

Reproduction Steps?

  • call any function
  • call any function again

In my case it was users listing with pagination that broke everything.

Additional Information?

No response

Golang Version

go version go1.21.3 darwin/arm64

SDK Version

v4.1.0

OS version

No response

@az-zemr az-zemr added the bug Something isn't working label May 28, 2024
@duytiennguyen-okta
Copy link
Contributor

@ArikWiz
Copy link

ArikWiz commented Jun 6, 2024

Hi @duytiennguyen-okta
In v4.1.1 i'm also seeing panics related to requests with private key when DPoP is disabled
this one slightly different than the panic reported here.
When DPoP is enabled, there are no panics.

/usr/local/go/src/runtime/panic.go:770 +0x124

github.com/okta/okta-sdk-golang/v4/okta.(*PrivateKeyAuth).Authorize(0x4002482460, {0x32f185a, 0x3}, {0x4002a60a20, 0x30})

	/go/pkg/mod/github.com/okta/okta-sdk-golang/v4@v4.1.1/okta/client.go:339 +0x57c
github.com/okta/okta-sdk-golang/v4/okta.(*APIClient).prepareRequest(0x40033af688, {0x5176a38, 0x4003879b90}, {0x4002a60930, 0x26}, {0x32f185a, 0x3}, {0x0?, 0x0?}, 0x400473abb8, ...)

	/go/pkg/mod/github.com/okta/okta-sdk-golang/v4@v4.1.1/okta/client.go:1016 +0x1210
github.com/okta/okta-sdk-golang/v4/okta.(*SystemLogAPIService).ListLogEventsExecute(0x40033af690, {{0x5176a38, 0x4003879b90}, {0x5167bd8, 0x40033af690}, 0x4001a878f0, 0x0, 0x0, 0x0, 0x4006e4b5a0, ...})

@duytiennguyen-okta
Copy link
Contributor

@ArikWiz do you have the log? My guess is it panic because it couldn't get the accessToken

@ArikWiz
Copy link

ArikWiz commented Jun 6, 2024

thank you for the quick response

it is a nil pointer deref:

runtime error: invalid memory address or nil pointer dereference

and the backtrace is:

runtime/debug.Stack()
	/usr/local/go/src/runtime/debug/stack.go:24 +0x64
......
	/usr/local/go/src/runtime/panic.go:770 +0x124
github.com/okta/okta-sdk-golang/v4/okta.(*PrivateKeyAuth).Authorize(0x4001cdf180, {0x32f357a, 0x3}, {0x4003208ff0, 0x30})
	/go/pkg/mod/github.com/okta/okta-sdk-golang/v4@v4.1.1/okta/client.go:339 +0x57c
github.com/okta/okta-sdk-golang/v4/okta.(*APIClient).prepareRequest(0x4004e79b08, {0x5179638, 0x4004f06b60}, {0x4003208ea0, 0x26}, {0x32f357a, 0x3}, {0x0?, 0x0?}, 0x400591abb8, ...)
	/go/pkg/mod/github.com/okta/okta-sdk-golang/v4@v4.1.1/okta/client.go:1016 +0x1210
github.com/okta/okta-sdk-golang/v4/okta.(*SystemLogAPIService).ListLogEventsExecute(0x4004e79b10, {{0x5179638, 0x4004f06b60}, {0x516a7d8, 0x4004e79b10}, 0x4001ab7c20, 0x0, 0x0, 0x0, 0x4005a4e670, ...})
	/go/pkg/mod/github.com/okta/okta-sdk-golang/v4@v4.1.1/okta/api_system_log.go:205 +0xb34
github.com/okta/okta-sdk-golang/v4/okta.ApiListLogEventsRequest.Execute(...)
	/go/pkg/mod/github.com/okta/okta-sdk-golang/v4@v4.1.1/okta/api_system_log.go:104

@duytiennguyen-okta
Copy link
Contributor

@ArikWiz Sorry I mean do you have log for the API call? I want to know why dpop works but bearer doesn't because I have tested both case. Line 335 should also catch if there is error when getting the accessToken, Dpop or Bearer

@duytiennguyen-okta
Copy link
Contributor

Close with #466

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Triaged waiting-response
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants