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

Forward success response to fetch_raw_info callback (#47) #51

Merged

Conversation

danschmidt5189
Copy link
Collaborator

@danschmidt5189 danschmidt5189 commented Jul 13, 2018

Exposes ServiceTicketValidator.success_body and passes it as a fifth argument to the user-supplied fetch_raw_info callback. This gives new clients access to the raw CAS XML response, allowing them to solve issues like #47 without breaking parsing for existing clients.

There were previously no tests for the fetch_raw_info callback. That is now implicitly tested by checks on new cas:roles fixture elements, which are populated by that callback.

@danschmidt5189 danschmidt5189 force-pushed the 47-squashing-multivalued-fields branch 2 times, most recently from 93f2eb8 to 2a72d4c Compare July 15, 2018 19:31
@vjt
Copy link
Collaborator

vjt commented Jul 18, 2018

Thanks for this. I have no objection to merge, I'll wait to hear from other collaborators.

Thanks!

@vjt vjt requested review from vjt, dlindahl and mpeteuil July 18, 2018 09:40
Forwards the XML response received from CASAuth to the fetch_raw_info
callback, which now accepts five arguments (cas, options, ticket, user
info, and the raw response). Developers who override fetch_raw_info with
a Proc are unaffected -- the new argument is silently discarded -- but
developers who use a Lambda will need to update their code.
@danschmidt5189 danschmidt5189 force-pushed the 47-squashing-multivalued-fields branch from 2a72d4c to 5fcb389 Compare July 18, 2018 17:18
@danschmidt5189
Copy link
Collaborator Author

@vjt We tried this out on a few internal apps and were reminded that it's a breaking change for developers using lambda, which has an arity check. Developers using Proc are unaffected. I updated the README accordingly.

That might also be worth noting in the Changelog, but I'll leave that to you and the other maintainers.

Thank you!

@danschmidt5189 danschmidt5189 changed the title WIP: Forward success response to fetch_raw_info callback (#47) Forward success response to fetch_raw_info callback (#47) Feb 14, 2019
@danschmidt5189 danschmidt5189 merged commit 7087bda into dlindahl:master Feb 14, 2019
This was referenced Feb 14, 2019
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

Successfully merging this pull request may close these issues.

2 participants