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(wolf-rbac): Return 403 error code when the user does not have per… #7497

Merged
merged 1 commit into from
Jul 24, 2022

Conversation

iGeeky
Copy link
Contributor

@iGeeky iGeeky commented Jul 19, 2022

…mission.

Description

Return 403 error code when the user does not have permission.
It is more reasonable to return 401 error code when user does not have permission

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)

@tzssangglass
Copy link
Member

401 is no authentication information or wrong authentication information, then the client can modify the authentication information to retry;

403 is the client with the correct authentication information, but the server believes that the authentication information corresponding to the user does not have the corresponding resource access rights, so there is no need to retry before obtaining the relevant rights from the administrator.

The logic of using 401 instead of 403 here is actually to hide the message. It is possible for an attacker to determine whether the account password is correct by judging the status code.

@iGeeky
Copy link
Contributor Author

iGeeky commented Jul 20, 2022

@tzssangglass

403 is the client with the correct authentication information, but the server believes that the authentication information corresponding to the user does not have the corresponding resource access rights, so there is no need to retry before obtaining the relevant rights from the administrator.

Yes, this is the case for the place I want to change. So it should use 403.

The logic of using 401 instead of 403 here is actually to hide the message. It is possible for an attacker to determine whether the account password is correct by judging the status code.

I don't understand here. The place to change is when accessing resources, which should have nothing to do with authentication. So there will be no account password operation.

@tzssangglass
Copy link
Member

The place to change is when accessing resources, which should have nothing to do with authentication. So there will be no account password operation.

If it returns 403, then the reverse proves that the account password is correct, just without permissions.

This is to prevent password blasting.

Previously, APISIX had a focused change to the auth class plugin to reduce the amount of internal information exposed to the client.

@tzssangglass
Copy link
Member

But I'm not so strict about it, so let's hear what others think.

I think 403 can give better support to the client in terms of semantics. The security issues it may cause are less important than changing from 401 to 403.

@spacewander spacewander merged commit 1771c51 into apache:master Jul 24, 2022
Liu-Junlin pushed a commit to Liu-Junlin/apisix that referenced this pull request Nov 4, 2022
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