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

Proposed by Andrea Azzarone on 2019-02-18
Status: Approved
Approved by: Marco Trevisan (TreviƱo) on 2019-02-18
Approved revision: b23c0a1083665f51b2a54a784acd711352365727
Proposed branch: ~azzar1/unity:fix-1733557-bionic
Merge into: unity:bionic
Diff against target: 108 lines (+32/-3)
4 files modified
lockscreen/LockScreenController.cpp (+2/-0)
lockscreen/UserAuthenticator.h (+1/-0)
lockscreen/UserAuthenticatorPam.cpp (+27/-3)
lockscreen/UserAuthenticatorPam.h (+2/-0)
Reviewer Review Type Date Requested Status
Unity Team 2019-02-18 Pending
Review via email: mp+363310@code.launchpad.net

Commit message

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

To post a comment you must log in.

Unmerged commits

b23c0a1... by Andrea Azzarone on 2019-02-13

locksreen: 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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lockscreen/LockScreenController.cpp b/lockscreen/LockScreenController.cpp
2index 8f6e7bc..907a130 100644
3--- a/lockscreen/LockScreenController.cpp
4+++ b/lockscreen/LockScreenController.cpp
5@@ -536,6 +536,8 @@ void Controller::OnUnlockRequested()
6
7 HideBlankWindow();
8 HideShields();
9+
10+ user_authenticator_->AuthenticateCancel();
11 }
12
13 void Controller::HideShields()
14diff --git a/lockscreen/UserAuthenticator.h b/lockscreen/UserAuthenticator.h
15index e30ce2f..60e8684 100644
16--- a/lockscreen/UserAuthenticator.h
17+++ b/lockscreen/UserAuthenticator.h
18@@ -43,6 +43,7 @@ public:
19
20 // Authenticate the user in a background thread.
21 virtual bool AuthenticateStart(std::string const& username, AuthenticateEndCallback const&) = 0;
22+ virtual void AuthenticateCancel() = 0;
23
24 sigc::signal<void> start_failed;
25 sigc::signal<void, std::string, PromiseAuthCodePtr const&> echo_on_requested;
26diff --git a/lockscreen/UserAuthenticatorPam.cpp b/lockscreen/UserAuthenticatorPam.cpp
27index 463d8f9..d1d6c27 100644
28--- a/lockscreen/UserAuthenticatorPam.cpp
29+++ b/lockscreen/UserAuthenticatorPam.cpp
30@@ -50,6 +50,7 @@ bool UserAuthenticatorPam::AuthenticateStart(std::string const& username,
31 }
32
33 first_prompt_ = true;
34+ cancelled_ = false;
35 username_ = username;
36 authenticate_cb_ = authenticate_cb;
37
38@@ -64,6 +65,18 @@ bool UserAuthenticatorPam::AuthenticateStart(std::string const& username,
39 return !error;
40 }
41
42+void UserAuthenticatorPam::AuthenticateCancel()
43+{
44+ if (!pam_handle_)
45+ {
46+ LOG_DEBUG(logger) << "Unable to cancel authentication because none has been started";
47+ return;
48+ }
49+
50+ LOG_DEBUG(logger) << "Cancelling the authentication";
51+ cancelled_ = true;
52+}
53+
54 gpointer UserAuthenticatorPam::AuthenticationThreadFunc(gpointer data)
55 {
56 auto self = static_cast<UserAuthenticatorPam*>(data);
57@@ -92,7 +105,10 @@ gpointer UserAuthenticatorPam::AuthenticationThreadFunc(gpointer data)
58
59 pam_end(self->pam_handle_, self->status_);
60 self->pam_handle_ = nullptr;
61- self->source_manager_.AddTimeout(0, [self] { self->authenticate_cb_(self->status_ == PAM_SUCCESS); return false; });
62+
63+ if (!self->cancelled_)
64+ self->source_manager_.AddTimeout(0, [self] { self->authenticate_cb_(self->status_ == PAM_SUCCESS); return false; });
65+
66 return nullptr;
67 }
68
69@@ -188,9 +204,17 @@ int UserAuthenticatorPam::ConversationFunction(int num_msg,
70 auto future = promise->get_future();
71 pam_response* resp_item = &tmp_response[i++];
72 resp_item->resp_retcode = 0;
73- resp_item->resp = strdup(future.get().c_str());
74+ resp_item->resp = nullptr;
75+
76+ std::future_status status;
77+ do
78+ {
79+ status = future.wait_for(std::chrono::seconds(1));
80+ if (status == std::future_status::ready)
81+ resp_item->resp = strdup(future.get().c_str());
82+ } while (status != std::future_status::ready && !user_auth->cancelled_);
83
84- if (!resp_item->resp)
85+ if (!resp_item->resp || user_auth->cancelled_)
86 {
87 raise_error = true;
88 break;
89diff --git a/lockscreen/UserAuthenticatorPam.h b/lockscreen/UserAuthenticatorPam.h
90index 82273cd..4043cd4 100644
91--- a/lockscreen/UserAuthenticatorPam.h
92+++ b/lockscreen/UserAuthenticatorPam.h
93@@ -39,6 +39,7 @@ class UserAuthenticatorPam : public UserAuthenticator
94 public:
95 UserAuthenticatorPam() = default;
96 bool AuthenticateStart(std::string const& username, AuthenticateEndCallback const&) override;
97+ void AuthenticateCancel() override;
98
99 private:
100 UserAuthenticatorPam(UserAuthenticatorPam const&) = delete;
101@@ -57,6 +58,7 @@ private:
102
103 int status_ = 0;
104 bool first_prompt_ = true;
105+ bool cancelled_ = false;
106 pam_handle* pam_handle_ = nullptr;
107 glib::SourceManager source_manager_;
108 };

Subscribers

People subscribed via source and target branches