Skip to content

Storagepassword's wildcard check for create and delete actions #187

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

Merged
merged 4 commits into from
Jul 20, 2022

Conversation

ashah-splunk
Copy link
Contributor

Added check for presence of wildcard's in 'owner' and/or 'app' while creating or deleting a StoragePassword. Check and error message is kept consistent with the other SDKs.

Added check for wildcards in 'app' and 'owner' while creating/removing a StoragePassword
return super.remove(String.format("%s:%s:", realm, name));
}

@Override
public Password remove(String key) {
if(checkForWildcards()){

Choose a reason for hiding this comment

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

It might be missing context here, but why do we have to check for the wildcard when removing the password? What happens if I'm trying to remove a password that has - in it given that passwords with - have been allowed till now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we check for wildcards present in 'app' and 'owner' properties and not in the password itself. As we need 'app' and 'owner' values to determine from where the StoragePassword is being delete.

changed 'Boolean' to 'boolean'
@ashah-splunk ashah-splunk requested a review from vupadhyay July 12, 2022 16:11
@ashah-splunk ashah-splunk merged commit cf26ad8 into develop Jul 20, 2022
@ashah-splunk ashah-splunk deleted the storagepasswords-wildcard-check branch November 29, 2022 11:39
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.

2 participants