search

wso2-extensions / identity-outbound-auth-totp

Apache License 2.0
4 stars 95 forks source link

Refactoring TOTP service #159

Closed lashinijay closed 1 year ago

lashinijay commented 1 year ago

Purpose

With this TOTP service will check the availability of the EnableEncryption property of secretkey and verifySecretKey claims, and if the property is available it will pass the plain secret key and encryption will be taken place at the user-core level, otherwise encryption will happen at the service level.

jenkins-is-staging commented 1 year ago

PR builder started Link: https://github.com/wso2/product-is/actions/runs/5155042776

jenkins-is-staging commented 1 year ago

PR builder completed Link: https://github.com/wso2/product-is/actions/runs/5155042776 Status: failure

jenkins-is-staging commented 1 year ago

PR builder started Link: https://github.com/wso2/product-is/actions/runs/5155042776

jenkins-is-staging commented 1 year ago

PR builder completed Link: https://github.com/wso2/product-is/actions/runs/5155042776 Status: failure

lashinijay commented 1 year ago

Please include unit tests

There are no functional changes introduced with the refactoring and there are existing unit tests covering this functionality.

https://github.com/wso2-extensions/identity-outbound-auth-totp/blob/master/component/authenticator/src/test/java/org/wso2/carbon/identity/application/authenticator/totp/TOTPKeyGeneratorTest.java#L66

@janakamarasena could you please approve the PR?

janakamarasena commented 1 year ago

There are no functional changes introduced with the refactoring and there are existing unit tests covering this functionality.

https://github.com/wso2-extensions/identity-outbound-auth-totp/blob/master/component/authenticator/src/test/java/org/wso2/carbon/identity/application/authenticator/totp/TOTPKeyGeneratorTest.java#L66

Is this test covering the possible cases for the newly introduced method getProcessedClaimValue? Went through the code a bit and it doesn't seem to cover it.

lashinijay commented 1 year ago

There are no functional changes introduced with the refactoring and there are existing unit tests covering this functionality. https://github.com/wso2-extensions/identity-outbound-auth-totp/blob/master/component/authenticator/src/test/java/org/wso2/carbon/identity/application/authenticator/totp/TOTPKeyGeneratorTest.java#L66

Is this test covering the possible cases for the newly introduced method getProcessedClaimValue? Went through the code a bit and it doesn't seem to cover it.

The getProcessedClaimValue() this method will return a string (encrypted or plain text) and it is covered by testGenerateClaims() method by checking whether it is null or not.

lashinijay commented 1 year ago

Closing this PR as this will be covered with https://github.com/wso2-extensions/identity-outbound-auth-totp/pull/162