Skip to content

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

Merged
spacewander merged 3 commits into
apache:masterfrom
tzssangglass:ake
Apr 18, 2022
Merged

fix(authz-keycloak): do not expose internal errors to the client#6854
spacewander merged 3 commits into
apache:masterfrom
tzssangglass:ake

Conversation

@tzssangglass

Copy link
Copy Markdown
Member

Description

Fixes # (issue)

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

local err = "username is missing."
log.error(err)
return 422, err
return 422, {message = err}

Copy link
Copy Markdown
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
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

add

local err = "password is missing."
log.error(err)
return 422, err
return 422, {message = err}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ditto

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

add

Comment thread apisix/plugins/authz-keycloak.lua Outdated
end
if not password then
local err = "password is missing."
log.error(err)

Copy link
Copy Markdown
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
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed

Comment thread t/plugin/authz-keycloak.t Outdated



=== TEST 19: no username of password

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
=== TEST 19: no username of password
=== TEST 19: no username or password

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.

3 participants