From fddd09875f04d080fb8fcdd7f439a9f7cf6183e8 Mon Sep 17 00:00:00 2001
From: Piotr Sawicki
Date: Mon, 17 Jul 2017 15:54:18 +0200
Subject: [PATCH] Add the 'privilege' parameter to popup response callback
Apart from that, a protection against sending multiple requests
with the same privilege has been added. A new status code:
PRIVACY_PRIVILEGE_MANAGER_ERROR_ALREADY_IN_PROGRESS informs
an application when the above error occurs.
Change-Id: Ifb2397c99452d56f7327f0be3f54ce33b766488f
---
src/capi/impl/privacy_privilege_manager.c | 7 +-
src/capi/include/privacy_privilege_manager.h | 17 +++--
src/capi/test/privacy_privilege_manager_test.cpp | 11 ++--
src/client/api/ApiInterface.h | 1 +
src/client/api/askuser-notification-client.cpp | 6 +-
src/client/impl/ApiInterfaceImpl.cpp | 84 ++++++++++++++++--------
src/client/impl/ApiInterfaceImpl.h | 8 ++-
src/client/impl/PopupCallbackClosure.h | 12 +++-
src/client/include/askuser-notification-client.h | 5 ++
9 files changed, 107 insertions(+), 44 deletions(-)
diff --git a/src/capi/impl/privacy_privilege_manager.c b/src/capi/impl/privacy_privilege_manager.c
index 3ff87bb..63c04fd 100644
--- a/src/capi/impl/privacy_privilege_manager.c
+++ b/src/capi/impl/privacy_privilege_manager.c
@@ -71,6 +71,8 @@ static ppm_error_e ask_user_to_ppm_error(int ask_error)
case ASKUSER_API_CONNECTION_ERROR:
ret = PRIVACY_PRIVILEGE_MANAGER_ERROR_IO_ERROR;
break;
+ case ASKUSER_API_ALREADY_IN_PROGRESS:
+ ret = PRIVACY_PRIVILEGE_MANAGER_ERROR_ALREADY_IN_PROGRESS;
default:
break;
}
@@ -194,7 +196,8 @@ static void ask_status_callback(int fd, int events, void *p_user_data)
}
static void ppm_popup_response_callback(UNUSED int request_id, askuser_call_cause cause,
- askuser_popup_result result, void *p_user_data)
+ askuser_popup_result result, const char *privilege,
+ void *p_user_data)
{
ppm_callback_closure *callback_closure = (ppm_callback_closure *) p_user_data;
@@ -209,7 +212,7 @@ static void ppm_popup_response_callback(UNUSED int request_id, askuser_call_caus
ppm_call_cause_e ppm_cause = askuser_client_popup_cause_to_ppm(cause);
ppm_check_result_e ppm_result = askuser_client_popup_result_to_ppm(result);
- callback_closure->callback(ppm_cause, ppm_result, callback_closure->user_data);
+ callback_closure->callback(ppm_cause, ppm_result, privilege, callback_closure->user_data);
free(callback_closure);
}
diff --git a/src/capi/include/privacy_privilege_manager.h b/src/capi/include/privacy_privilege_manager.h
index a433a18..e3a2e08 100644
--- a/src/capi/include/privacy_privilege_manager.h
+++ b/src/capi/include/privacy_privilege_manager.h
@@ -40,6 +40,8 @@ typedef enum
PRIVACY_PRIVILEGE_MANAGER_ERROR_IO_ERROR = TIZEN_ERROR_IO_ERROR,
/**< Invalid parameter */
PRIVACY_PRIVILEGE_MANAGER_ERROR_INVALID_PARAMETER = TIZEN_ERROR_INVALID_PARAMETER,
+ /**< Operation already in progress */
+ PRIVACY_PRIVILEGE_MANAGER_ERROR_ALREADY_IN_PROGRESS = TIZEN_ERROR_ALREADY_IN_PROGRESS,
/**< Out of memory */
PRIVACY_PRIVILEGE_MANAGER_ERROR_OUT_OF_MEMORY = TIZEN_ERROR_OUT_OF_MEMORY,
/**< Unknown error */
@@ -93,13 +95,15 @@ typedef enum {
* @param[in] result A result of a response triggered by calling ppm_popup_request().
* This is a valid value only if the @a cause parameter is equal to
* **PRIVACY_PRIVILEGE_MANAGER_CALL_CAUSE_ANSWER**.
- * @param[in] user_data User specific data, this parameter is passed
+ * @param[in] privilege A privilege that has been checked.
+ * @param[in] user_data User specific data, this parameter has been passed
* to ppm_popup_request().
*
* @see ppm_popup_request()
*/
typedef void (*ppm_popup_response_cb) (ppm_call_cause_e cause,
ppm_popup_result_e result,
+ const char *privilege,
void *user_data);
/**
@@ -153,11 +157,12 @@ int ppm_check_privilege(const char *privilege, ppm_check_result_e *result);
* the registered callback.
*
* @return 0 on success, otherwise a negative error value
- * @retval #PRIVACY_PRIVILEGE_MANAGER_ERROR_NONE Successful
- * @retval #PRIVACY_PRIVILEGE_MANAGER_ERROR_IO_ERROR I/O error
- * @retval #PRIVACY_PRIVILEGE_MANAGER_ERROR_INVALID_PARAMETER Invalid parameter
- * @retval #PRIVACY_PRIVILEGE_MANAGER_ERROR_OUT_OF_MEMORY Out of memory
- * @retval #PRIVACY_PRIVILEGE_MANAGER_ERROR_UNKNOWN Unknown error
+ * @retval #PRIVACY_PRIVILEGE_MANAGER_ERROR_NONE Successful
+ * @retval #PRIVACY_PRIVILEGE_MANAGER_ERROR_IO_ERROR I/O error
+ * @retval #PRIVACY_PRIVILEGE_MANAGER_ERROR_INVALID_PARAMETER Invalid parameter
+ * @retval #PRIVACY_PRIVILEGE_MANAGER_ERROR_OUT_OF_MEMORY Out of memory
+ * @retval #PRIVACY_PRIVILEGE_MANAGER_ERROR_ALREADY_IN_PROGRESS Operation already in progress
+ * @retval #PRIVACY_PRIVILEGE_MANAGER_ERROR_UNKNOWN Unknown error
*
* @post ppm_popup_response_cb() will be invoked.
* @see ppm_popup_response_cb()
diff --git a/src/capi/test/privacy_privilege_manager_test.cpp b/src/capi/test/privacy_privilege_manager_test.cpp
index b7866c7..5138b89 100644
--- a/src/capi/test/privacy_privilege_manager_test.cpp
+++ b/src/capi/test/privacy_privilege_manager_test.cpp
@@ -60,7 +60,6 @@ typedef std::set PrivilegeRequestSet;
struct ClientRequest {
RequestId m_id;
- Privilege m_privilege;
GMainLoop *m_mainLoop;
bool m_stopAppAfterDeny;
};
@@ -156,6 +155,9 @@ void printClientError(ppm_error_e error)
case PRIVACY_PRIVILEGE_MANAGER_ERROR_UNKNOWN:
printf("Unknown error\n");
break;
+ case PRIVACY_PRIVILEGE_MANAGER_ERROR_ALREADY_IN_PROGRESS:
+ printf("Operation already in progress\n");
+ break;
}
}
@@ -439,14 +441,15 @@ public:
private:
- static void popupResponseCallback(ppm_call_cause_e cause, ppm_popup_result_e result, void *user_data) {
+ static void popupResponseCallback(ppm_call_cause_e cause, ppm_popup_result_e result,
+ const char *privilege, void *user_data) {
ClientRequest *request = static_cast(user_data);
if (request == nullptr) {
printf("ERROR: User data is nullptr\n");
return;
}
- printf("localId: %d privilege: \"%s\" ", request->m_id, request->m_privilege.c_str());
+ printf("localId: %d privilege: \"%s\" ", request->m_id, privilege);
switch (cause) {
case PRIVACY_PRIVILEGE_MANAGER_CALL_CAUSE_ANSWER:
@@ -498,7 +501,7 @@ private:
printf("sending localId: %d privilege: \"%s\"\n", clientRequestId, privilege);
- request = new ClientRequest{ clientRequestId++, privilege, m_mainloop, stopAppAfterDeny };
+ request = new ClientRequest{ clientRequestId++, m_mainloop, stopAppAfterDeny };
err = static_cast(ppm_popup_request(privilege, &AppContext::popupResponseCallback, request));
if (err != PRIVACY_PRIVILEGE_MANAGER_ERROR_NONE) {
delete request;
diff --git a/src/client/api/ApiInterface.h b/src/client/api/ApiInterface.h
index 9aef998..87bb603 100644
--- a/src/client/api/ApiInterface.h
+++ b/src/client/api/ApiInterface.h
@@ -42,6 +42,7 @@ public:
virtual askuser_check_result checkPrivilege(const std::string &privilege) = 0;
virtual RequestId popupRequest(const PopupCallbackClosure &closure,
const std::string &privilege) = 0;
+ virtual bool popupRequestInProgress(const std::string &privilege) const = 0;
};
} // namespace Client
diff --git a/src/client/api/askuser-notification-client.cpp b/src/client/api/askuser-notification-client.cpp
index f0d8ff3..9de7c6d 100644
--- a/src/client/api/askuser-notification-client.cpp
+++ b/src/client/api/askuser-notification-client.cpp
@@ -116,7 +116,11 @@ int askuser_client_popup_request(askuser_client *p_client, const char *privilege
}
return AskUser::Client::tryCatch([&]() {
- AskUser::Client::PopupCallbackClosure closure(response_callback, p_user_data);
+ if (p_client->impl->popupRequestInProgress(privilege)) {
+ return ASKUSER_API_ALREADY_IN_PROGRESS;
+ }
+
+ AskUser::Client::PopupCallbackClosure closure(response_callback, privilege, p_user_data);
AskUser::Client::RequestId id = p_client->impl->popupRequest(closure, privilege);
if (p_request_id) {
diff --git a/src/client/impl/ApiInterfaceImpl.cpp b/src/client/impl/ApiInterfaceImpl.cpp
index 8cc2593..12021ed 100644
--- a/src/client/impl/ApiInterfaceImpl.cpp
+++ b/src/client/impl/ApiInterfaceImpl.cpp
@@ -33,19 +33,19 @@
namespace {
-int eventsToAskUserMask(int events)
+inline int eventsToAskUserMask(int events)
{
return ((events & ASKUSER_READ_EVENT) ? AskUser::Protocol::FdMask::READ : 0) |
((events & ASKUSER_WRITE_EVENT) ? AskUser::Protocol::FdMask::WRITE : 0);
}
-int askUserMaskToEvents(int mask)
+inline int askUserMaskToEvents(int mask)
{
return ((AskUser::Protocol::FdMask::READ & mask) ? ASKUSER_READ_EVENT : 0) |
((AskUser::Protocol::FdMask::WRITE & mask) ? ASKUSER_WRITE_EVENT : 0);
}
-askuser_popup_result responseToAskUserPopupResult(int response)
+inline askuser_popup_result responseToAskUserPopupResult(int response)
{
switch (response) {
case ASKUSER_ALLOW_FOREVER:
@@ -59,6 +59,15 @@ askuser_popup_result responseToAskUserPopupResult(int response)
return ASKUSER_POPUP_RESULT_DENY_ONCE;
}
+inline askuser_call_cause deduceCauseFromResponse(int response)
+{
+ if (response == ASKUSER_UNKNOWN_ERROR) {
+ return ASKUSER_CALL_CAUSE_ERROR;
+ }
+
+ return ASKUSER_CALL_CAUSE_ANSWER;
+}
+
} // namespace
namespace AskUser {
@@ -75,12 +84,8 @@ ApiInterfaceImpl::ApiInterfaceImpl(const StatusCallbackClosure &statusClosure)
ApiInterfaceImpl::~ApiInterfaceImpl()
{
- for (const auto &closure : m_callbacks) {
- closure.second(closure.first, askuser_call_cause::ASKUSER_CALL_CAUSE_FINALIZE,
- askuser_popup_result::ASKUSER_POPUP_RESULT_DENY_ONCE);
- }
+ respondToAllRequests(ASKUSER_CALL_CAUSE_FINALIZE, ASKUSER_POPUP_RESULT_DENY_ONCE);
- m_callbacks.clear();
m_channel.reset();
}
@@ -102,7 +107,8 @@ askuser_check_result ApiInterfaceImpl::checkPrivilege(const std::string &privile
auto policies = fetch.fetchPolicy();
if (policies.size() != 1) {
- ALOGE("Unusual situation, there are " << policies.size() << " policies for (" << appId << ", " << geteuid() << ", " << privilege << ")");
+ ALOGE("Unusual situation, there are " << policies.size() <<
+ " policies for (" << appId << ", " << geteuid() << ", " << privilege << ")");
return ASKUSER_CHECK_RESULT_DENY;
}
@@ -126,48 +132,70 @@ askuser_check_result ApiInterfaceImpl::checkPrivilege(const std::string &privile
RequestId ApiInterfaceImpl::popupRequest(const PopupCallbackClosure &closure,
const std::string &privilege)
{
- Client::RequestId id = static_cast(m_channel->popupRequest(privilege));
+ RequestId id = static_cast(m_channel->popupRequest(privilege));
+
+ auto it = m_popupClosures.find(id);
+ if (it != m_popupClosures.end()) {
+ ALOGE("Popup closure exists for id: " << id <<
+ " privilege: " << it->second.privilege() << ", replacing");
+ popupResponse(id, ASKUSER_UNKNOWN_ERROR);
+ }
- auto it = m_callbacks.find(id);
- if (it != m_callbacks.end()) {
- it->second(it->first, ASKUSER_CALL_CAUSE_ERROR, ASKUSER_POPUP_RESULT_DENY_ONCE);
- m_callbacks.erase(it);
+ if (popupRequestInProgress(privilege)) {
+ ALOGE("Privilege " << closure.privilege() << " already exists in the pending privileges set");
}
- m_callbacks.insert({id, closure});
+ m_requestedPrivileges.insert(privilege);
+ m_popupClosures.insert({id, closure});
return id;
}
+bool ApiInterfaceImpl::popupRequestInProgress(const std::string &privilege) const
+{
+ return m_requestedPrivileges.find(privilege) != m_requestedPrivileges.end();
+}
+
void ApiInterfaceImpl::updateConnection(Protocol::ConnectionFd fd, int mask)
{
m_statusClosure(fd, askUserMaskToEvents(mask));
- // remove all pending events
+ // the connection is about to close, respond to all pending requests
if (mask == ASKUSER_EMPTY_EVENTS) {
- for (const auto &closure : m_callbacks) {
- closure.second(closure.first, ASKUSER_CALL_CAUSE_ERROR, ASKUSER_POPUP_RESULT_DENY_ONCE);
- }
-
- m_callbacks.clear();
+ respondToAllRequests(ASKUSER_CALL_CAUSE_ERROR, ASKUSER_POPUP_RESULT_DENY_ONCE);
}
}
void ApiInterfaceImpl::popupResponse(Protocol::RequestId id, int response)
{
- auto it = m_callbacks.find(id);
- if (it == m_callbacks.end()) {
+ auto it = m_popupClosures.find(id);
+ if (it == m_popupClosures.end()) {
+ ALOGE("Couldn't find popup callback closure for id: " << id);
return;
}
- askuser_call_cause cause = ASKUSER_CALL_CAUSE_ANSWER;
- if (response == ASKUSER_UNKNOWN_ERROR) {
- cause = ASKUSER_CALL_CAUSE_ERROR;
+ const auto &closure = it->second;
+ if (!popupRequestInProgress(closure.privilege())) {
+ ALOGE("Couldn't find privilege " << closure.privilege() << " in the pending privileges set");
}
+ askuser_call_cause cause = deduceCauseFromResponse(response);
askuser_popup_result res = responseToAskUserPopupResult(response);
- it->second(id, cause, res);
- m_callbacks.erase(it);
+
+ closure(id, cause, res);
+
+ m_requestedPrivileges.erase(closure.privilege());
+ m_popupClosures.erase(it);
+}
+
+void ApiInterfaceImpl::respondToAllRequests(askuser_call_cause cause, askuser_popup_result result)
+{
+ for (const auto &closure : m_popupClosures) {
+ closure.second(closure.first, cause, result);
+ }
+
+ m_requestedPrivileges.clear();
+ m_popupClosures.clear();
}
} // namespace Client
diff --git a/src/client/impl/ApiInterfaceImpl.h b/src/client/impl/ApiInterfaceImpl.h
index 3734a68..efa97c6 100644
--- a/src/client/impl/ApiInterfaceImpl.h
+++ b/src/client/impl/ApiInterfaceImpl.h
@@ -22,7 +22,9 @@
#pragma once
+#include