Two instances of missing parameters causing scenario failures

[mpkut]

Version

6.2.52

Installation method

Local RPM build using unmodified Fedora/EPEL SPEC file and unmodified 6.2.52 release tag

Expected behavior

• Setting a list's send scenario to "public_bcc" should not generate scenario errors while navigating the GUI
• Changing email address using the move_user function should work

Actual behavior

• Setting the send scenario to "public_bcc" results in errors of the following form on each GUI request thereafter.

Jan 14 19:29:35 HOSTNAME wwsympa[1075]: debug2 Sympa::Scenario::authz(Sympa::Scenario <send.public_nobcc;/usr/share/sympa/default/scenari/send.public_nobcc>, md5, HASH, ...)
Jan 14 19:29:35 HOSTNAME wwsympa[1075]: err main::#1576 > main::check_param_out#2841 > Sympa::Scenario::authz#433 Error in scenario Sympa::Scenario <send.public_nobcc;/usr/share/sympa/default/scenari/send.public_nobcc>, context Sympa::List <move-user-test@HOSTNAME>: (Missing parameter 'is_bcc' at (eval 328) line 6.#012)
Jan 14 19:29:35 HOSTNAME wwsympa[1075]: info Sympa::Scenario::authz() Error in scenario Sympa::Scenario <send.public_nobcc;/usr/share/sympa/default/scenari/send.public_nobcc>, context Sympa::List <move-user-test@HOSTNAME>: (error-performing-condition)

• Attempting to use the address change form works up to the point of the final confirmation dialog. Upon selecting the "Confirm" button, a red "you are not authorized" message appears and the log contains messages of the following form:

Jan 14 19:31:01 HOSTNAME wwsympa[1076]: debug2 Sympa::Scenario::authz(Sympa::Scenario <move_user.auth;/usr/share/sympa/default/scenari/move_user.auth>, md5, HASH, ...)
Jan 14 19:31:01 HOSTNAME wwsympa[1076]: err main::#1544 > main::do_auth#17096 > Sympa::Spindle::spin#95 > Sympa::Request::Handler::auth::_twist#65 > Sympa::Spindle::spin#95 > Sympa::Spindle::AuthorizeRequest::_twist#87 > Sympa::Scenario::authz#433 Error in scenario Sympa::Scenario <move_user.auth;/usr/share/sympa/default/scenari/move_user.auth>, context HOSTNAME: (Missing parameter 'current_email' at (eval 565) line 6, <GEN390> line 1.#012)
Jan 14 19:31:01 HOSTNAME wwsympa[1076]: info Sympa::Scenario::authz() Error in scenario Sympa::Scenario <move_user.auth;/usr/share/sympa/default/scenari/move_user.auth>, context HOSTNAME: (error-performing-condition)

The common element is the Missing parameter text, which tracks to line 590 of src/lib/Sympa/Scenario.pm:

            $req = sprintf 'die "Missing parameter \'%s\'" unless exists $context->{%s};', $_, $_;

This appears to be from commit 11a54e6. The prior code just returns undef:

           $req = sprintf 'return undef unless exists $context->{%s};', $_;

It seems that some scenarios have relied on the previous behavior where missing parameters are inferred to be empty. I am reluctant to suggest a specific change to the current check but perhaps there is a way to log and return a suitable empty value that doesn't break the intent of the patch?

Thanks as always for Sympa and the work you do!

[ikedas]

Hi @mpkut,

Could you please show us the steps to reproduce the second case? I.e.:

Attempting to use the address change form works up to the point of the final confirmation dialog.

Please describe such as: 'Go to the page "...", click "..." button, enter "..." into "..." textbox, ...'.

[mpkut]

Hi @ikedas,

This procedure requires two Sympa accounts, which I will call userA and userB.

• Log in as userA

• Use the top right pop-up menu to visit the "My Preferences" page

• Navigate to the "Changing your email address" section of the preferences page

• Enter address userB and select "Change Email"

• Select "Confirm" on the "Are you sure you wish to change your email" dialog

• Stay logged in to Sympa

• Sympa sends an authorization link to userB

• Visit the link. In my specific case, clicking on the link in my mail reader starts up a new window using the same Web browser as my active Sympa session

• Sympa displays a confirmation page with the text "Do you really want this action to be taken?"

• Select "Confirm"

• The red AUTHORIZATION REJECT panel appears in the GUI
  

• The wwsympa log contains the 'Missing parameter' error

Jan 15 09:32:40 HOSTNAME wwsympa[27052]: err main::#1544 > main::do_auth#17096 > Sympa::Spindle::spin#95 > Sympa::Request::Handler::auth::_twist#65 > Sympa::Spindle::spin#95 > Sympa::Spindle::AuthorizeRequest::_twist#87 > Sympa::Scenario::authz#434 Error in scenario Sympa::Scenario <move_user.auth;/usr/share/sympa/default/scenari/move_user.auth>, context HOSTNAME: (Missing parameter 'current_email' at (eval 4223) line 6.#012)

• List subscriptions for userA are not moved to userB
• Listmasters receive mail with subject Failed to perform scenario condition and contents as below

Sympa failed to perform scenario condition for the following reason:
Error: error-performing-condition
Check the Sympa log files for more information.

Please let me know if there is any other information I can provide.

WRT to impact on our site, we have put in a small one-line patch to set a default value of $context->{is_bcc} to 0 so the public_nobcc send scenario does not fail on every GUI click, pending a more permanent resolution. However we have no workaround for the move_user error, but to this point only two of our users have encountered the issue since we upgraded to 6.2.52 on Saturday 1/11.

[ikedas]

@mpkut,
Could you please apply this patch and check if reported problem will be solved?
(This may solve the first problem, but may not the second).

[mpkut]

Hello @ikedas,

I applied the patch to the stock 6.2.52 lib/sympa/Scenario.pm in a test environment and saw the following results:

• setting a list to public_nobcc no longer results in per-click scenario alerts to listmasters
• using the user name change procedure provided above results in a green "auth: action completed" pop-up and the user's identity is successfully changed from "userA" to "userB":
  

It seems that these changes have addresses both issues we have encountered. Thank you so much for such a quick turnaround!

BTW a side comment. I had to stare at the conditional that is now in message_is_bcc() to realize that it's just checking whether the list name is in the To: and Cc: fields. Would regexes work just as well here?

    # a message is a bcc if neither To: or Cc: headers contains the list name
    return ( ($message->get_header('To') !~ /$that->{'name'}/i) &&
                ($message->get_header('Cc') !~ /$that->{'name'}/i) );

[mpkut]

On second thought, index() avoids any issues of list names that may have regex characters. Side comment retracted. :-P

[ikedas]

@mpkut, thanks for confirming fixes quickly. I'll merge it soon,

Well, index() is used to avoid troubles with regexp metacharacters. :)