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

Faulty logic when retrieving `federatedEmailAttributeKey` and `federatedMobileAttributeKey` parameters for the Email and SMS OTP Authenticators #19482

Open vfraga opened 4 months ago

vfraga commented 4 months ago

Describe the issue:

Having both EmailOTP and SMS in a single step makes the iterator logic fail at [1] as by using Iterator#next, only one of the authenticators is picked even though there are two of them in the list as per the step config. As a workaround, adding both federatedEmailAttributeKey and federatedMobileAttributeKey parameters to both the authenticators solves the issue:

[authentication.authenticator.email_otp.parameters]
useEventHandlerBasedEmailSender = true
sendOTPToFederatedEmailAttribute = true
federatedEmailAttributeKey = "mail"
# [TEMPORARY] [WORKAROUND] SMSOTP parameters: 
federatedMobileAttributeKey = "mobilePhone"

[authentication.authenticator.sms_otp.parameters]
SendOtpToFederatedMobile = true
SMSOTPMandatory = true
federatedMobileAttributeKey = "mobilePhone"
# [TEMPORARY] [WORKAROUND] EmailOTP parameters: 
federatedEmailAttributeKey = "mail"

EmailOTP is usually the first authenticator in this list (maybe due to String sorting), so it might not display an issue while the SMSOTP does, but its logic has the same issue [2].

How to reproduce:

[authentication.authenticator.sms_otp.parameters] SendOtpToFederatedMobile = true SMSOTPMandatory = true federatedMobileAttributeKey = "mobilePhone"

- Set up a Service Provider with a federated authenticator on the first step, and both e-mail and SMS OTP on the second step.
- Use the adaptive authentication script below for setting up the federated claims (pay attention to the authenticator name in `authenticationOptions`):
```js
var onLoginRequest = function(context) {
  executeStep(1, {
    onSuccess: function(context) {
      if (context.currentKnownSubject.userStoreDomain == null) {
          var federatedAuthUser = context.currentKnownSubject;

          var email = "testmail@mail.com";
          var mobile = "+12343334444";

          federatedAuthUser.remoteClaims["mail"] = email;
          federatedAuthUser.remoteClaims["mobilePhone"] = mobile;
      }

      executeStep(2, {
        authenticationOptions: [{ idp: "SMSOTP" }, { idp: "EmailOTP" }],
        authenticatorParams: { federated: { SMSOTP: { enableRetryFromAuthenticator: "true" }, EmailOTP: { enableRetryFromAuthenticator: "true" }} }
      }, {});
    }
  });
};

Expected behavior: The authenticators should pick up the properties from their own authenticator configuration.

Environment information:

tharakawijekoon commented 4 months ago

Inside authenticator why do we need to get the currentStepAuthenticator? we already know the authenticator if we are inside the authenticator😀