Fineract 853

[DBryzz]

Description

Added spotbugsplugin "com.h3xstream.findsecbugs:findsecbugs-plugin:1.8.0" to dependencies.gradle and dev-dependencies.gradle.
To fix issue ## FINERACT-853

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

• Commit message starts with the issue number from https://issues.apache.org/jira/projects/FINERACT/. Ex: FINERACT-646 Pockets API.

• Coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions have been followed.

• API documentation at https://github.com/apache/fineract/blob/develop/api-docs/apiLive.htm has been updated with details of any API changes.

• Integration tests have been created/updated for verifying the changes made.

• All Integrations tests are passing with the new commits.

• Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the list.)

[DBryzz]

Hello @vorburger

Can you review my PR and correct it

[thesmallstar]

@DBryzz hello, can you please change the commit messege? It is recommended that the commit messege starts with issue number 😃

Edit: The build is failing, you can run
./gradlew clean build
to ensure the build passes locally.

[xurror]

Also, 2 commit messages for 1 PR does not look good in the commit history. So i suggests you squash your commit messages into a single message and don't forget to apply @thesmallstar comment.

[DBryzz]

Thanks for your comments I will work on them

…

On Wed, Apr 1, 2020 at 12:08 PM Yemdjih Kaze Nasser < ***@***.***> wrote: Also, 2 commit messages for 1 PR does not look good in the commit history. So i suggests you squash your commit messages into a single message and don't forget to apply @thesmallstar <https://github.com/thesmallstar> comment. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#749 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMBFONBUYTV2UVTWR77QESLRKMOCFANCNFSM4LXWWRXA> .

[percyashu]

@DBryzz try adding

spotbugsMain.maxHeapSize = '2g'

To the build.gradle and fix the violation that come up.

[vorburger]

fix the violation that come up.

@DBryzz as @awasum points out, this PR can obviously only be merged if it doesn't fail the build (which it currently does, so NOK), hope this makes sense?

I just looked at it a bit more closely (the logs are currently not available on travis-ci.org due to "There was an error while trying to fetch the log.", so I pulled and ran it locally), and noticed FINERACT-879 ... I've also left a related comment in FINERACT-853.

[DBryzz]

Greetings

Thanks for your review and comments @vorburger. I will try to find out why CORS is the only violation findsecbug detects.

Regarding the Overly permissive CORS policy, I located the file and the violation is due to the line response.setHeader("Access-Control-Allow-Origin", "*"); in class org.apache.fineract.infrastructure.security.filter.TenantAwareTenantIdentifierFilter

"*" needs to be replaced by a particular domain. I'll be happy if I someone could help me with the domain.

Thanks

[vorburger]

@DBryzz Following #764 and #761 this will pass the build (or not at least not fail because of integration tests) if you rebase this now.

[vorburger]

@DBryzz see (new) https://github.com/apache/fineract#pull-requests and do not wait for an active maintainer to come and help you re. build failures - it's your responsibility to get a green build. Have you checked the log, and done what the README suggests to do in such cases? 😺

[thesmallstar]

Hello @DBryzz can you please solve the conflicts? if this still fails due to "flaky" test please follow the readme :)

[vorburger]

I'm not super familiar with CORS, but this looks curious, to me. What does this hard-coded URL mean and do exactly? We have a dedicated JIRA re. CORS (search). Perhaps it would be best to first and separately solve that, before adding secbugs?

[github-actions]

This pull request seems to be stale. Are you still planning to work on it? We will automatically close it in 30 days.