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

Fix update error #17

Merged
merged 5 commits into from
Jun 21, 2021
Merged

Fix update error #17

merged 5 commits into from
Jun 21, 2021

Conversation

colq2
Copy link
Contributor

@colq2 colq2 commented Mar 12, 2019

This pull request solves #15 .

The problem was parseResponse function from AbstractProvider which force the reponse to be an array and have no possibility to parse application/jwt.
If we use encrypted userinfo endpoint we are getting only the jwt token which is a simple string.

This results in an UnexpectedValueException with the message An OAuth server error was encountered that did not contain a JSON body. Also see AbstractProvider line 694.

@stevenmaguire stevenmaguire self-assigned this Mar 12, 2019
@Airforce111
Copy link

Any chance we can get this merged?

@Zaszczyk
Copy link

Zaszczyk commented Oct 7, 2019

@stevenmaguire can you merge it?

@stevenmaguire
Copy link
Owner

@colq2 Thanks for your patience on this. I have been reluctant to simply merge this as is because of the lack of test cases that demonstrate the specific use case that is being addressed here. Would you mind adding some specific tests to cover this use case?

@colq2
Copy link
Contributor Author

colq2 commented Nov 19, 2019

Yes, I will add some tests soon.

@stevenmaguire
Copy link
Owner

Thank you!

@walva
Copy link

walva commented Jun 9, 2020

@colq2 do you need help on this? What is the current status?

.gitignore Outdated Show resolved Hide resolved
@colq2
Copy link
Contributor Author

colq2 commented Jul 23, 2020

Hello @lobodol and @stevenmaguire,
I looked into it again today. Every test you need is already there. If we remove the parseResponse function from Keycloak.php two test will fail:

image

image

"Invalid response received from Authorization Server. Expected JSON." is an exception which occurs because, as I said, keycloak responses with just a string.

I think there is no reason why you would not merge this.

@stevenmaguire
Copy link
Owner

I'm happy to add another contributor to this project - #27 (comment)

.gitignore Outdated Show resolved Hide resolved
Copy link

@arthurtemple arthurtemple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mstefan21 Any chance to get it merged after all this time? Downstream projects might not be able to update past 2.0.0 because of conflicting dependency constraints.

@mstefan21
Copy link
Collaborator

@arthurtemple Sorry for late reply, I will look at the end of this week and prepare relase of new version

@mstefan21 mstefan21 merged commit 2860f17 into stevenmaguire:master Jun 21, 2021
@mstefan21
Copy link
Collaborator

@arthurtemple Released as version 2.2.2

@arthurtemple
Copy link

@arthurtemple Released as version 2.2.2

Very nice, thanks!

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.

8 participants