search

wso2 / product-is

Welcome to the WSO2 Identity Server source code! For info on working with the WSO2 Identity Server repository and contributing code, click the link below.
http://wso2.github.io/
Apache License 2.0
742 stars 722 forks source link

Old password is not set in IdentityEvent when changing it #9553

Open geve82 opened 4 years ago

geve82 commented 4 years ago

**Describe the issue: When a user or an admin change a password EventProperty.CREDENTIAL is set with the new password and EventProperty.OLD_CREDENTIAL is never set.

We can not write password policies to compare the current password and the new one.

How to reproduce: change a password by scim2 or soap and debug interactively/make a custom eventHandler to check the content of eventProperties.

Expected behavior: EventProperty.CREDENTIAL is defined in https://github.com/wso2/carbon-identity-framework/blob/master/components/identity-event/org.wso2.carbon.identity.event/src/main/java/org/wso2/carbon/identity/event/IdentityEventConstants.java#L185 so it need to be properly set during password change.

Environment information (Please complete the following information; remove any unnecessary fields) : N/A

Optional Fields

Related issues:

Suggested labels:

ruwanta commented 4 years ago

+1, for the general idea.

We may need to use new property "OLD_PASSWORD_INFO", containing the Hash, salt. algorithm etc. instead of OLD_CREDENTIAL. JDBC User store, we do not store the plain password, but store the hash. The policy needs to be enforced with the hash only.

However, there is no way we can do the same for LDAP type user stores. We may have another property on the event to pass the information about the "User Store Type", so that the event listeners can skip certain actions based on the type of the user store.

geve82 commented 4 years ago

Hi Ruwanta,

Currently we can query the last password in table IDN_HISTORY_DATA if password history is enabled to get a HASH and its SALT. With hash and salt we can only try a brute force attack based on the new password to test the similarity but is not great for performance and limit possibilities.

What I want is when IS receives a request to change a password, if it is done by the enduser itself, the current password should be added in existing event properties OLD_CREDENTIAL. If so we can make advanced password comparisons like leveinstein, check if it is a sequence and so on with new password set in property CREDENTIAL. There is no security issue as this information is received in the request and will be deleted at the end of it. It is just that currently it is not set in the OLD_CREDENTIAL property. (Other IAM can do this you can look at Ping or Forgerock for example)

ruwanta commented 4 years ago

Hi @geve82, Looks like a good proposal. +1.