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

Improve provider connection: allow ommited "/auth" and improve login-error-messages #332

Merged
merged 2 commits into from
Jul 14, 2020

Conversation

elmarx
Copy link
Contributor

@elmarx elmarx commented Jun 30, 2020

Hey,

Strictly speaking this PR addresses two distinct issues, but both are rather small.

First, the provider previously did not check the status-code of the initial login, so if some reverse-proxy returns an error and a non-JSON-error-message, the error-message is just about a parsing failure. By explicitly checking the HTTP-status-code, the provider can show more precise error messages.

The second commit enables to use keycloak at another web-context then "/auth" (e.g. to leave out "/auth" completely).
Maybe it would be nicer to just use the setting baseUrl and require users to include "/auth" if necessary, but that would introduce a breaking change, so I opted to add an additional optional setting to override the "/auth" part.

@elmarx elmarx force-pushed the improved-provider-connection branch from c56cf1e to 93a4e39 Compare July 1, 2020 07:02
If the login to keycloak fails with a non-JSON-response (e.g. due to a
reverse-proxy returning 502 or 403 etc.), the
error-message is rather arcane.

By checking the status-code this commit turns
"Error: invalid character '<' looking for beginning of value" into
"Error: error sending POST request to http:///localhost:8080/auth/realms/master/protocol/openid-connect/token: 403 Forbidden"
@elmarx elmarx force-pushed the improved-provider-connection branch from 93a4e39 to ac015ef Compare July 6, 2020 16:04
Copy link
Contributor

@mrparkers mrparkers left a comment

Choose a reason for hiding this comment

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

LGTM!

@mrparkers mrparkers merged commit eb1b1eb into keycloak:master Jul 14, 2020
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