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
730 stars 713 forks source link

TotpSecretKey migration does not work with UserStoreBasedIdentityDataStore #14891

Open rksk opened 1 year ago

rksk commented 1 year ago

Describe the issue: When the UserStoreBasedIdentityDataStore is used, the TotpSecretKey migration does not work does not work due to two reasons.

  1. The migrateAll property checked at [1] is not set by default and it is not documented
  2. The claim mappings fetched at [2] are incorrect. By that time, the InMemoryClaimManager is initialized and it contains the claim mappings in the claim-config.xml file. But the claim mappings in the DB could be different, but the mappings from the DB are not loaded until the DefaultClaimManager is initialized.

How to reproduce:

  1. Configure UserStoreBasedIdentityDataStore with IS 5.2.0
  2. Enable TOTP by referring to [3] and configure a user with TOTP
  3. Run the migration to IS 5.11/6.0
  4. Then try to login to the same user with TOTP

The TOTP secret key claim value will not be changed during the migration.

Expected behavior: It should not make any difference for UserStoreBasedIdentityDataStore.

[1] https://github.com/wso2-extensions/identity-migration-resources/blob/082c006d911742dae0c81f332c84e6dd87fe3392/components/org.wso2.is.migration/migration-service/src/main/java/org/wso2/carbon/is/migration/util/TotpSecretUtil.java#L115 [2] https://github.com/wso2-extensions/identity-migration-resources/blob/082c006d911742dae0c81f332c84e6dd87fe3392/components/org.wso2.is.migration/migration-service/src/main/java/org/wso2/carbon/is/migration/util/TotpSecretUtil.java#L172 [3] https://docs.wso2.com/display/ISCONNECTORS/Configuring+TOTP+Authenticator

RushanNanayakkara commented 9 months ago

Closing due to not being reproducible

rksk commented 9 months ago

This will be impacted for users who are using UserStoreBasedIdentityDataStore as below.

toml

[event.default_listener.governance_identity_store]
data_store = "org.wso2.carbon.identity.governance.store.UserStoreBasedIdentityDataStore"

OR identity.xml

        <EventListener id="governance_identity_store"
                       type="org.wso2.carbon.user.core.listener.UserOperationEventListener"
                       name="org.wso2.carbon.identity.governance.listener.IdentityStoreEventListener"
                       orderId="97"
                       enable="true">
            <Property name="Data.Store">org.wso2.carbon.identity.governance.store.UserStoreBasedIdentityDataStore</Property>

Search for EncryptionUserFlowMigrator in migration-config.yaml and add migrateAll: true after the line schema: "identity" to solve it,

Also, if the mapped attribute for http://wso2.org/claims/identity/secretkey claim is not totpSecretkey in your previous version, make sure claim-config.xml in new version is updated with the mapped attribute you used in the previous version.

RushanNanayakkara commented 9 months ago

The migrateAll configuration is now set by default, therefore fixes one of the mentioned issues. The changed mapped attribute should be added to the claim-config.xml file which should resolve the issue.

However the existing claim mapping configuration ideally should be migrated without further intervention being required. Therefore this should be improved. Will update the issue once the analysis on this is complete.