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
180 stars 145 forks source link

Type for Query Param "provider" not always correct #290

Closed tgoodsell-tempus closed 2 years ago

tgoodsell-tempus commented 2 years ago

Describe the bug?

In the SDK, the query param for the Provider is set as a bool. This makes sense in the context of the create user API calls (https://developer.okta.com/docs/reference/api/users/#create-user-with-authentication-provider).

However, in a "Reset Password" API call, particularly for converting a user to a federated user, this needs to be the value "FEDERATION" (https://developer.okta.com/docs/reference/api/users/#request-example-convert-a-user-to-a-federated-user). Which is not possible with the standard safe types.

While this is explicitly a SDK bug, this isn't an issue for non strongly typed languages, I consider this more an upstream API problem since type mixing like this is not a great time, particularly when using a language like Go downstream (what we're doing).

What is expected to happen?

Per https://developer.okta.com/docs/reference/api/users/#request-example-convert-a-user-to-a-federated-user:

Need to pass the value "FEDERATION", which is not possible.

What is the actual behavior?

If you use the value of true (see the create user call), you get the error: the API returned an error: The request was not valid: Invalid value for 'provider' argument

Reproduction Steps?

func ConvertToFederated(id string) error {
    federated := true
    _, _, err := api.User.ResetPassword(ctx, id, &query.Params{Provider: &federated})
    if err != nil {
        return fmt.Errorf("failed to convert to federated: %v", err)
    }
    return nil
}

Additional Information?

No response

Golang Version

go version go1.18.1 darwin/arm64

SDK Version

github.com/okta/okta-sdk-golang/v2 v2.9.2

OS version

No response

monde commented 2 years ago

Thanks for the details @tgoodsell-tempus and steps to reproduce. I will look into this.

tgoodsell-tempus commented 2 years ago

@monde Here is some test results I did using Postman:

POST /api/v1/users/00u16f4n33oV9c7W20h8/lifecycle/reset_password?provider=FEDERATION HTTP/1.1
Authorization: SSWS redacted
User-Agent: PostmanRuntime/7.29.0
Accept: */*
Postman-Token: redacted
Host: tempus.oktapreview.com
Accept-Encoding: gzip, deflate, br
Connection: keep-alive
Content-Length: 0
HTTP/1.1 200 OK
Date: Thu, 14 Apr 2022 18:37:52 GMT
Content-Type: application/json
Transfer-Encoding: chunked
Connection: keep-alive
Server: nginx
Public-Key-Pins-Report-Only: pin-sha256="jZomPEBSDXoipA9un78hKRIeN/+U4ZteRaiX8YpWfqc="; pin-sha256="axSbM6RQ+19oXxudaOTdwXJbSr6f7AahxbDHFy3p8s8="; pin-sha256="SE4qe2vdD9tAegPwO79rMnZyhHvqj3i5g1c2HkyGUNE="; pin-sha256="ylP0lMLMvBaiHn0ihLxHjzvlPVQNoyQ+rMiaj0da/Pw="; max-age=60; report-uri="https://okta.report-uri.com/r/default/hpkp/reportOnly"
Vary: Accept-Encoding
x-okta-request-id: YlhqAAxSU2YNV2jIwTPukQAACE8
x-xss-protection: 0
p3p: CP="HONK"
x-rate-limit-limit: 600
x-rate-limit-remaining: 599
x-rate-limit-reset: 1649961532
cache-control: no-cache, no-store
pragma: no-cache
expires: 0
content-security-policy-report-only: default-src 'self' tempus.oktapreview.com *.oktacdn.com; connect-src 'self' tempus.oktapreview.com tempus-admin.oktapreview.com *.oktacdn.com *.mixpanel.com *.mapbox.com app.pendo.io data.pendo.io pendo-static-5634101834153984.storage.googleapis.com tempus.kerberos.oktapreview.com https://oinmanager.okta.com data:; script-src 'unsafe-inline' 'unsafe-eval' 'self' tempus.oktapreview.com *.oktacdn.com; style-src 'unsafe-inline' 'self' tempus.oktapreview.com *.oktacdn.com app.pendo.io cdn.pendo.io pendo-static-5634101834153984.storage.googleapis.com; frame-src 'self' tempus.oktapreview.com tempus-admin.oktapreview.com login.okta.com; img-src 'self' tempus.oktapreview.com *.oktacdn.com *.tiles.mapbox.com *.mapbox.com app.pendo.io data.pendo.io cdn.pendo.io pendo-static-5634101834153984.storage.googleapis.com data: blob:; font-src 'self' tempus.oktapreview.com data: *.oktacdn.com fonts.gstatic.com; report-uri https://okta.report-uri.com/r/d/csp/reportOnly; report-to csp
content-security-policy: default-src 'self' tempus.oktapreview.com *.oktacdn.com; connect-src 'self' tempus.oktapreview.com tempus-admin.oktapreview.com *.oktacdn.com *.mixpanel.com *.mapbox.com app.pendo.io data.pendo.io pendo-static-5634101834153984.storage.googleapis.com tempus.kerberos.oktapreview.com https://oinmanager.okta.com data:; script-src 'unsafe-inline' 'unsafe-eval' 'self' tempus.oktapreview.com *.oktacdn.com; style-src 'unsafe-inline' 'self' tempus.oktapreview.com *.oktacdn.com app.pendo.io cdn.pendo.io pendo-static-5634101834153984.storage.googleapis.com; frame-src 'self' tempus.oktapreview.com tempus-admin.oktapreview.com login.okta.com; img-src 'self' tempus.oktapreview.com *.oktacdn.com *.tiles.mapbox.com *.mapbox.com app.pendo.io data.pendo.io cdn.pendo.io pendo-static-5634101834153984.storage.googleapis.com data: blob:; font-src 'self' tempus.oktapreview.com data: *.oktacdn.com fonts.gstatic.com; report-uri https://okta.report-uri.com/r/d/csp/enforce; report-to csp
report-to: {"group":"csp","max_age":31536000,"endpoints":[{"url":"https://okta.report-uri.com/a/d/g"}],"include_subdomains":true}
expect-ct: report-uri="https://oktaexpectct.report-uri.com/r/t/ct/reportOnly", max-age=0
x-content-type-options: nosniff
Strict-Transport-Security: max-age=315360000; includeSubDomains
Content-Encoding: gzip
set-cookie: sid=""; Expires=Thu, 01-Jan-1970 00:00:10 GMT; Path=/
set-cookie: autolaunch_triggered=""; Expires=Thu, 01-Jan-1970 00:00:10 GMT; Path=/
set-cookie: JSESSIONID=redacted; Path=/; Secure; HttpOnly

{}
monde commented 2 years ago

@tgoodsell-tempus the fix for this in PR #289 which will hopefully be released tomorrow/Friday as v2.12.0

monde commented 2 years ago

Released in v2.12.0