okta / okta-sdk-golang

A Golang SDK for interacting with the Okta management API, enabling server-side code to manage Okta users, groups, applications, and more.
https://github.com/okta/okta-sdk-golang
Other
177 stars 143 forks source link

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

Closed az-zemr closed 3 months ago

az-zemr commented 4 months ago

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:

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?

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

duytiennguyen-okta commented 4 months ago

https://oktainc.atlassian.net/browse/OKTA-733550

ArikWiz commented 4 months ago

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 commented 4 months ago

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

ArikWiz commented 4 months ago

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 commented 4 months ago

@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 commented 3 months ago

Close with #466