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

return bad request instead of server error for identity group cycle detection #15912

Merged
merged 5 commits into from Jun 10, 2022

Conversation

ccapurso
Copy link
Contributor

@ccapurso ccapurso commented Jun 9, 2022

Providing member_group_ids that contains a cycle for an identity group to identity/group/name/<name> will currently result in a 500 response. This should instead be a 400 response stating that a cycle was detected.

Reproduction steps:

Create groups:

ID_Group_0=`vault write -format=json identity/group/name/Group_0 | jq -r ".data.id"
ID_Group_1=`vault write -format=json identity/group/name/Group_1 | jq -r ".data.id"
ID_Group_2=`vault write -format=json identity/group/name/Group_2  | jq -r ".data.id"

Specify group IDs for Group_0 and Group_1:

vault write identity/group/name/Group_0 member_group_ids= 
vault write identity/group/name/Group_1 member_group_ids=$ID_Group_2 

Specify group IDs for Group_2 will fail due to cycle:

vault write identity/group/name/Group_2 member_group_ids=$ID_Group_0,$ID_Group_1 

Error writing data to identity/group/name/Group_2: Error making API request.

URL: PUT http://127.0.0.1:8200/v1/identity/group/name/Group_2
Code: 500. Errors:

1 error occurred:
	* failed to perform cyclic relationship detection for member group ID "c5ac421e-0748-8b7e-466b-5a951542c2f8"

This PR introduces improved error handling such that an error caused by a cycle detection can be handled more specifically by a request handler. This allows for returning a logical.ErrorResponse rather than bubbling up the error to Vault's response handling logic and resulting in a 500 as so:

vault write identity/group/name/Group_2 member_group_ids=$ID_Group_0,$ID_Group_1 

Error writing data to identity/group/name/Group_2: Error making API request.

URL: PUT http://127.0.0.1:8200/v1/identity/group/name/Group_2
Code: 400. Errors:

* cyclic relationship detected for member group ID "752b6146-2cc3-b3f5-8ab6-9feea4da98b5"

@ccapurso ccapurso marked this pull request as ready for review June 9, 2022 19:00
@ccapurso ccapurso requested review from a team June 9, 2022 19:00
@ccapurso
Copy link
Contributor Author

ccapurso commented Jun 9, 2022

It looks like some tests failed, looking into those.

Copy link
Contributor

@hghaf099 hghaf099 left a comment

Choose a reason for hiding this comment

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

Asked a question, otherwise looks good!

changelog/15912.txt Show resolved Hide resolved
@ccapurso ccapurso merged commit efdf2f6 into main Jun 10, 2022
@ccapurso ccapurso deleted the vault-1024-identity-group-cycle-detection branch June 10, 2022 14:15
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.

None yet

2 participants