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

[BACKLOG-39521] Added if statement to remove username and password be… #5499

Merged
merged 1 commit into from
Jan 19, 2024

Conversation

MarcDiamantHitachi
Copy link
Contributor

…ing printed

@buildguy

This comment has been minimized.

@buildguy

This comment has been minimized.

@buildguy
Copy link
Collaborator

✅ Build finished in 4m 42s

Build command:

mvn clean verify -B -e -Daudit -amd -pl tomcat-logs

❗ No tests found!

ℹ️ This is an automatic message

Copy link

SonarQube Quality Gate

Quality Gate failed

0.0% 0.0% Security Hotspots Reviewed on New Code (is less than 100%)

See analysis details on SonarQube

@peterrinehart peterrinehart merged commit 4761e10 into pentaho:master Jan 19, 2024
3 of 5 checks passed
Comment on lines +39 to +44
if ( tempString.contains( "/pentaho/api/csrf" ) || tempString.contains( "/pentaho/api/repo/files/backup" ) ) {
tempString = tempString.replaceAll( "\\?userid[^&]+%26", "" );
tempString = tempString.replaceAll( "\\?userid[^&]+", "" );
tempString = tempString.replaceAll( "password[^&]+%26", "" );
tempString = tempString.replaceAll( "\\&password[^&]+", "" );
}
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, this implementation should not have been merged as is.

Why does the filter only apply to these two URLs?
The parameter names used by RequestParameterAuthenticationFilter are configurable via the properties of the requestParameterProcessingFilter bean. Some customers may have these configured differently.

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.

4 participants