Try to return a redirect instead a 400 for "not logged in" state
authorRob Crittenden <rcritten@redhat.com>
Wed, 25 Mar 2015 21:29:22 +0000 (17:29 -0400)
committerPatrick Uiterwijk <puiterwijk@redhat.com>
Fri, 27 Mar 2015 18:43:26 +0000 (19:43 +0100)
If the user is not logged in and submits a valid logout request
then just redirect the user to the RelayState in the request
indicating that the logout was successful. This provides a better
user experience.

https://fedorahosted.org/ipsilon/ticket/88

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-by: Patrick Uiterwijk <puiterwijk@redhat.com>
ipsilon/providers/saml2/logout.py

index da8edcf..bfb5d0d 100644 (file)
@@ -80,15 +80,14 @@ class LogoutRequest(ProviderPageBase):
                 self.error('loading session failed: %s' % e)
                 raise cherrypy.HTTPError(400, 'Invalid logout session')
         else:
                 self.error('loading session failed: %s' % e)
                 raise cherrypy.HTTPError(400, 'Invalid logout session')
         else:
-            self.error('Logout attempt without being loggged in.')
-            raise cherrypy.HTTPError(400, 'Not logged in')
+            return self._not_logged_in(logout, message)
 
         try:
             logout.validateRequest()
         except lasso.ProfileSessionNotFoundError, e:
             self.error('Logout failed. No sessions for %s' %
                        logout.remoteProviderId)
 
         try:
             logout.validateRequest()
         except lasso.ProfileSessionNotFoundError, e:
             self.error('Logout failed. No sessions for %s' %
                        logout.remoteProviderId)
-            raise cherrypy.HTTPError(400, 'Not logged in')
+            return self._not_logged_in(logout, message)
         except lasso.LogoutUnsupportedProfileError:
             self.error('Logout failed. Unsupported profile %s' %
                        logout.remoteProviderId)
         except lasso.LogoutUnsupportedProfileError:
             self.error('Logout failed. Unsupported profile %s' %
                        logout.remoteProviderId)
@@ -156,11 +155,48 @@ class LogoutRequest(ProviderPageBase):
                             (user.name, user.fullname,
                              logout.remoteProviderId))
             else:
                             (user.name, user.fullname,
                              logout.remoteProviderId))
             else:
-                self.error('Logout attempt without being loggged in.')
-                raise cherrypy.HTTPError(400, 'Not logged in')
+                return self._not_logged_in(logout, message)
 
         return
 
 
         return
 
+    def _not_logged_in(self, logout, message):
+        """
+        The user requested a logout but isn't logged in, or we can't
+        find a session for the user. Try to be nice and redirect them
+        back to the RelayState in the logout request.
+
+        We are only nice in the case of a valid logout request. If the
+        request is invalid (not signed, unknown SP, etc) then an
+        exception is raised.
+        """
+        self.error('Logout attempt without being logged in.')
+
+        if logout.msgRelayState is not None:
+            raise cherrypy.HTTPRedirect(logout.msgRelayState)
+
+        try:
+            logout.processRequestMsg(message)
+        except (lasso.ServerProviderNotFoundError,
+                lasso.ProfileUnknownProviderError) as e:
+            msg = 'Invalid SP [%s] (%r [%r])' % (logout.remoteProviderId,
+                                                 e, message)
+            self.error(msg)
+            raise UnknownProvider(msg)
+        except (lasso.ProfileInvalidProtocolprofileError,
+                lasso.DsError), e:
+            msg = 'Invalid SAML Request: %r (%r [%r])' % (logout.request,
+                                                          e, message)
+            self.error(msg)
+            raise InvalidRequest(msg)
+        except lasso.Error, e:
+            self.error('SLO unknown error: %s' % message)
+            raise cherrypy.HTTPError(400, 'Invalid logout request')
+
+        if logout.msgRelayState:
+            raise cherrypy.HTTPRedirect(logout.msgRelayState)
+        else:
+            raise cherrypy.HTTPError(400, 'Not logged in')
+
     def logout(self, message, relaystate=None, samlresponse=None):
         """
         Handle HTTP Redirect logout. This is an asynchronous logout
     def logout(self, message, relaystate=None, samlresponse=None):
         """
         Handle HTTP Redirect logout. This is an asynchronous logout
@@ -186,8 +222,7 @@ class LogoutRequest(ProviderPageBase):
         saml_sessions = us.get_provider_data('saml2')
         if saml_sessions is None:
             # No sessions means nothing to log out
         saml_sessions = us.get_provider_data('saml2')
         if saml_sessions is None:
             # No sessions means nothing to log out
-            self.error('Logout attempt without being loggged in.')
-            raise cherrypy.HTTPError(400, 'Not logged in')
+            return self._not_logged_in(logout, message)
 
         self.debug('%d sessions loaded' % saml_sessions.count())
         saml_sessions.dump()
 
         self.debug('%d sessions loaded' % saml_sessions.count())
         saml_sessions.dump()
@@ -244,8 +279,7 @@ class LogoutRequest(ProviderPageBase):
 
         saml_sessions = us.get_provider_data('saml2')
         if saml_sessions is None or saml_sessions.count() == 0:
 
         saml_sessions = us.get_provider_data('saml2')
         if saml_sessions is None or saml_sessions.count() == 0:
-            self.error('Logout attempt without being loggged in.')
-            raise cherrypy.HTTPError(400, 'Not logged in')
+            return self._not_logged_in(logout, message)
 
         try:
             session = saml_sessions.get_last_session()
 
         try:
             session = saml_sessions.get_last_session()