[Rt-commit] rt branch 5.0/login-logout-adjustments2 created. rt-5.0.3-279-gca5e923698

BPS Git Server git at git.bestpractical.com
Wed Mar 8 21:20:30 UTC 2023


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "rt".

The branch, 5.0/login-logout-adjustments2 has been created
        at  ca5e923698bdecfe31eabb168eebafe3858a7235 (commit)

- Log -----------------------------------------------------------------
commit ca5e923698bdecfe31eabb168eebafe3858a7235
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Wed Mar 8 16:05:25 2023 -0500

    Document callback change in upgrading

diff --git a/docs/UPGRADING-5.0 b/docs/UPGRADING-5.0
index a1c786cd7d..e21f58a5c2 100644
--- a/docs/UPGRADING-5.0
+++ b/docs/UPGRADING-5.0
@@ -501,6 +501,17 @@ we have replaced it with the GraphViz2 module.
 Systems using C<--enable-graphviz> will be prompted to install the Perl
 GraphViz2 module when upgrading.
 
+=item * ModifyLoginRedirect callback in Logout.html moved
+
+We try hard not to modify callbacks since they are made for external
+code to reference, but in this case the logic of the page changed
+and we had to move the callback location so it could correctly
+modify the URL value, if needed. If you were using this callback to
+modify the redirect URL on logout, your code will continue to work
+as intended. However, if you were using this callback for other
+reasons, you may need to update your code to use the C<BeforeSessionDelete>
+callback instead.
+
 =back
 
 =cut

commit cc698126f9876f9391900ea15dd0d410402beb07
Author: Ronaldo Richieri <ronaldo at bestpractical.com>
Date:   Thu Mar 2 10:12:34 2023 -0300

    Move ModifyLoginRedirect Callback to the end of Logout processing
    
    The URL generated by the ModifyLoginRedirect callback could be
    overwritten by the LogoutURL config if Web External Auth was enabled.
    This change moves the callback to the end of the Logout processing so
    that the callback can override the final URL to redirect the user.

diff --git a/share/html/NoAuth/Logout.html b/share/html/NoAuth/Logout.html
index d2136b34e1..f24b172129 100644
--- a/share/html/NoAuth/Logout.html
+++ b/share/html/NoAuth/Logout.html
@@ -72,10 +72,6 @@
 # Set default redirect location to RT homepage
 my $URL = RT->Config->Get('WebPath')."/";
 
-# Allow a callback to modify the URL we redirect to, which is useful for
-# external webauth systems
-$m->callback( %ARGS, CallbackName => 'ModifyLoginRedirect', URL => \$URL );
-
 $m->callback( %ARGS, CallbackName => 'BeforeSessionDelete' );
 
 my $username;
@@ -109,6 +105,10 @@ if (keys %session) {
 }
 
 $m->callback( %ARGS, CallbackName => 'AfterSessionDelete' );
+
+# Allow a callback to modify the URL we redirect to, which is useful for
+# external webauth systems
+$m->callback( %ARGS, CallbackName => 'ModifyLoginRedirect', URL => \$URL );
 $m->notes->{RefreshURL} = $URL;
 
 RT->Logger->debug("Redirecting to $URL");

commit 726286f05ed103eecf40cf86774765fcf537f72d
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Wed Mar 8 16:00:04 2023 -0500

    Clarify logout messages for local and SAML logouts

diff --git a/share/html/NoAuth/Logout.html b/share/html/NoAuth/Logout.html
index 085ab6fb90..d2136b34e1 100644
--- a/share/html/NoAuth/Logout.html
+++ b/share/html/NoAuth/Logout.html
@@ -79,6 +79,7 @@ $m->callback( %ARGS, CallbackName => 'ModifyLoginRedirect', URL => \$URL );
 $m->callback( %ARGS, CallbackName => 'BeforeSessionDelete' );
 
 my $username;
+my $remote_addr = RT::Interface::Web::RequestENV('REMOTE_ADDR');
 if (keys %session) {
     $username = $session{'CurrentUser'}->Name;
     my $externally_authed = $session{'WebExternallyAuthed'};
@@ -95,6 +96,16 @@ if (keys %session) {
     # Clear the session
     RT::Interface::Web::InstantiateNewSession();
     $session{'CurrentUser'} = RT::CurrentUser->new;
+
+    if ( $externally_authed ) {
+        # For SAML-type auth, there is another session which will need to be
+        # logged out using the configured URL. Report here only that we cleared
+        # the RT session.
+        RT->Logger->info("Successful cleared session for $username from $remote_addr");
+    }
+    else {
+        RT->Logger->info("Successful logout for $username from $remote_addr");
+    }
 }
 
 $m->callback( %ARGS, CallbackName => 'AfterSessionDelete' );

commit a2bda7bcaa40185515e0d3fd9cebcb485d438c99
Author: Ronaldo Richieri <ronaldo at bestpractical.com>
Date:   Thu Mar 2 10:05:02 2023 -0300

    Simplify setting the redirect URL on logout

diff --git a/share/html/NoAuth/Logout.html b/share/html/NoAuth/Logout.html
index f6bd25ea3b..085ab6fb90 100644
--- a/share/html/NoAuth/Logout.html
+++ b/share/html/NoAuth/Logout.html
@@ -69,6 +69,7 @@
 % $m->abort();
 
 <%INIT>
+# Set default redirect location to RT homepage
 my $URL = RT->Config->Get('WebPath')."/";
 
 # Allow a callback to modify the URL we redirect to, which is useful for
@@ -80,13 +81,15 @@ $m->callback( %ARGS, CallbackName => 'BeforeSessionDelete' );
 my $username;
 if (keys %session) {
     $username = $session{'CurrentUser'}->Name;
+    my $externally_authed = $session{'WebExternallyAuthed'};
 
-    # If WebRemoteUserAuth and LogoutURL are set, redirect to the LogoutURL set on Config
-    if (
-        $session{'WebExternallyAuthed'}
-        && RT->Config->Get('LogoutURL')
-    ) {
-        $URL = RT->Config->Get('LogoutURL');
+    my $LogoutURL = RT->Config->Get('LogoutURL');
+    if ( $externally_authed && $LogoutURL
+         && $LogoutURL ne '/NoAuth/Logout.html' ) {
+
+        # RT is configured with SAML or other external auth.
+        # Redirect to the configured Logout URL.
+        $URL = $LogoutURL;
     }
 
     # Clear the session
@@ -97,5 +100,5 @@ if (keys %session) {
 $m->callback( %ARGS, CallbackName => 'AfterSessionDelete' );
 $m->notes->{RefreshURL} = $URL;
 
-RT->Logger->info("User $username logged out. Redirecting to $URL") if $username;
+RT->Logger->debug("Redirecting to $URL");
 </%INIT>

commit 6e6d6f2869f18d911a25b2a2e7cfdb82ea5f211b
Author: Ronaldo Richieri <ronaldo at bestpractical.com>
Date:   Thu Mar 2 09:03:48 2023 -0300

    Always show a Logout link in the menu
    
    The updates in 4e46f947 are intended to send all logout requests
    through Logout.html first, even if another logout link is
    configured. This allows us to remove the user session and write
    a log message, even if we then redirect to an external logout
    endpoint, as with SAML auth. Since we will always do this, we should
    always show the Logout link.
    
    Remove the code to hide the link in some cases.

diff --git a/lib/RT/Interface/Web/MenuBuilder.pm b/lib/RT/Interface/Web/MenuBuilder.pm
index b6260023a8..52adfa844f 100644
--- a/lib/RT/Interface/Web/MenuBuilder.pm
+++ b/lib/RT/Interface/Web/MenuBuilder.pm
@@ -339,7 +339,7 @@ sub BuildMainNav {
         }
     }
     if ( $current_user->Name ) {
-        _BuildLogoutMenu( $about_me );
+        $about_me->child( logout => title => loc('Logout'), path => '/NoAuth/Logout.html' );
     }
     if ( $request_path =~ m{^/Dashboards/(\d+)?}) {
         if ( my $id = ( $1 || $HTML::Mason::Commands::DECODED_ARGS->{'id'} ) ) {
@@ -1621,18 +1621,6 @@ sub _BuildAdminMenu {
     }
 }
 
-sub _BuildLogoutMenu {
-    my $about_me = shift;
-
-    my $logout_url = RT->Config->Get('LogoutURL') || '/NoAuth/Logout.html';
-    # If user is not externally authenticated, show the logout link
-    # otherwise, show the logout link if LogoutURL is set to something other than the default
-    if ( !$HTML::Mason::Commands::session{'WebExternallyAuthed'} || $logout_url ne '/NoAuth/Logout.html' )
-    {
-        $about_me->child( logout => title => loc('Logout'), path => '/NoAuth/Logout.html' );
-    }
-}
-
 sub BuildSelfServiceNav {
     my $request_path = shift;
     my $top          = shift;
@@ -1704,7 +1692,7 @@ sub BuildSelfServiceNav {
     }
 
     if ( $current_user->Name ) {
-        _BuildLogoutMenu($about_me);
+        $about_me->child( logout => title => loc('Logout'), path => '/NoAuth/Logout.html' );
     }
 
     if ( RT->Config->Get('SelfServiceShowArticleSearch') ) {

commit 17b15ccd7372aec162f12ac1a9cc71b2eb52fb33
Author: Ronaldo Richieri <ronaldo at bestpractical.com>
Date:   Fri Jan 6 17:43:05 2023 -0300

    Update basic_auth.t test since logout will be always available

diff --git a/t/web/basic_auth.t b/t/web/basic_auth.t
index ff77f29f26..2eea552fec 100644
--- a/t/web/basic_auth.t
+++ b/t/web/basic_auth.t
@@ -23,7 +23,7 @@ $m->content_like(
     qr{<span class="current-user">\Qroot\E</span>}i,
     "Has user on the page"
 );
-$m->content_unlike(qr/Logout/i, "Has no logout button, no WebFallbackToRTLogin");
+$m->content_like(qr/Logout/i, "Has logout button");
 
 # Again, testing the plack middleware
 $m->get($url);

commit 600105ad04f070bd07bbc3e0899a242c3223f87a
Author: Ronaldo Richieri <ronaldo at bestpractical.com>
Date:   Fri Jan 6 17:20:51 2023 -0300

    Add source IP address to the external auth login log message

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index e5291ce861..17dfc131e2 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -806,7 +806,7 @@ sub AttemptExternalAuth {
         }
 
         if ( _UserLoggedIn() ) {
-            RT->Logger->info("Session created from REMOTE_USER for user $user");
+            RT->Logger->info("Session created from REMOTE_USER for user $user from " . RequestENV('REMOTE_ADDR'));
             $HTML::Mason::Commands::session{'WebExternallyAuthed'} = 1;
             $m->callback( %$ARGS, CallbackName => 'ExternalAuthSuccessfulLogin', CallbackPage => '/autohandler' );
             # It is possible that we did a redirect to the login page,

-----------------------------------------------------------------------


hooks/post-receive
-- 
rt


More information about the rt-commit mailing list