ory / sdk

The place where ORY's SDKs are being auto-generated
Apache License 2.0
139 stars 86 forks source link

fix: python client scope field type #223

Closed SavvasMohito closed 1 year ago

SavvasMohito commented 1 year ago

Proposed fix for #220

As described in the issue post, after the update to version 2.0.1, the hydra python client had a misconfiguration which caused the token change flow not to work at all. This had to do with the type of the "scope" value the client expects to receive from hydra.

Based on the documentation and the OAuth2 standard, the scope field should be a string value containing space-separated entries. Instead, the new client update proposed it to have a value of int, thus causing the client not to work properly with hydra because of this type mismatch.

Tests

I have applied this proposed change on my local hydra python client and I can confirm it works as expected with hydra v2.0.1.

Checklist

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

jonas-jonas commented 1 year ago

Hi @SavvasMohito, thanks for your contribution! Unfortunately, these files are automatically generated from our OpenAPI schemas. So your change would be overwritten the next time our generation is run.

We need to investigate why the generator outputs the wrong type here and adjust either 1) our schema or 2) fix the generator. My guess is, that we should be able to adjust our schema accordingly, but that needs to be done in https://github.com/ory/hydra.

The definition in hydra is here: https://github.com/ory/hydra/blob/925013e39508c22d1038560ef552a0764f2b0177/oauth2/handler.go#L827 It looks like int is the expected type, so we need to figure out why the SDK tries to input a str instead. If you're up for the task, feel free to contribute in the hydra repository. :)

aeneasr commented 1 year ago

Oh the problem is that

https://github.com/ory/hydra/blob/925013e39508c22d1038560ef552a0764f2b0177/oauth2/handler.go#L827

should actually be a string, not an int!

SavvasMohito commented 1 year ago

Well, I guess we finally found the root of the problem, good call @jonas-jonas! As @aeneasr said, this should definitely be a string and not an integer value. I assume you guys are going to fix this, but let me know if you'd prefer I make a PR for that as well. Thank you for looking into this.

jonas-jonas commented 1 year ago

Feel free to open a PR in hydra! :) Contributions are always welcome.

SavvasMohito commented 1 year ago

PR created under the hydra repo https://github.com/ory/hydra/pull/3337