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

fix: improper null checks on the CustomAuthenticationConverter #137

Closed
wants to merge 2 commits into from
Closed

Conversation

m-gora
Copy link
Contributor

@m-gora m-gora commented Oct 31, 2023

Description

The CustomAuthenticationConverter performed typecasting before the null checks, which would result in a NullPointerException if the return value before the cast was null. To remedy this issue I refactored the code block to use a functional style Optional object, which automatically returns an empty Set of GrantedAuthorities if any of value in the json tree is null or the return value does not match the desired type of Map<String, Object>.

Note that this converter is used to perform the authentication. So fixing this bug is security relevant.

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

@DominikPinsel
Copy link
Contributor

@m-gora Great contribution :) It would be great if you could sign the Eclipse CLA with the same email, you did the commit with.

See also

@m-gora
Copy link
Contributor Author

m-gora commented Nov 10, 2023

Hi @DominikPinsel ,
I signed the agreement. Does the check rerun automatically once everything is in place?
I can't seem to force a rerun manually.

Copy link

sonarcloud bot commented Nov 14, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@DominikPinsel
Copy link
Contributor

@borisrizov-zf Can you approve the workflow? :)

@borisrizov-zf
Copy link
Contributor

borisrizov-zf commented Nov 22, 2023

@m-gora can you please rebase and change this to merge into develop.

Basic rebase workflow (if you want to use main in your form, you might have to enable force push for your fork):

git remote add upstream [email protected]:eclipse-tractusx/managed-identity-wallet.git
git remote fetch upstream
git checkout <your-branch>
git rebase upstream/develop
git push -f

@m-gora
Copy link
Contributor Author

m-gora commented Nov 22, 2023

@borisrizov-zf

@m-gora can you please rebase and change this to merge into develop.

Basic rebase workflow (if you want to use main in your form, you might have to enable force push for your fork):

git remote add upstream [email protected]:eclipse-tractusx/managed-identity-wallet.git
git remote fetch upstream
git checkout <your-branch>
git rebase upstream/develop
git push -f

hmm, i did not see that there was a develop branch actually. it's probably easier to just close this PR and reopen rather than going through god-knows-how-many-commits and rebase.

@m-gora m-gora closed this by deleting the head repository Nov 22, 2023
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.

3 participants