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 #include +#include #include #include @@ -48,14 +50,18 @@ public: virtual askuser_check_result checkPrivilege(const std::string &privilege); virtual RequestId popupRequest(const PopupCallbackClosure &closure, const std::string &privilege); + virtual bool popupRequestInProgress(const std::string &privilege) const; void updateConnection(Protocol::ConnectionFd fd, int mask); void popupResponse(Protocol::RequestId id, int response); private: + void respondToAllRequests(askuser_call_cause cause, askuser_popup_result result); + StatusCallbackClosure m_statusClosure; std::unique_ptr m_channel; - std::map m_callbacks; + std::map m_popupClosures; + std::set m_requestedPrivileges; }; } // namespace Client diff --git a/src/client/impl/PopupCallbackClosure.h b/src/client/impl/PopupCallbackClosure.h index 7f395c7..9bb2953 100644 --- a/src/client/impl/PopupCallbackClosure.h +++ b/src/client/impl/PopupCallbackClosure.h @@ -22,6 +22,8 @@ #pragma once +#include + #include namespace AskUser { @@ -30,18 +32,24 @@ namespace Client { class PopupCallbackClosure { public: - PopupCallbackClosure(askuser_popup_response_callback callback, void *userData) + PopupCallbackClosure(askuser_popup_response_callback callback, const char *privilege, void *userData) : m_callback(callback) + , m_privilege(privilege) , m_userData(userData) {} void operator()(int requestId, askuser_call_cause cause, askuser_popup_result result) const { - m_callback(requestId, cause, result, m_userData); + m_callback(requestId, cause, result, m_privilege.c_str(), m_userData); + } + + const std::string &privilege() const { + return m_privilege; } private: askuser_popup_response_callback m_callback; + std::string m_privilege; void *m_userData; }; diff --git a/src/client/include/askuser-notification-client.h b/src/client/include/askuser-notification-client.h index 1b32d14..8398fdd 100644 --- a/src/client/include/askuser-notification-client.h +++ b/src/client/include/askuser-notification-client.h @@ -43,6 +43,9 @@ /*! \brief indicating that a connection error occurred */ #define ASKUSER_API_CONNECTION_ERROR -4 +/*! \brief indicating that a request has already been sent */ +#define ASKUSER_API_ALREADY_IN_PROGRESS -5 + /** @} */ /** @@ -199,12 +202,14 @@ typedef void (*askuser_status_callback) (int fd, int events, void *p_user_data); * askuser_client_popup_request(). This should be * interpreted as a valid value only if cause is equal to * askuser_call_cause::ASKUSER_CALL_CAUSE_ANSWER. + * \param[in] privilege A privilege that has been checked. * \param[in] p_user_data User specific data, this parameter was previously * passed to askuser_client_popup_request(). */ typedef void (*askuser_popup_response_callback) (int request_id, askuser_call_cause cause, askuser_popup_result result, + const char *privilege, void *p_user_data); /** -- 2.7.4