incodehq / incode-platform

Combines incode.org modules and isisaddons.org into a single set of modules.
http://platform.incode.org
Apache License 2.0
8 stars 9 forks source link

Security Module Vulnerability: Non-existing User gets created in DB even though not authenticated via LDAP #113

Open andi-huber opened 5 years ago

andi-huber commented 5 years ago

When using the Security Module with delegated authentication to LDAP, any login attempt of an user yet not existent within the DB, authenticated or not will create an (disabled) user-account in the DB.

While not a security risk, this allows attackers to 'fill' the database with arbitrary garbage.

Desired behavior for this scenario is to auto-create user accounts in the DB only if these do successfully authenticate with the delegated authentication mechanism.

I've fixed this in the Apache Isis 'v2' branch. See [1]

[1] https://issues.apache.org/jira/browse/ISIS-2157

andi-huber commented 5 years ago

Fix applies only to a single class: IsisModuleSecurityRealm

https://github.com/apache/isis/blob/v2/extensions/secman/realm-shiro/src/main/java/org/apache/isis/extensions/secman/shiro/IsisModuleSecurityRealm.java

This version in the 'v2' has slightly progressed and deviated from the Incode version.

danhaywood commented 5 years ago

This was kind-of by design, but in Estatio we ended up disabling it too, I think.

Happy with the fix, thanks.

On Fri, 2 Aug 2019 at 10:08, Andi Huber notifications@github.com wrote:

Fix only applies to a single class: IsisModuleSecurityRealm

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/incodehq/incode-platform/issues/113?email_source=notifications&email_token=AAH33SLGUP62E6HIPSC6HX3QCP2R7A5CNFSM4II32QU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3NFJFQ#issuecomment-517624982, or mute the thread https://github.com/notifications/unsubscribe-auth/AAH33SPNVV6FSHIK3XC3J4TQCP2R7ANCNFSM4II32QUQ .

johandoornenbal commented 5 years ago

We did indeed.

Grtz Johan

Op vr 2 aug. 2019 om 11:30 schreef Dan Haywood notifications@github.com:

This was kind-of by design, but in Estatio we ended up disabling it too, I think.

Happy with the fix, thanks.

On Fri, 2 Aug 2019 at 10:08, Andi Huber notifications@github.com wrote:

Fix only applies to a single class: IsisModuleSecurityRealm

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub < https://github.com/incodehq/incode-platform/issues/113?email_source=notifications&email_token=AAH33SLGUP62E6HIPSC6HX3QCP2R7A5CNFSM4II32QU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3NFJFQ#issuecomment-517624982 , or mute the thread < https://github.com/notifications/unsubscribe-auth/AAH33SPNVV6FSHIK3XC3J4TQCP2R7ANCNFSM4II32QUQ

.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/incodehq/incode-platform/issues/113?email_source=notifications&email_token=AB6NCFL4QWCCHS2MJCJZZT3QCP5DHA5CNFSM4II32QU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3NHEDY#issuecomment-517632527, or mute the thread https://github.com/notifications/unsubscribe-auth/AB6NCFMGVCE43MIV5QZFST3QCP5DHANCNFSM4II32QUQ .

andi-huber commented 5 years ago

(basically, the fix is to do a delegated authentication attempt before auto-creating any user accounts)