Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception when attempting to add myself to list after Shibboleth authentication #641

Closed
dpc22 opened this issue Jun 6, 2019 · 6 comments · Fixed by #642
Closed

Exception when attempting to add myself to list after Shibboleth authentication #641

dpc22 opened this issue Jun 6, 2019 · 6 comments · Fixed by #642
Assignees
Labels
Milestone

Comments

@dpc22
Copy link
Contributor

dpc22 commented Jun 6, 2019

Version

6.2.42

Installation method

Source package

Expected behavior

I should be able to add myself to a list as a list manager or global listmaster.

Actual behavior

wwsympa throws an exception (although I am still added to the list).

Additional information

I am attempting to configure Shibboleth logins on a test system. The
following page:

https://sympa-community.github.io/manual/customize/authentication-web.html

has a recipe if you need to generate an email challenge to verify the email address:

generic_sso
    service_name                Ucam Shibboth federation
    service_id                  ucam_federation
    http_header_list            mail,displayName,eduPersonAffiliation
    email_http_header           mail
    netid_http_header           REMOTE_USER
    internal_email_by_netid     1
    force_email_verify          1

This works nicely to create a new account. However if I attempt to subscribe to a list after logging in using Shibboleth, I see an exception:

DIED: Cannot quote a reference at /opt/sympa-6.2.42/share/sympa/lib/Sympa/Database.pm line 465.
 at /opt/sympa-6.2.42/share/sympa/lib/Sympa/Database.pm line 465.
	Sympa::Database::quote(Sympa::DatabaseDriver::PostgreSQL <db_host=localhost;db_name=sympa;db_user=sympa>, HASH(0x5652d88502a0), undef) called at /opt/sympa-6.2.42/share/sympa/lib/Sympa/DatabaseDriver/PostgreSQL.pm line 84
	Sympa::DatabaseDriver::PostgreSQL::quote(Sympa::DatabaseDriver::PostgreSQL <db_host=localhost;db_name=sympa;db_user=sympa>, HASH(0x5652d88502a0)) called at /opt/sympa-6.2.42/share/sympa/lib/Sympa/User.pm line 683
	Sympa::User::update_global_user('dpc22@cam.ac.uk', 'gecos', undef, 'email', 'dpc22@cam.ac.uk', 'cookie_delay', undef, 'data', ...) called at /opt/sympa-6.2.42/share/sympa/lib/Sympa/User.pm line 202
	Sympa::User::save(Sympa::User <dpc22@cam.ac.uk>) called at /opt/sympa-6.2.42/share/sympa/lib/Sympa/Request/Handler/add.pm line 146
	Sympa::Request::Handler::add::_twist(Sympa::Spindle::ProcessRequest=HASH(0x5652d886fa28), Sympa::Request <action=add;context=test-dpc99@magenta.csi.cam.ac.uk;email=dpc22@cam.ac.uk>) called at /opt/sympa-6.2.42/share/sympa/lib/Sympa/Spindle.pm line 95
	Sympa::Spindle::spin(Sympa::Spindle::ProcessRequest=HASH(0x5652d886fa28)) called at /opt/sympa/lib/sympa/cgi/wwsympa.fcgi line 7142
	main::do_add() called at /opt/sympa/lib/sympa/cgi/wwsympa.fcgi line 1558

"Cannot quote a reference" appears to be a consequence of Sympa attempting to convert the following into a Perl hash:

sympa=> SELECT attributes_user FROM user_table WHERE email_user='dpc22@cam.ac.uk';
                                      attributes_user                                       
--------------------------------------------------------------------------------------------
 mail__PAIRS_SEP__david.carter@uis.cam.ac.uk__ATT_SEP__displayName__PAIRS_SEP__David Carter
(1 row)

This column appears to be populated each time that I log in using Shibboleth.

I don't see the exception if I clear that column immediately before I click the "Add" button on the Manage list members screen using a bit of SQL:

sympa=> UPDATE user_table SET attributes_user='';
UPDATE 3

It also works if I just comment the http_header_list line from the generic_sso clause.

generic_sso
    service_name                Ucam Shibboth federation
    service_id                  ucam_federation
    #http_header_list            mail,displayName,eduPersonAffiliation
    email_http_header           mail
    netid_http_header           REMOTE_USER
    internal_email_by_netid     1

Sympa still uses the email address provided by the Shibboleth Identity Provider. However it doesn't record the displayName attribute.

@ikedas ikedas added the bug label Jun 6, 2019
@ikedas ikedas self-assigned this Jun 6, 2019
@ikedas
Copy link
Member

ikedas commented Jun 7, 2019

Hi @dpc22,

Could you please apply this patch and check if the problem will be solved? Thank you.

@dpc22
Copy link
Contributor Author

dpc22 commented Jun 7, 2019

No improvement I am afraid:

DIED: Cannot quote a reference at /opt/sympa-6.2.42/share/sympa/lib/Sympa/Database.pm line 465.
 at /opt/sympa-6.2.42/share/sympa/lib/Sympa/Database.pm line 465.
	Sympa::Database::quote(Sympa::DatabaseDriver::PostgreSQL <db_host=localhost;db_name=sympa;db_user=sympa>, HASH(0x56166c6b7008), undef) called at /opt/sympa-6.2.42/share/sympa/lib/Sympa/DatabaseDriver/PostgreSQL.pm line 84
	Sympa::DatabaseDriver::PostgreSQL::quote(Sympa::DatabaseDriver::PostgreSQL <db_host=localhost;db_name=sympa;db_user=sympa>, HASH(0x56166c6b7008)) called at /opt/sympa-6.2.42/share/sympa/lib/Sympa/User.pm line 686
	Sympa::User::update_global_user('dpc22@cam.ac.uk', 'attributes', HASH(0x56166c6b7008), 'gecos', undef, 'lang', undef, 'last_login_host', ...) called at /opt/sympa-6.2.42/share/sympa/lib/Sympa/User.pm line 202
	Sympa::User::save(Sympa::User <dpc22@cam.ac.uk>) called at /opt/sympa-6.2.42/share/sympa/lib/Sympa/Request/Handler/add.pm line 146
	Sympa::Request::Handler::add::_twist(Sympa::Spindle::ProcessRequest=HASH(0x56166c6b77d0), Sympa::Request <action=add;context=test-dpc99@magenta.csi.cam.ac.uk;email=dpc22@cam.ac.uk>) called at /opt/sympa-6.2.42/share/sympa/lib/Sympa/Spindle.pm line 95
	Sympa::Spindle::spin(Sympa::Spindle::ProcessRequest=HASH(0x56166c6b77d0)) called at /opt/sympa/lib/sympa/cgi/wwsympa.fcgi line 7142
	main::do_add() called at /opt/sympa/lib/sympa/cgi/wwsympa.fcgi line 1558

actually that error is subtly different. The Sympa::User::update_global_user line includes:

'attributes', HASH(0x56166c6b7008), 

Which is a fairly obvious smoking gun which wasn't there previously.

If I change the patch to test for "attributes" rather than "data":

$ diff -ud lib/Sympa/User.pm- lib/Sympa/User.pm
--- lib/Sympa/User.pm-  2019-06-07 07:09:30.837471684 +0100
+++ lib/Sympa/User.pm   2019-06-07 07:20:43.659023456 +0100
@@ -679,6 +679,9 @@
         if ($numeric_field{$map_field{$field}}) {
             $value ||= 0;    ## Can't have a null value
             $set = sprintf '%s=%s', $map_field{$field}, $value;
+        } elsif ($field eq 'attributes' and ref $value eq 'HASH') {
+            $set = sprintf '%s=%s', $map_field{$field},
+                $sdm->quote(Sympa::Tools::Data::hash_2_string($value));
         } else {
             $set = sprintf '%s=%s', $map_field{$field}, $sdm->quote($value);
         }

then the exception disappears. Of course I don't know if that is an appropriate thing to do.

The attributes_user column in user_table now has slightly odd behaviour.

After the first login which creates a new row in the table:

sympa=> SELECT attributes_user FROM user_table WHERE email_user='dpc22@cam.ac.uk';
                                      attributes_user                                       
--------------------------------------------------------------------------------------------
 mail__PAIRS_SEP__david.carter@uis.cam.ac.uk__ATT_SEP__displayName__PAIRS_SEP__David Carter
(1 row)

After adding myself to a list

sympa=> SELECT attributes_user FROM user_table WHERE email_user='dpc22@cam.ac.uk';
                        attributes_user                        
---------------------------------------------------------------
 ;mail="david.carter@uis.cam.ac.uk";displayName="David Carter"
(1 row)

It doesn't look like Sympa is using this information to populate the gecos field when I add myself to a list. However I don't know if that is expected behaviour.

@ikedas
Copy link
Member

ikedas commented Jun 7, 2019

Hi @dpc22,

I think your fix is close to correct solution. Could you please check this patch?

@dpc22
Copy link
Contributor Author

dpc22 commented Jun 7, 2019

I needed to apply the first patch before that applied, but otherwise it seemed to work:

After initial user creation:

sympa=> SELECT attributes_user FROM user_table WHERE email_user='dpc22@cam.ac.uk';
                                 attributes_user                                 
---------------------------------------------------------------------------------
 mail__PAIRS_SEP__dpc22@cam.ac.uk__ATT_SEP__displayName__PAIRS_SEP__David Carter
(1 row)

after adding myself to list:

sympa=> SELECT attributes_user FROM user_table WHERE email_user='dpc22@cam.ac.uk';
                                 attributes_user                                 
---------------------------------------------------------------------------------
 displayName__PAIRS_SEP__David Carter__ATT_SEP__mail__PAIRS_SEP__dpc22@cam.ac.uk
(1 row)

which appears to be the same thing with the two attributes reversed.

@ikedas
Copy link
Member

ikedas commented Jun 7, 2019

@dpc22 thanks, that's what I expected. If you have no more problem with my two patches, I'll commit them.

@ikedas
Copy link
Member

ikedas commented Jun 10, 2019

I'll commit the PR above which will be included in the next beta. If you have inconvenience by my fix, please reopen this issue to report it.

Thanks for reporting bug and confirming fixes!

ikedas added a commit that referenced this issue Jun 10, 2019
Updating user crashes due to missing serialization of structured data. (#641)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants