Merge pull request #1255 from ikedas/refactor_auth.conf by ikedas

Refactoring auth.conf (1).
  - Fix: Invalid POST data breaks sso_login form for all users (#1245)
  - Fix: CAS negotiation can be messed via query parameter.

default/web_tt2/header.tt2

@@ -47,7 +47,7 @@
                             [% PROCESS 'user_menu.tt2' %]
                         </ul>
                     </li>
-                [%~ ELSIF use_passwd || use_sso || authentication_info_url ~%]
+                [%~ ELSIF use_passwd || sso.size || authentication_info_url ~%]
                     [% PROCESS 'login_menu.tt2' %]
                 [%~ END %]
             </ul>

default/web_tt2/login.tt2

@@ -6,7 +6,7 @@
 [% ELSE ~%]
     <h2>
         <i class="fa fa-user"></i>
-        [% IF use_sso && use_passwd && only_passwd ~%]
+        [% IF sso.size && use_passwd && only_passwd ~%]
             [%|loc%]Login locally[%END%]
         [%~ ELSE ~%]
             [%|loc%]Login[%END%]
@@ -17,7 +17,7 @@
         [%|loc%]In order to perform a privileged operation (one that requires your email address), you need to login.[%END%]
     </p>
 
-    [% IF use_sso && !only_passwd ~%]
+    [% IF sso.size && !only_passwd ~%]
         <div class="row">
             <div class="columns">
                 <form id="use-sso" action="[% path_cgi %]" method="post">
@@ -27,7 +27,7 @@
                         <input type="hidden" name="list"            value="[% list %]" />
                         <input type="hidden" name="action"          value="sso_login" />
                         <input type="hidden" name="nomenu"          value="[% nomenu %]" />
-                        [% IF sso_number == 1 ~%]
+                        [% IF sso.size == 1 ~%]
                             [% FOREACH server = sso ~%]
                                 <input type="hidden" name="auth_service_name" value="[% server.key %]" />
                                 <button type="submit" name="action_sso_login" value="[% server.value %]">
@@ -53,9 +53,9 @@
                 </form>
             </div>
         </div>
-    [% END # IF use_sso && !only_passwd ~%]
+    [% END # IF sso.size && !only_passwd ~%]
 
-    [% IF use_passwd && use_sso && !only_passwd ~%]
+    [% IF use_passwd && sso.size && !only_passwd ~%]
         <hr />
         <h3>
             <i class="fa fa-user"></i> [%|loc%]Login locally[%END%]

default/web_tt2/login_menu.tt2

 <!-- login_menu.tt2 -->
-[% IF use_sso ~%]
+[% IF sso.size ~%]
     <li>
         <form id="use-sso" action="[% path_cgi %]" method="post">
             <fieldset>
                 <input type="hidden" name="list" value="[% list %]" />
                 <input type="hidden" name="action" value="sso_login" />
-                [% IF sso_number == 1 ~%]
+                [% IF sso.size == 1 ~%]
                     [% FOREACH server = sso ~%]
                         <input type="hidden" name="auth_service_name" value="[% server.key %]" />
                         <span>
@@ -36,22 +36,22 @@
     </li>
 [%~ END %]
 
-[% IF use_passwd == '1' AND use_sso ~%]
+[% IF use_passwd AND sso.size ~%]
     <li class="divider show-for-small" role="separator"></li>
 [%~ END %]
 
-[% IF use_passwd == '1' ~%]
+[% IF use_passwd ~%]
     [% IF action == 'login'; SET class = 'active'; END ~%]
     <li class="[% class %]">
         <form action="[% path_cgi %]" method="POST">
             <fieldset>
-                [% IF use_sso ~%]
+                [% IF sso.size ~%]
                     <input type="hidden" name="only_passwd" value="1" />
                 [%~ END %]
                 <span>
                     <button type="submit" name="action_login" value="[%|loc%]Login[%END%]">
                         <i class="fa fa-user"></i>
-                        [% IF use_sso ~%]
+                        [% IF sso.size ~%]
                             [%|loc%]Login locally[%END%]
                         [%~ ELSE ~%]
                             [%|loc%]Login[%END%]
@@ -61,5 +61,5 @@
             </fieldset>
         </form>
     </li>
-[%~ END # IF use_passwd == '1' %]
+[%~ END # IF use_passwd %]
 <!-- end login_menu.tt2 -->

default/web_tt2/pref.tt2

@@ -58,7 +58,7 @@
     </form>
     <br />
 
-    [% UNLESS use_sso %]
+    [% UNLESS sso.size %]
         <h4>[%|loc%]Changing your password[%END%]</h4>
 
         <form action="[% path_cgi %]" method="post">

src/lib/Conf.pm

@@ -803,20 +803,30 @@ sub valid_robot {
     return 1;
 }
 
-## Returns the SSO record correponding to the provided sso_id
-## return undef if none was found
-sub get_sso_by_id {
-    my %param = @_;
-
-    unless (defined $param{'service_id'} && defined $param{'robot'}) {
-        return undef;
-    }
-
-    foreach my $sso (@{$Conf{'auth_services'}{$param{'robot'}}}) {
-        $log->syslog('notice', 'SSO: %s', $sso->{'service_id'});
-        next unless ($sso->{'service_id'} eq $param{'service_id'});
-
-        return $sso;
+# Returns the SSO record correponding to the provided service_id
+# return undef if none was found
+# Old name: get_sso_by_id().
+sub get_auth_service {
+    my $robot        = shift;
+    my $service_type = shift;
+    my $service_id   = shift;
+
+    if ($service_type eq 'cas') {
+        return undef unless $service_id;
+        return [
+            grep {
+                        $_->{service_type} eq $service_type
+                    and $_->{auth_service_name} eq $service_id
+            } @{$Conf{'auth_services'}{$robot}}
+        ]->[-1];
+    } elsif ($service_type eq 'generic_sso') {
+        return undef unless $service_id;
+        return [
+            grep {
+                        $_->{service_type} eq $service_type
+                    and $_->{service_id} eq $service_id
+            } @{$Conf{'auth_services'}{$robot}}
+        ]->[-1];
     }
 
     return undef;
@@ -942,11 +952,6 @@ sub _load_auth {
         return undef;
     }
 
-    $Conf{'cas_number'}{$robot}         = 0;
-    $Conf{'generic_sso_number'}{$robot} = 0;
-    $Conf{'ldap_number'}{$robot}        = 0;
-    $Conf{'use_passwd'}{$robot}         = 0;
-
     ## Parsing  auth.conf
     while (<IN>) {
 
@@ -1008,7 +1013,6 @@ sub _load_auth {
                             'Incorrect CAS paragraph in auth.conf');
                         next;
                     }
-                    $Conf{'cas_number'}{$robot}++;
 
                     eval "require AuthCAS";
                     if ($EVAL_ERROR) {
@@ -1053,26 +1057,10 @@ sub _load_auth {
                         next;
                     }
 
-                    $Conf{'cas_id'}{$robot}
-                        {$current_paragraph->{'auth_service_name'}}{'casnum'}
-                        = scalar @paragraphs;
-
-                    ## Default value for auth_service_friendly_name IS
-                    ## auth_service_name
-                    $Conf{'cas_id'}{$robot}
-                        {$current_paragraph->{'auth_service_name'}}
-                        {'auth_service_friendly_name'} =
-                           $current_paragraph->{'auth_service_friendly_name'}
-                        || $current_paragraph->{'auth_service_name'};
-
                     ## Force the default scope because '' is interpreted as
                     ## 'base'
                     $current_paragraph->{'scope'} ||= 'sub';
                 } elsif ($current_paragraph->{'auth_type'} eq 'generic_sso') {
-                    $Conf{'generic_sso_number'}{$robot}++;
-                    $Conf{'generic_sso_id'}{$robot}
-                        {$current_paragraph->{'service_id'}} =
-                        $#paragraphs + 1;
                     ## Force the default scope because '' is interpreted as
                     ## 'base'
                     $current_paragraph->{'scope'} ||= 'sub';
@@ -1089,13 +1077,11 @@ sub _load_auth {
                             if (defined $current_paragraph->{$parameter});
                     }
                 } elsif ($current_paragraph->{'auth_type'} eq 'ldap') {
-                    $Conf{'ldap'}{$robot}++;
-                    $Conf{'use_passwd'}{$robot} = 1;
                     ## Force the default scope because '' is interpreted as
                     ## 'base'
                     $current_paragraph->{'scope'} ||= 'sub';
                 } elsif ($current_paragraph->{'auth_type'} eq 'user_table') {
-                    $Conf{'use_passwd'}{$robot} = 1;
+                    ;
                 }
                 # setting default
                 $current_paragraph->{'regexp'} = '.*'

src/lib/Sympa/WWW/Auth.pm

@@ -307,61 +307,55 @@ sub ldap_authentication {
 # fetch user email using their cas net_id and the paragrapah number in auth.conf
 # NOTE: This might be moved to Robot package.
 sub get_email_by_net_id {
-
+    $log->syslog('debug', '(%s, %s, %s)', @_);
     my $robot      = shift;
-    my $auth_id    = shift;
+    my $auth       = shift;
     my $attributes = shift;
 
-    $log->syslog('debug', '(%s, %s)', $auth_id, $attributes->{'uid'});
-
-    if (defined $Conf::Conf{'auth_services'}{$robot}[$auth_id]
-        {'internal_email_by_netid'}) {
-        my $sso_config   = @{$Conf::Conf{'auth_services'}{$robot}}[$auth_id];
-        my $netid_cookie = $sso_config->{'netid_http_header'};
+    if (defined $auth->{internal_email_by_netid}) {
+        my $netid_cookie = $auth->{netid_http_header};
 
         $netid_cookie =~ s/(\w+)/$attributes->{$1}/ig;
 
         my $email =
             Sympa::Robot::get_netidtoemail_db($robot, $netid_cookie,
-            $Conf::Conf{'auth_services'}{$robot}[$auth_id]{'service_id'});
+            $auth->{service_id});
 
         return $email;
     }
 
-    my $ldap = $Conf::Conf{'auth_services'}{$robot}->[$auth_id];
-
-    my $db = Sympa::Database->new('LDAP', %$ldap);
-
-    unless ($db and $db->connect()) {
+    my %ldap = %$auth;
+    my $db = Sympa::Database->new('LDAP', %ldap);
+    unless ($db and $db->connect) {
         $log->syslog('err', 'Unable to connect to the LDAP server "%s"',
-            $ldap->{'host'});
+            $ldap{host});
         return undef;
     }
 
-    my $filter = $ldap->{'get_email_by_uid_filter'};
+    my $filter = $auth->{get_email_by_uid_filter};
     $filter =~ s/\[([\w-]+)\]/$attributes->{$1}/ig;
 
     my $mesg = $db->do_operation(
         'search',
-        base    => $ldap->{'suffix'},
+        base    => $auth->{suffix},
         filter  => $filter,
-        scope   => $ldap->{'scope'},
-        timeout => $ldap->{'timeout'},
-        attrs   => [$ldap->{'email_attribute'}],
+        scope   => $auth->{scope},
+        timeout => $auth->{timeout},
+        attrs   => [$auth->{email_attribute}],
     );
 
-    unless ($mesg and $mesg->count()) {
+    unless ($mesg and $mesg->count) {
         $log->syslog('notice', "No entry in the LDAP Directory Tree of %s",
-            $ldap->{'host'});
-        $db->disconnect();
+            $ldap{host});
+        $db->disconnect;
         return undef;
     }
 
-    $db->disconnect();
+    $db->disconnect;
 
     # Return only the first attribute.
     foreach my $result ($mesg->entries) {
-        my $email = $result->get_value($ldap->{'email_attribute'});
+        my $email = $result->get_value($auth->{email_attribute});
         return undef unless Sympa::Tools::Text::valid_email($email);
         return Sympa::Tools::Text::canonic_email($email);
     }

src/lib/Sympa/WWW/SOAP.pm

@@ -229,13 +229,9 @@ sub casLogin {
 
     ## Validate the CAS ST against all known CAS servers defined in auth.conf
     ## CAS server response will include the user's NetID
-    my ($user, @proxies, $email, $cas_id);
-    foreach my $service_id (0 .. $#{$Conf::Conf{'auth_services'}{$robot}}) {
-        my $auth_service = $Conf::Conf{'auth_services'}{$robot}[$service_id];
-        ## skip non CAS entries
-        next
-            unless ($auth_service->{'auth_type'} eq 'cas');
-
+    my ($user, @proxies, $email, $auth);
+    foreach my $auth_service (grep { $_->{auth_type} eq 'cas' }
+        @{$Conf::Conf{'auth_services'}{$robot}}) {
         my $cas = AuthCAS->new(
             casUrl => $auth_service->{'base_url'},
             #CAFile => '/usr/local/apache/conf/ssl.crt/ca-bundle.crt',
@@ -257,7 +253,7 @@ sub casLogin {
             $user, $auth_service->{'base_url'});
 
         ## User was authenticated
-        $cas_id = $service_id;
+        $auth = $auth_service;
         last;
     }
 
@@ -269,11 +265,9 @@ sub casLogin {
     }
 
     ## Now fetch email attribute from LDAP
-    unless (
-        $email = Sympa::WWW::Auth::get_email_by_net_id(
-            $robot, $cas_id, {'uid' => $user}
-        )
-    ) {
+    unless ($email =
+        Sympa::WWW::Auth::get_email_by_net_id($robot, $auth, {uid => $user}))
+    {
         $log->syslog('err',
             'Could not get email address from LDAP for user %s', $user);
         die SOAP::Fault->faultcode('Server')

src/sbin/sympa_wizard.pl.in

@@ -240,11 +240,9 @@ sub display_configuration {
     foreach my $key (sort keys %Conf::Conf) {
         next
             if grep { $key eq $_ }
-            qw(auth_services blocklist cas_number crawlers_detection
-            generic_sso_number ldap ldap_number listmasters
+            qw(auth_services blocklist crawlers_detection listmasters
             locale2charset nrcpt_by_domain robot_by_http_host request
-            robot_name robots source_file sympa trusted_applications
-            use_passwd);
+            robot_name robots source_file sympa trusted_applications);
 
         $var = $Conf::Conf{$key};