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

Update Dependencies and address defaultGlobalReadOnly param #2305

Merged
merged 3 commits into from
Aug 29, 2023

Conversation

chrisknoll
Copy link
Collaborator

@chrisknoll chrisknoll commented Aug 28, 2023

Updated circe to version 1.11.1
Updated StandardAnalysisUtils to 1.4.0.
Fixed the assignment of defaultGlobalReadOnly field.

@chrisknoll
Copy link
Collaborator Author

@rkboyce , just FYI, I found something odd in how the permissions were being specified...although I did the previous PR and tested the functionality, I didn't pay close enough to what the value of the paramter was vs. what it represented (ie, was testing turning it on and off to test the difference but didn't look closely at the name).

In this case, We are talking about the setting security.defaultGlobalReadPermissions, which sounds like if it is true, you would be granted global read permissions by default. But what I found in code was a sort of reversal:

	@Value("#{'${security.defaultGlobalReadPermissions}'.equals(false)}")
	private boolean defaultGlobalReadPermissions;

So it's storing the field 'defaultGlobalReadPermissions as the opposite value that was specified in the config...which feels wrong. I've updated it here:

	@Value("${security.defaultGlobalReadPermissions}")
	private boolean defaultGlobalReadPermissions;

I am pinging you here because you may have this deployed in your own environment where you are reversing the value (true vs. false) in your own config, and I just wanted to let you know of the correction I'm making here.

@chrisknoll chrisknoll merged commit fe070e5 into master Aug 29, 2023
2 checks passed
@delete-merged-branch delete-merged-branch bot deleted the standardizedAnalysisUtils_v1.4.0 branch August 29, 2023 13:03
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