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 an edge case bug that "identity_policies" is nil #17007

Merged
merged 7 commits into from
May 28, 2024
Merged

fix an edge case bug that "identity_policies" is nil #17007

merged 7 commits into from
May 28, 2024

Conversation

tianhaopx
Copy link
Contributor

@tianhaopx tianhaopx commented Sep 3, 2022 •
edited
Loading

Fixes #17006

client side: check if "identity_policies" is nil, goto DONE, then cli vault login will not return unexpected error.

server side: check if key out.NamespaceID exists in the map identityPolicies returned from func fetchEntityAndDerivedPolicies

@hashicorp-cla
Copy link

hashicorp-cla commented Sep 3, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@tianhaopx tianhaopx mentioned this pull request Sep 3, 2022
Closed
@tianhaopx
add changelog
5ac93a9
@tianhaopx
skip add identity_policies to resp when there's no identity_policies …
c0615ea 
…associated in token's namespace

This is an edge case, when an entity has identity_policies associated in other namespaces but no identity_policies in this token's namespace, `identityPolicies[out.NamespaceID]` is nil, client side doesn't handle nil which raises error.
@tianhaopx
update changelog
bc8b510
@tianhaopx tianhaopx changed the title check if "identity_policies" is nil to fix cli vault login error fix an edge case bug that "identity_policies" is nil Sep 4, 2022
@peteski22
Copy link

Hi @tianhaopx, thanks for the submission. It looks good, I just wonder if you might be able to add a test to secret_test.go to check the return from TokenPolicies? I think we're good to merge it in after that. :)

@heatherezell heatherezell added core/identity bug Used to indicate a potential bug labels Sep 9, 2022
VioletHynes
VioletHynes approved these changes May 28, 2024
View reviewed changes
Copy link
Contributor

@VioletHynes VioletHynes left a comment

Choose a reason for hiding this comment

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

Approving and merging this, thank you for your commit!

While it would be better with a test, this seems to fix a legitimate bug, so I'd rather get it merged than not. We really appreciate the contribution!

@VioletHynes VioletHynes merged commit 2a1775f into hashicorp:main May 28, 2024
66 of 67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VioletHynes VioletHynes VioletHynes approved these changes

Assignees
No one assigned
Labels
bug Used to indicate a potential bug core/identity
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

vault cli login using token error: unable to convert identity policies to expected format
6 participants
@tianhaopx @hashicorp-cla @peteski22 @VioletHynes @heatherezell @biazmoreira