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

KNOX-2396 - TokenResource Renew and Revoke Should Respond With Error Codes That Identify Specific Errors #480

Merged
merged 1 commit into from Aug 25, 2021

Conversation

zeroflag
Copy link
Contributor

@zeroflag zeroflag commented Aug 11, 2021

What changes were proposed in this pull request?

TokeResource currently responds to failed requests to renew or revoke a token with an error message. Any client behavior based on the nature of the failure must currently depend on the error message content, which is not robust.

This patch extends the response with an error code, which is backward compatible with the existing responses.

How was this patch tested?

Created a token with API call:

$ curl -v -k -u admin:admin-password  https://localhost:8443/gateway/homepage/knoxtoken/api/v1/token

Tried renew/revoke/enable/disable endpoints:

$ curl -X PUT -d "<tokenId>" -vvv -k -u admin:admin-password https://localhost:8443/knoxtoken/api/v1/token/enable

{
  "setEnabledFlag": "false",
  "error": "Token is already enabled",
  "code": 70
}
$ curl -X PUT -d "<tokenId>" -vvv -k -u admin:admin-password https://localhost:8443/knoxtoken/api/v1/token/disable
{
  "setEnabledFlag": "false",
  "error": "Token is already disabled",
  "code": 60
}
$ curl -d "invalid" -vvv -k -u admin:admin-password https://localhost:8443/knoxtoken/api/v1/token/revoke
{
  "revoked": "false",
  "error": "Invalid serialized unsecured/JWS/JWE object: Missing part delimiters",
  "code": 40
}
curl -d "invalid" -vvv -k -u admin:admin-password https://localhost:8443/knoxtoken/api/v1/token/renew
{
  "renewed": "false",
  "error": "Invalid serialized unsecured/JWS/JWE object: Missing part delimiters",
  "code": 40
}

@zeroflag
Copy link
Contributor Author

Copy link
Contributor

@smolnar82 smolnar82 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@pzampino pzampino left a comment

Choose a reason for hiding this comment

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

LGTM
We should add these codes to the Knox user guide for them to be really useful.

@pzampino pzampino merged commit 9ee06f3 into apache:master Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants