Make group auth check case-insensitive (including Copilot's recommendations regarding null pointer checks) #11687
Conversation
…ations regarding null pointer checks)
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a case-sensitivity issue in the group-based authentication system where cancer study access was incorrectly denied due to case mismatches between authorities and cancer study groups. The fix replaces the case-sensitive Collections.disjoint with a new case-insensitive utility method that also includes null pointer safety checks.
- Replaces case-sensitive group comparison with case-insensitive logic
- Adds null safety checks for both collections and individual elements
- Maintains the same logical behavior while fixing the case-sensitivity bug
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (c1 == null || c2 == null) { | ||
| return true; // If either collection is null, they are considered disjoint. | ||
| } | ||
| Set<String> upperC1 = c1.stream().filter(Objects::nonNull).map(String::toUpperCase).collect(Collectors.toSet()); |
There was a problem hiding this comment.
Missing import for Objects class. Add import java.util.Objects; to the imports section to resolve the compilation error.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Copilot statement incorrect. The import is contained in “import java.util.*;”.
1 participant
Same as pull request #11610 but also including the Copilot's recommendations regarding null pointer checks.
@inodb and @haynescd
Because the user "akulyakhtin" is not answering and not changing the code at his own pull request 11610, I have created a pull request on my on now.
Description is still the same:
Closes #11609
This request fixes the following issue:
if we have set authorities=true in app properties
and if we have a group value in cancer_study table sample_group
and if we have authority sample_group (or cbioportal:sample_group, depending on the filtering option) in authorities table for a user.
Then the access to that cancer study is not granted to the user even though the authroity matches the cancer study group.
This happens because authorities are convereted to upper case while cancer group names are not.
To fix this the pull request changes Collections.disjoint (which is case-sensitive) to a case-insensitive utility method caseInsensitiveDisjoint.