Merge ~azzar1/unity:fix-1733557-xenial into unity:xenial

[Andrea Azzarone]

lockscreen: cancel authentication if externally unlocked

The lockscreen can be externellay unlocked. This happens e.g. when
you unlock the session from LightDM or from the CLI. We need to
cancel the authentication in order to ensure that the next one does
not fail.

Fixes: https://bugs.launchpad.net/unity/+bug/1733557

[Marco Trevisan (Treviño) (3v1n0)]

review: Approve

diff --git a/lockscreen/LockScreenController.cpp b/lockscreen/LockScreenController.cpp
index 5f5d54c..f8ba8c4 100644
--- a/lockscreen/LockScreenController.cpp
+++ b/lockscreen/LockScreenController.cpp
@@ -531,6 +531,8 @@ void Controller::OnUnlockRequested()
 
   HideBlankWindow();
   HideShields();
+
+  user_authenticator_->AuthenticateCancel();
 }
 
 void Controller::HideShields()
diff --git a/lockscreen/UserAuthenticator.h b/lockscreen/UserAuthenticator.h
index e30ce2f..60e8684 100644
--- a/lockscreen/UserAuthenticator.h
+++ b/lockscreen/UserAuthenticator.h
@@ -43,6 +43,7 @@ public:
 
   // Authenticate the user in a background thread.
   virtual bool AuthenticateStart(std::string const& username, AuthenticateEndCallback const&) = 0;
+  virtual void AuthenticateCancel() = 0;
 
   sigc::signal<void> start_failed;
   sigc::signal<void, std::string, PromiseAuthCodePtr const&> echo_on_requested;
diff --git a/lockscreen/UserAuthenticatorPam.cpp b/lockscreen/UserAuthenticatorPam.cpp
index f4e1952..d1d6c27 100644
--- a/lockscreen/UserAuthenticatorPam.cpp
+++ b/lockscreen/UserAuthenticatorPam.cpp
@@ -25,6 +25,8 @@
 #include "unity-shared/UnitySettings.h"
 #include "UnityCore/GLibWrapper.h"
 
+#include <NuxCore/Logger.h>
+
 #include <cstring>
 #include <security/pam_appl.h>
 #include <vector>
@@ -33,23 +35,48 @@ namespace unity
 {
 namespace lockscreen
 {
+namespace
+{
+DECLARE_LOGGER(logger, "unity.lockscreen");
+}
 
 bool UserAuthenticatorPam::AuthenticateStart(std::string const& username,
                                              AuthenticateEndCallback const& authenticate_cb)
 {
   if (pam_handle_)
+  {
+    LOG_ERROR(logger) << "Unable to start authentication because another one has already been started";
     return false;
+  }
 
   first_prompt_ = true;
+  cancelled_ = false;
   username_ = username;
   authenticate_cb_ = authenticate_cb;
 
   glib::Error error;
-  g_thread_try_new(nullptr, AuthenticationThreadFunc, this, &error);
+  g_autoptr(GThread) thread = g_thread_try_new(nullptr, AuthenticationThreadFunc, this, &error);
+
+  if (!thread || error)
+  {
+    LOG_ERROR(logger) << "Unable to create a new thread for PAM authentication: " << error.Message();
+  }
 
   return !error;
 }
 
+void UserAuthenticatorPam::AuthenticateCancel()
+{
+  if (!pam_handle_)
+  {
+    LOG_DEBUG(logger) << "Unable to cancel authentication because none has been started";
+    return;
+  }
+
+  LOG_DEBUG(logger) << "Cancelling the authentication";
+  cancelled_ = true;
+}
+
 gpointer UserAuthenticatorPam::AuthenticationThreadFunc(gpointer data)
 {
   auto self = static_cast<UserAuthenticatorPam*>(data);
@@ -78,7 +105,10 @@ gpointer UserAuthenticatorPam::AuthenticationThreadFunc(gpointer data)
 
   pam_end(self->pam_handle_, self->status_);
   self->pam_handle_ = nullptr;
-  self->source_manager_.AddTimeout(0, [self] {   self->authenticate_cb_(self->status_ == PAM_SUCCESS); return false; });
+
+  if (!self->cancelled_)
+    self->source_manager_.AddTimeout(0, [self] {   self->authenticate_cb_(self->status_ == PAM_SUCCESS); return false; });
+
   return nullptr;
 }
 
@@ -88,8 +118,16 @@ bool UserAuthenticatorPam::InitPam()
   conversation.conv = ConversationFunction;
   conversation.appdata_ptr = static_cast<void*>(this);
 
-  return pam_start("unity", username_.c_str(),
-                   &conversation, &pam_handle_) == PAM_SUCCESS;
+  int ret = pam_start("unity", username_.c_str(),
+                      &conversation, &pam_handle_);
+
+  if (ret != PAM_SUCCESS)
+  {
+    LOG_ERROR(logger) << "Unable to start pam: " << pam_strerror(pam_handle_, ret);
+    return false;
+  }
+
+  return true;
 }
 
 int UserAuthenticatorPam::ConversationFunction(int num_msg,
@@ -166,9 +204,17 @@ int UserAuthenticatorPam::ConversationFunction(int num_msg,
     auto future = promise->get_future();
     pam_response* resp_item = &tmp_response[i++];
     resp_item->resp_retcode = 0;
-    resp_item->resp = strdup(future.get().c_str());
+    resp_item->resp = nullptr;
+
+    std::future_status status;
+    do
+    {
+        status = future.wait_for(std::chrono::seconds(1));
+        if (status == std::future_status::ready)
+          resp_item->resp = strdup(future.get().c_str());
+    } while (status != std::future_status::ready && !user_auth->cancelled_);
 
-    if (!resp_item->resp)
+    if (!resp_item->resp || user_auth->cancelled_)
     {
       raise_error = true;
       break;
diff --git a/lockscreen/UserAuthenticatorPam.h b/lockscreen/UserAuthenticatorPam.h
index 82273cd..4043cd4 100644
--- a/lockscreen/UserAuthenticatorPam.h
+++ b/lockscreen/UserAuthenticatorPam.h
@@ -39,6 +39,7 @@ class UserAuthenticatorPam : public UserAuthenticator
 public:
   UserAuthenticatorPam() = default;
   bool AuthenticateStart(std::string const& username, AuthenticateEndCallback const&) override;
+  void AuthenticateCancel() override;
 
 private:
   UserAuthenticatorPam(UserAuthenticatorPam const&) = delete;
@@ -57,6 +58,7 @@ private:
 
   int status_ = 0;
   bool first_prompt_ = true;
+  bool cancelled_ = false;
   pam_handle* pam_handle_ = nullptr;
   glib::SourceManager source_manager_;
 };