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

LogRequest and LogEvent.Target not according to spec; LogEvent.GetLogs crashes in unmarshalling #65

Closed
gtourkas opened this issue May 31, 2019 · 3 comments

Comments

@gtourkas
Copy link

Hi !

According to spec:

  • LogEvent.Target should be of type []*LogTarget instead of []string
  • LogRequest.IpChain should be of type []*LogIpAddress instead of []string

Unfortunately this is not the case and GetLogs returns an unmarshalling error for the LogEvent.Target and LogEvent.Request fields.

Tried to re-generate the SDK to submit a PR but it seems that https://github.com/okta/openapi is turned private and as such, cannot get hold of the okta-sdk-generator tool.

Thanx.

@johanbrandhorst
Copy link
Contributor

I think both of the mentioned issues have been fixed, however, LogEvent.GetLogs is still failing to unmarshal the response, due to the incorrect use of the authenticationContext LogAuthenticationContext.CredentialProvider. In the generated client it is a slice of *LogCredentialProvider which does not line up with the definitions on the documentation (https://developer.okta.com/docs/reference/api/system-log/#logevent-object), which claims it is one of a defined enumeration of string values or JSON null. The same goes for the CredentialType in the same type.

I think both of these errors stem from these lines: https://github.com/okta/okta-sdk-golang/blob/master/openapi/spec.json#L4957-L4970. This file does not seem to have been updated in 2 years so there may be more errors.

It also appears incorrect in the Java SDK: https://github.com/okta/okta-sdk-java/blob/master/src/swagger/api.yaml#L3699-L3708.

I'd be happy to fix this if all it is is updating the OpenAPI spec.

@robertjd
Copy link

Thanks @johanbrandhorst for the info. We'll get this fixed shortly, we're actually working on a new major rev of all our SDKs with an updated OpenAPI spec. We currently have a 2.x RC of this SDK available, see the README for details. It doesn't yet include a fix for this issue, but I just fixed it in our spec so you'll see it come to this SDK before we have the official 2.x release.

@bretterer
Copy link
Collaborator

With v2 release, this is now resolved. If you continue to have any issues, please re-open this issue, or create a new one for us to look at. Thank you for your report here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants