On 29 June 2016 at 17:55, Thomas Darimont <thomas.darimont@googlemail.com> wrote:

Hello group,

I just ran findbugs [1] with the find-sec-bugs [0] and found quite a bunch of rather 

suspicious places in the Keycloak codebase.

Note that I don't wont to blame anyone but rather try to improve the codebase :)

For instance there are some quite prominent (and sensitive) non-final public static fields that could 

be easily changed to something else (in case they aren't inlined).

https://github.com/keycloak/keycloak/blob/3c0f7e2ee2140a9e69e4e95eb24d5a122e63e09a/server-spi/src/main/java/org/keycloak/models/AdminRoles.java#L25 

Further more there seem to be some dead code left-overs from merges spread over the codebase e.g:

https://github.com/keycloak/keycloak/blob/3a669ad7d5b4a72a8eb2bbb23e91083b63f59a2f/adapters/saml/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/saml/CatalinaSamlSessionStore.java#L144 

Question is how to deal with that?

I could send PRs for those issues - they would contain quite a bunch of files 

with minor changes. Would you be open to such contributions and if so, what JIRA issue 

should one reference here?

Ideally it would be broken into JIRAs and sent PRs for a few changes at a time. If you send to many changes in one PR/JIRA it would be much more effort to review the PR.

 

Cheers,

Thomas

[0] http://find-sec-bugs.github.io/

[1] https://github.com/find-sec-bugs/find-sec-bugs/wiki/Maven-configuration 

_______________________________________________
keycloak-dev mailing list
keycloak-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev