From fad774f41b9a0b2a3f8bea36571fd01ccd9aec04 Mon Sep 17 00:00:00 2001 From: Xavier Guimard Date: Mon, 8 Feb 2010 10:06:21 +0000 Subject: [PATCH] Fix some little bugs --- .../lib/Lemonldap/NG/Portal/AuthSAML.pm | 187 ++++++++---------- .../lib/Lemonldap/NG/Portal/_SAML.pm | 120 ++++++----- 2 files changed, 143 insertions(+), 164 deletions(-) diff --git a/modules/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/AuthSAML.pm b/modules/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/AuthSAML.pm index d78429b36..35b1e59a4 100644 --- a/modules/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/AuthSAML.pm +++ b/modules/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/AuthSAML.pm @@ -22,69 +22,70 @@ sub authInit { return PE_ERROR unless $self->loadLasso(); # Activate SOAP - $self->{Soap} = 1; + $self->abort( 'To use SAML, you must set "Soap => 1"', 'error' ) + unless ( $self->{Soap} ); # Check presence of private key in configuration unless ( $self->{samlServicePrivateKey} ) { $self->lmLog( "SAML private key not found in configuration", 'error' ); - return PE_ERROR; - } + return PE_ERROR; + } - # Get metadata from configuration - $self->lmLog( "Get Metadata for this service", 'debug' ); + # Get metadata from configuration + $self->lmLog( "Get Metadata for this service", 'debug' ); my $service_metadata = Lemonldap::NG::Common::Conf::SAML::Metadata->new(); - # Create Lasso server with service metadata - my $server = $self->createServer( + # Create Lasso server with service metadata + my $server = $self->createServer( $service_metadata->serviceToXML( $ENV{DOCUMENT_ROOT} . "/skins/common/saml2-metadata.tpl", $self ), - $self->{samlServicePrivateKey}, - ); + $self->{samlServicePrivateKey}, + ); - unless ($server) { - $self->lmLog( 'Unable to create Lasso server', 'error' ); - return PE_ERROR; - } + unless ($server) { + $self->lmLog( 'Unable to create Lasso server', 'error' ); + return PE_ERROR; + } - $self->lmLog( "Service created", 'debug' ); + $self->lmLog( "Service created", 'debug' ); - # Check presence of at least one identity provider in configuration - unless ( $self->{samlIDPMetaData} - and keys %{ $self->{samlIDPMetaData} } ) - { - $self->lmLog( "No IDP found in configuration", 'error' ); - return PE_ERROR; - } + # Check presence of at least one identity provider in configuration + unless ( $self->{samlIDPMetaData} + and keys %{ $self->{samlIDPMetaData} } ) + { + $self->lmLog( "No IDP found in configuration", 'error' ); + return PE_ERROR; + } - # Load identity provider metadata - # IDP are listed in $self->{samlIDPMetaData} - # Each key is the IDP name and value is the metadata - # Build IDP list for later use in extractFormInfo - foreach ( keys %{ $self->{samlIDPMetaData} } ) { + # Load identity provider metadata + # IDP are listed in $self->{samlIDPMetaData} + # Each key is the IDP name and value is the metadata + # Build IDP list for later use in extractFormInfo + foreach ( keys %{ $self->{samlIDPMetaData} } ) { - $self->lmLog( "Get Metadata for IDP $_", 'debug' ); + $self->lmLog( "Get Metadata for IDP $_", 'debug' ); - # Get metadata from configuration + # Get metadata from configuration my $idp_metadata = Lemonldap::NG::Common::Conf::SAML::Metadata->new(); - unless ( - $idp_metadata->initializeFromConfHash( - $self->{samlIDPMetaData}->{$_} - ) - ) - { - $self->lmLog( "Fail to read IDP $_ Metadata from configuration", - 'error' ); - return PE_ERROR; - } + unless ( + $idp_metadata->initializeFromConfHash( + $self->{samlIDPMetaData}->{$_} + ) + ) + { + $self->lmLog( "Fail to read IDP $_ Metadata from configuration", + 'error' ); + return PE_ERROR; + } - # Add this IDP to Lasso::Server - my $result = $self->addIDP( $server, $idp_metadata->toXML() ); + # Add this IDP to Lasso::Server + my $result = $self->addIDP( $server, $idp_metadata->toXML() ); - unless ($result) { - $self->lmLog( "Fail to use IDP $_ Metadata", 'error' ); - return PE_ERROR; - } + unless ($result) { + $self->lmLog( "Fail to use IDP $_ Metadata", 'error' ); + return PE_ERROR; + } # Store IDP entityID and Organization Name my $entityID = $idp_metadata->{entityID}; @@ -93,8 +94,8 @@ sub authInit { $self->{_idpList}->{$_}->{entityID} = $entityID; $self->{_idpList}->{$_}->{name} = $name; - $self->lmLog( "IDP $_ added", 'debug' ); - } + $self->lmLog( "IDP $_ added", 'debug' ); + } # Store Lasso::Server object $self->{_lassoServer} = $server; @@ -106,7 +107,7 @@ sub authInit { # Check authentication statement or create authentication request # @return Lemonldap::NG::Portal error code sub extractFormInfo { - my $self = shift; + my $self = shift; my $server = $self->{_lassoServer}; my $idp; @@ -114,16 +115,16 @@ sub extractFormInfo { # if ( authnStatement ) ... # TODO - check authstatement # else - + # 2. IDP resolution - + # Get IDP resolution cookie - my %cookies = fetch CGI::Cookie; + my %cookies = fetch CGI::Cookie; my $idp_cookie = $cookies{ $self->{samlIdPResolveCookie} }; if ($idp_cookie) { - $idp = $idp_cookie->value; + $idp = $idp_cookie->value; $self->lmLog( "IDP $idp found in IDP resolution cookie", 'debug' ); - } + } # If no IDP resolve cookie, find another way to get it # Case 1: IDP was choosen from portal IDP list @@ -139,37 +140,28 @@ sub extractFormInfo { $self->lmLog( "No IDP found, redirecting user to IDP list", 'debug' ); # IDP list - my $html = "

"; - $html .= - &Lemonldap::NG::Portal::_i18n::msg( PM_SAML_IDPSELECT, - $ENV{HTTP_ACCEPT_LANGUAGE} ); - $html .= "

\n"; + my $html = "

" + . &Lemonldap::NG::Portal::_i18n::msg( PM_SAML_IDPSELECT, + $ENV{HTTP_ACCEPT_LANGUAGE} ) + . "

\n\n"; - $html .= "
\n"; foreach ( keys %{ $self->{_idpList} } ) { - $html .= ""; + . '" />'; } $html .= -""; - $html .= "\n"; +'
"; $html .= - "\n"; - $html .= ""; - $html .= $self->{_idpList}->{$_}->{name}; - $html .= "
' + . $self->{_idpList}->{$_}->{name} + . '
"; - $html .= - &Lemonldap::NG::Portal::_i18n::msg( PM_REMEMBERCHOICE, - $ENV{HTTP_ACCEPT_LANGUAGE} ); - $html .= "
' + . &Lemonldap::NG::Portal::_i18n::msg( PM_REMEMBERCHOICE, + $ENV{HTTP_ACCEPT_LANGUAGE} ) + . "
\n" - $html .= "\n"; - - # Script to autoselect first choice - $html .= -"\n"; + # Script to autoselect first choice + . ''; $self->info($html); @@ -184,48 +176,43 @@ sub extractFormInfo { -expires => '-1d', ); - return PE_CONFIRM; + return PE_CONFIRM; } - + # If IDP is found but not confirmed, let the user confirm it if ( $confirm_flag != 1 ) { $self->lmLog( "IDP $idp selected, need user confirmation", 'debug' ); # Choosen IDP - my $html = "

"; - $html .= - &Lemonldap::NG::Portal::_i18n::msg( PM_SAML_IDPCHOOSEN, - $ENV{HTTP_ACCEPT_LANGUAGE} ); - $html .= "

\n"; - - $html .= "

$idp

\n"; - - $html .= "\n"; + my $html = '

' + . &Lemonldap::NG::Portal::_i18n::msg( PM_SAML_IDPCHOOSEN, + $ENV{HTTP_ACCEPT_LANGUAGE} ) + . "

\n

$idp

\n\n"; $self->info($html); - return PE_CONFIRM; + return PE_CONFIRM; } # Here confirmation is OK (confirm_flag == 1), store choosen IDP in cookie unless ( $idp_cookie and ( $idp eq $idp_cookie->value ) ) { $self->lmLog( "Build cookie to remember $idp as IDP choice", 'debug' ); - # User can choose temporary (0) or persistent cookie (1) - my $cookie_type = $self->param("cookie_type") || "0"; + # User can choose temporary (0) or persistent cookie (1) + my $cookie_type = $self->param("cookie_type") || "0"; - push @{ $self->{cookie} }, - $self->cookie( - -name => $self->{samlIdPResolveCookie}, - -value => $idp, - -domain => $self->{domain}, - -path => "/", - -secure => $self->{securedCookie}, - -httponly => $self->{httpOnly}, - -expires => $cookie_type ? "+365d" : "", - ); + push @{ $self->{cookie} }, + $self->cookie( + -name => $self->{samlIdPResolveCookie}, + -value => $idp, + -domain => $self->{domain}, + -path => "/", + -secure => $self->{securedCookie}, + -httponly => $self->{httpOnly}, + -expires => $cookie_type ? "+365d" : "", + ); } - + # 3. Build authentication request my $login = $self->createAuthnRequest( $server, $idp ); diff --git a/modules/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/_SAML.pm b/modules/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/_SAML.pm index 927897942..5a1e59d33 100644 --- a/modules/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/_SAML.pm +++ b/modules/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/_SAML.pm @@ -17,6 +17,42 @@ our @EXPORT = qw( our $VERSION = '0.01'; +BEGIN { + + # Load Glib if available + eval 'use Glib;'; + if ($@) { + print STDERR + "Glib Lasso messages will not be catched (require Glib module)\n"; + eval "use constant GLIB => 0"; + } + else { + eval "use constant GLIB => 0"; + } + + # Load Lasso.pm + eval 'use Lasso;'; + if ($@) { + print STDERR "Lasso.pm not loaded :$@"; + eval 'use constant LASSO => 0;use constant BADLASSO => 0;'; + } + else { + no strict 'subs'; + eval 'use constant LASSO => 1'; + my $lasso_check_version_mode = Lasso::Constants::CHECK_VERSION_NUMERIC; + my $check_version = + Lasso::check_version( 2, 2, 91, $lasso_check_version_mode ); + unless ($check_version) { + eval 'use constant BADLASSO => 1'; + } + else { + eval 'use constant BADLASSO => 0'; + } + } + + # Check Lasso version >= 2.2.91 +} + ## @method boolean loadLasso() # Load Lasso module # @return boolean result @@ -27,8 +63,7 @@ sub loadLasso { return 1 if $self->{_lasso}; # Catch GLib Lasso messages (require Glib) - eval { use Glib; }; - unless ($@) { + if (GLIB) { Glib::Log->set_handler( "Lasso", [qw/ error critical warning message info debug /], @@ -38,35 +73,15 @@ sub loadLasso { } ); } - else { - $self->lmLog( - "Glib Lasso messages will not be catched (require Glib module)", - 'info' ); - } - - # Load Lasso.pm - eval { use Lasso; }; - if ($@) { - $self->lmLog( "Module Lasso not found in @INC", 'error' ); - $self->lmLog( "$@", 'debug' ); + unless (LASSO) { + $self->lmLog( "Module Lasso not loaded (see bellow)", 'error' ); return 0; } - # Check Lasso version >= 2.2.91 - my $lasso_check_version_mode = Lasso::Constants::CHECK_VERSION_NUMERIC; - my $check_version = - Lasso::check_version( 2, 2, 91, $lasso_check_version_mode ); - - unless ($check_version) { + if (BADLASSO) { $self->lmLog( 'Lasso version >= 2.2.91 required', 'error' ); return 0; } - - $self->lmLog( "Module Lasso loaded", 'debug' ); - - # Remember we have loaded Lasso - $self->{_lasso} = 1; - return 1; } @@ -76,9 +91,8 @@ sub loadLasso { # @param string optional log level (debug by default) # @return 1 if no error sub checkLassoError { - my $self = shift; - my $error = shift; - my $level = shift || 'debug'; + my ( $self, $error, $level ) = splice @_; + my $level ||= 'debug'; # If $error is not a Lasso::Error object, display error string unless ( ref($error) and $error->isa("Lasso::Error") ) { @@ -106,16 +120,12 @@ sub checkLassoError { # @param string optional certificate # @return Lasso::Server object sub createServer { - my $self = shift; - my $metadata = shift; - my $private_key = shift; - my $private_key_password = shift; - my $certificate = shift; + my ( $self, $metadata, $private_key, $private_key_password, $certificate) = splice @_; my $server; eval { $server = Lasso::Server::new_from_buffers( $metadata, $private_key, - $private_key_password, $certificate ); + $private_key_password, $certificate ); }; return unless $self->checkLassoError($@); @@ -131,14 +141,11 @@ sub createServer { # @param string optional ca cert chain # @return boolean result sub addIDP { - my $self = shift; - my $server = shift; - my $metadata = shift; - my $public_key = shift; - my $ca_cert_chain = shift; + my ( $self, $server, $metadata, $public_key, $ca_cert_chain ) = splice @_; return 0 unless ( $server->isa("Lasso::Server") and defined $metadata ); + no strict 'subs'; return $self->addProvider( $server, Lasso::Constants::PROVIDER_ROLE_IDP, $metadata, $public_key, $ca_cert_chain ); } @@ -152,12 +159,8 @@ sub addIDP { # @param string optional ca cert chain # @return boolean result sub addProvider { - my $self = shift; - my $server = shift; - my $role = shift; - my $metadata = shift; - my $public_key = shift; - my $ca_cert_chain = shift; + my ( $self, $server, $role, $metadata, $public_key, $ca_cert_chain ) = + splice @_; return 0 unless ( $server->isa("Lasso::Server") @@ -179,11 +182,8 @@ sub addProvider { #@param string entityID #@return string organization name sub getOrganizationName { - my $self = shift; - my $server = shift; - my $idp = shift; - my $provider; - my $node; + my ( $self, $server, $idp ) = splice @_; + my ( $provider, $node ); # Get provider from server eval { $provider = Lasso::Server::get_provider( $server, $idp ); }; @@ -205,9 +205,7 @@ sub getOrganizationName { # @param string entityID # @return Lasso::Login object sub createAuthnRequest { - my $self = shift; - my $server = shift; - my $idp = shift; + my ( $self, $server, $idp ) = splice @_; # Create Lasso Login my $login = $self->createLogin($server); @@ -244,8 +242,7 @@ sub createAuthnRequest { # @param Lasso::Server server # @return Lasso::Login object sub createLogin { - my $self = shift; - my $server = shift; + my ( $self, $server ) = splice @_; my $login; eval { $login = Lasso::Login->new($server); }; @@ -261,13 +258,12 @@ sub createLogin { # @param string entityID # @return int HTTP method sub getHttpMethod { - my $self = shift; - my $server = shift; - my $idp = shift; + my ( $self, $server, $idp ) = splice @_; # TODO # By default, use HTTP REDIRECT + no strict; return Lasso::Constants::HTTP_METHOD_REDIRECT; } @@ -278,10 +274,7 @@ sub getHttpMethod { # @param int HTTP method # @return boolean result sub initAuthnRequest { - my $self = shift; - my $login = shift; - my $idp = shift; - my $method = shift; + my ( $self, $login, $idp, $method ) = splice @_; eval { Lasso::Login::init_authn_request( $login, $idp, $method ); }; @@ -293,8 +286,7 @@ sub initAuthnRequest { # @param Lasso::Login login # @return boolean result sub buildAuthnRequestMsg { - my $self = shift; - my $login = shift; + my ( $self, $login ) = splice @_; eval { Lasso::Login::build_authn_request_msg($login); };