Breaking change introduced in version 1.11.4

ahx commented 2 years ago

Preflight checklist

[X] I could not find a solution in the existing issues, docs, nor discussions.
[X] I agree to follow this project's Code of Conduct.
[X] I have read and am following this repository's Contribution Guidelines.
[ ] This issue affects my Ory Cloud project.
[X] I have joined the Ory Community Slack.
[X] I am signed up to the Ory Security Patch Newsletter.

Describe the bug

A breaking change was made in the method signature of OryHydraClient::AdminApi#accept_consent_request in version 1.11.4. This caused a problem in one of our systems after updating ory-hydra-client from version 1.10.6 to version 1.11.4.

The breaking change was introduced in this commit.

Whereas in version 1.10.6 the method should be called with a :body option like this:

admin_api.accept_consent_request(consent_challenge, body: accept_consent_request)

since version 1.11.4 is has to be called with a :accept_consent_request option like that:

admin_api.accept_consent_request(consent_challenge, accept_consent_request: accept_consent_request)

The same kind of breaking change was made for OryHydraClient::AdminApi#accept_login_request.

Reproducing the bug

Integrate Hydra using ory-hydra-client for Ruby version 1.10.6
The system works ✨
Update ory-hydra-client to version >= 1.11.4
The system fails with Hydra returning a 400, because unexpected parameters are sent. 💔

You cannot really catch this in a unit test without mocking the HTTP request, because the implementation does use an options Hash instead of named arguments or the like. So the requests goes all through to Hydra, which responds with 400.

Relevant log output

No response

Relevant configuration

No response

Version

>= 1.11.4

On which operating system are you observing this issue?

Linux

In which environment are you deploying?

Kubernetes with Helm

Additional Context

While the new method signature makes total sense, a major version change should have indicated the breaking change.

ahx commented 2 years ago

I have not traced down the issue to the change in the generator, but I assume it has something to do with the updated version of the openapi-generator.

gen1us2k commented 2 years ago

You can find usage examples of admin API here

require 'time'
require 'ory-hydra-client'

api_instance = OryHydraClient::AdminApi.new
consent_challenge = 'consent_challenge_example' # String | 
opts = {
  accept_consent_request: OryHydraClient::AcceptConsentRequest.new # AcceptConsentRequest | 
}

begin
  # Accept a Consent Request
  result = api_instance.accept_consent_request(consent_challenge, opts)
  p result
rescue OryHydraClient::ApiError => e
  puts "Error when calling AdminApi->accept_consent_request: #{e}"
end

According to your example you need to pass accept_consent_request using dict

opts = {accept_consent_request: accept_consent_request}

admin_api.accept_consent_request(consent_challenge, opts)

aeneasr commented 2 years ago

Sorry about those breaking changes, we upgraded OpenAPI Generator to a newer version in order to fix some pesky bugs n the Ory Kratos generated SDK, and it appears to have introduced breaking changes in this SDK! Regarding actually resolving the issue, @gen1us2k has you covered :)

ahx commented 2 years ago

@gen1us2k, @aeneasr We were able to fix the issue after seeing our application fail pretty quickly. But thanks for the info. Do you have a strategy how to avoid / detect these breaking changes in the future?

aeneasr commented 2 years ago

Probably tests? 😅 We have a Cypress test suite i. hydra you might be able to reuse

ory / hydra-client-ruby