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

Proposed by Andrea Azzarone on 2019-02-13
Status: Merged
Approved by: Marco Trevisan (Treviño) on 2019-02-15
Approved revision: b5582152d954cd795c49fbf515e221e50a935e52
Merged at revision: bada2f8fe60fbe25bc9733a1ffba908306260bd9
Proposed branch: ~azzar1/unity:fix-1733557
Merge into: unity:master
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
Marco Trevisan (Treviño) 2019-02-13 Approve on 2019-02-15
Review via email: mp+363124@code.launchpad.net

Commit message

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

To post a comment you must log in.
Rael Gugelmin Cunha (rael-gc) wrote :

Just a reminder that this issue affects 16.04 and 18.04 too.

Marco Trevisan (Treviño) (3v1n0) wrote :

Looks fine

review: Approve

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