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.

Sign up for GitHub

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

Jump to bottom

fix(authz-keycloak): do not expose internal errors to the client #6854

Merged
merged 3 commits into from
Apr 18, 2022
Merged

fix(authz-keycloak): do not expose internal errors to the client #6854

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
05761fd
fix(authz-keycloak): do not expose internal errors to the client
tzssangglass Apr 15, 2022
4d541ad
fix code review
tzssangglass Apr 17, 2022
b47cd43
fix review
tzssangglass Apr 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions apisix/plugins/authz-keycloak.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -722,12 +722,12 @@ local function generate_token_using_password_grant(conf,ctx)
if not username then
local err = "username is missing."
log.error(err)
return 422, err
return 422, {message = err}
Copy link
Member

Choose a reason for hiding this comment

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

Would be better to add test to cover this branch?

Copy link
Member Author

Choose a reason for hiding this comment

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

add

end
if not password then
local err = "password is missing."
log.error(err)
Copy link
Member

Choose a reason for hiding this comment

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

Shall we use log level warn instead of error level with 4xx status code ?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

return 422, err
return 422, {message = err}
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

Copy link
Member Author

Choose a reason for hiding this comment

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

add

end

local client_id = authz_keycloak_get_client_id(conf)
Expand All @@ -737,7 +737,7 @@ local function generate_token_using_password_grant(conf,ctx)
if not token_endpoint then
local err = "Unable to determine token endpoint."
log.error(err)
return 503, err
return 503, {message = err}
end
local httpc = authz_keycloak_get_http_client(conf)

Expand All @@ -763,7 +763,7 @@ local function generate_token_using_password_grant(conf,ctx)
err = "Accessing token endpoint URL (" .. token_endpoint
.. ") failed: " .. err
log.error(err)
return 401, {message = err}
return 401, {message = "Accessing token endpoint URL failed."}
end

log.debug("Response data: " .. res.body)
Expand All @@ -773,7 +773,7 @@ local function generate_token_using_password_grant(conf,ctx)
err = "Could not decode JSON from response"
.. (err and (": " .. err) or '.')
log.error(err)
return 401, {message = err}
return 401, {message = "Could not decode JSON from response."}
end

return res.status, res.body
Expand Down