resolve cancel issue; fix context not free issue

Change-Id: If4b8db8df4d8365e876819650f5d0a479e1fa2d9
Signed-off-by: Tianshi Liu <tianshi.liu@huawei.com>
This commit is contained in:
Tianshi Liu 2022-06-27 17:56:30 +08:00
parent f68b46dc8c
commit f2548d40dd
9 changed files with 101 additions and 86 deletions

View File

@ -24,6 +24,7 @@
#include "iam_check.h"
#include "iam_logger.h"
#include "iam_mem.h"
#include "iam_para2str.h"
#include "iam_ptr.h"
#include "iam_types.h"
#include "identify_command.h"
@ -87,28 +88,10 @@ ResultCode FrameworkExecutorCallback::OnEndExecuteInner(
uint64_t scheduleId, std::shared_ptr<UserIam::UserAuth::Attributes> consumerAttr)
{
IF_FALSE_LOGE_AND_RETURN_VAL(consumerAttr != nullptr, ResultCode::GENERAL_ERROR);
uint32_t commandId = 0;
bool getScheduleModeRet =
consumerAttr->GetUint32Value(UserIam::UserAuth::Attributes::ATTR_SCHEDULE_MODE, commandId);
IF_FALSE_LOGE_AND_RETURN_VAL(getScheduleModeRet == true, ResultCode::GENERAL_ERROR);
IAM_LOGI("%{public}s start process cmd %{public}u", GetDescription(), commandId);
ResultCode ret = ResultCode::GENERAL_ERROR;
switch (commandId) {
case UserIam::UserAuth::AUTH:
ret = ProcessCancelCommand(scheduleId);
break;
case UserIam::UserAuth::ENROLL:
ret = ProcessCancelCommand(scheduleId);
break;
case UserIam::UserAuth::IDENTIFY:
ret = ProcessCancelCommand(scheduleId);
break;
default:
IAM_LOGE("Command id %{public}u is not supported", commandId);
break;
}
IAM_LOGI("command id = %{public}u ret = %{public}d", commandId, ret);
ResultCode ret = ProcessCancelCommand(scheduleId);
IAM_LOGI("%{public}s cancel scheduleId %{public}s ret %{public}d", GetDescription(),
GET_MASKED_STRING(scheduleId).c_str(), ret);
return ret;
}

View File

@ -156,7 +156,7 @@ uint64_t ContextPool::GetNewContextId()
for (uint32_t i = 0; i < MAX_TRY_TIMES; i++) {
RAND_bytes(contextIdPtr, sizeof(uint64_t));
if (contextId == 0 || ContextPool::Instance().Select(contextId).lock() != nullptr) {
IAM_LOGE("duplicate context id");
IAM_LOGE("invalid or duplicate context id");
continue;
}
}

View File

@ -369,6 +369,7 @@ void ScheduleNodeImpl::OnScheduleFinished(FiniteStateMachine &machine, uint32_t
auto result = result_.value();
info_.callback->OnScheduleStoped(result.first, result.second);
info_.callback = nullptr;
}
} // namespace UserAuth
} // namespace UserIam

View File

@ -18,11 +18,15 @@
#include "user_idm_stub.h"
#include <memory>
#include <string>
#include <vector>
#include "system_ability.h"
#include "system_ability_definition.h"
#include "context.h"
namespace OHOS {
namespace UserIam {
namespace UserAuth {
@ -50,7 +54,9 @@ public:
const sptr<IdmCallback> &callback) override;
private:
uint64_t contextId_ {0};
int32_t CancelCurrentEnroll();
void CancelCurrentEnrollIfExist();
std::mutex mutex_;
};
} // namespace UserAuth
} // namespace UserIam

View File

@ -282,11 +282,6 @@ int32_t UserAuthService::CancelAuthOrIdentify(uint64_t contextId)
return FAIL;
}
// try to delete contextId to prevent duplicate cancel success
// it's possible that contextId is deleted before Stop() returns, so delete may fail
if (!ContextPool::Instance().Delete(contextId)) {
IAM_LOGI("failed to delete context");
}
return SUCCESS;
}

View File

@ -21,6 +21,7 @@
#include "context_pool.h"
#include "hdi_wrapper.h"
#include "iam_logger.h"
#include "iam_para2str.h"
#include "ipc_common.h"
#include "resource_node_pool.h"
#include "resource_node_utils.h"
@ -185,6 +186,9 @@ void UserIdmService::AddCredential(std::optional<int32_t> userId, AuthType authT
return;
}
std::lock_guard<std::mutex> lock(mutex_);
CancelCurrentEnrollIfExist();
uint64_t callingUid = static_cast<uint64_t>(this->GetCallingUid());
auto context =
ContextFactory::CreateEnrollContext(userId.value(), authType, pinSubType, token, callingUid, callback);
@ -240,18 +244,43 @@ int32_t UserIdmService::Cancel(std::optional<int32_t> userId, const std::optiona
return CHECK_PERMISSION_FAILED;
}
auto context = ContextPool::Instance().Select(contextId_).lock();
if (context == nullptr || !context->Stop()) {
IAM_LOGE("failed to cancel");
bool userIdIsValid = userId.has_value() && UserIdmSessionController::Instance().IsSessionOpened(userId.value());
bool challengeIsValid = challenge.has_value() &&
UserIdmSessionController::Instance().IsSessionOpened(challenge.value());
if (!userIdIsValid && !challengeIsValid) {
IAM_LOGE("both user id and challenge are invalid");
return FAIL;
}
if (!ContextPool::Instance().Delete(contextId_)) {
IAM_LOGE("failed to delete context");
return FAIL;
std::lock_guard<std::mutex> lock(mutex_);
return CancelCurrentEnroll();
}
void UserIdmService::CancelCurrentEnrollIfExist()
{
if (ContextPool::Instance().Select(CONTEXT_ENROLL).size() == 0) {
return;
}
return SUCCESS;
IAM_LOGI("cancel current enroll due to new add credential request or delete");
CancelCurrentEnroll();
}
int32_t UserIdmService::CancelCurrentEnroll()
{
IAM_LOGD("start");
auto contextList = ContextPool::Instance().Select(CONTEXT_ENROLL);
int32_t ret = FAIL;
for (const auto &context : contextList) {
if (auto ctx = context.lock(); ctx != nullptr) {
IAM_LOGE("stop the old context %{public}s", GET_MASKED_STRING(ctx->GetContextId()).c_str());
ctx->Stop();
ContextPool::Instance().Delete(ctx->GetContextId());
ret = SUCCESS;
}
}
IAM_LOGI("result %{public}d", ret);
return ret;
}
int32_t UserIdmService::EnforceDelUser(int32_t userId, const sptr<IdmCallback> &callback)
@ -261,6 +290,10 @@ int32_t UserIdmService::EnforceDelUser(int32_t userId, const sptr<IdmCallback> &
IAM_LOGE("callback is nullptr");
return INVALID_PARAMETERS;
}
std::lock_guard<std::mutex> lock(mutex_);
CancelCurrentEnrollIfExist();
Attributes extraInfo;
auto userInfo = UserIdmDatabase::Instance().GetSecUserInfo(userId);
@ -297,6 +330,9 @@ void UserIdmService::DelUser(std::optional<int32_t> userId, const std::vector<ui
return;
}
std::lock_guard<std::mutex> lock(mutex_);
CancelCurrentEnrollIfExist();
Attributes extraInfo;
if (!IpcCommon::CheckPermission(*this, MANAGE_USER_IDM_PERMISSION)) {
IAM_LOGE("failed to check permission");
@ -340,6 +376,9 @@ void UserIdmService::DelCredential(std::optional<int32_t> userId, uint64_t crede
return;
}
std::lock_guard<std::mutex> lock(mutex_);
CancelCurrentEnrollIfExist();
Attributes extraInfo;
if (!IpcCommon::CheckPermission(*this, MANAGE_USER_IDM_PERMISSION)) {
IAM_LOGE("failed to check permission");

View File

@ -27,7 +27,6 @@ ohos_fuzztest("UserIdmServiceFuzzTest") {
include_dirs = [
"//base/useriam/user_auth_framework/frameworks/co_auth/inc",
"//base/useriam/user_auth_framework/interfaces/inner_api/co_auth",
"//base/useriam/user_auth_framework/services/base/inc",
"//base/useriam/user_auth_framework/services/core/inc",
"//base/useriam/user_auth_framework/services/core/src",
@ -39,6 +38,7 @@ ohos_fuzztest("UserIdmServiceFuzzTest") {
"//base/useriam/user_auth_framework/common/logs/co_auth/inc",
"//base/useriam/user_auth_framework/common/logs",
"//base/useriam/user_auth_framework/interfaces/inner_api/user_auth/",
"//base/useriam/user_auth_framework/interfaces/inner_api/co_auth",
]
sources = [ "user_idm_service_fuzzer.cpp" ]

View File

@ -768,7 +768,7 @@ HWTEST_F(ExecutorUnitTest, UserAuthExecutor_OnBeginExecute_IdentifyTest_003, Tes
ASSERT_EQ(ret, ResultCode::GENERAL_ERROR);
}
HWTEST_F(ExecutorUnitTest, UserAuthExecutor_OnEndExecute_AUTH, TestSize.Level0)
HWTEST_F(ExecutorUnitTest, UserAuthExecutor_OnEndExecute_Success, TestSize.Level0)
{
static const uint64_t testScheduleId = 456;
@ -786,53 +786,6 @@ HWTEST_F(ExecutorUnitTest, UserAuthExecutor_OnEndExecute_AUTH, TestSize.Level0)
auto commandAttrs = MakeShared<Attributes>();
ASSERT_NE(commandAttrs, nullptr);
commandAttrs->SetUint32Value(Attributes::AttributeKey::ATTR_SCHEDULE_MODE, AUTH);
ret = executorCallback->OnEndExecute(testScheduleId, commandAttrs);
ASSERT_EQ(ret, ResultCode::SUCCESS);
}
HWTEST_F(ExecutorUnitTest, UserAuthExecutor_OnEndExecute_Enroll, TestSize.Level0)
{
static const uint64_t testScheduleId = 456;
shared_ptr<Executor> executor;
shared_ptr<ExecutorCallback> executorCallback;
shared_ptr<MockIAuthExecutorHdi> mockExecutorHdi;
sptr<MockIExecutorMessenger> mockMessenger;
int32_t ret = GetExecutorAndMockStub(executor, executorCallback, mockExecutorHdi, mockMessenger);
ASSERT_EQ(ret, ResultCode::SUCCESS);
EXPECT_CALL(*mockExecutorHdi, Cancel(_)).Times(Exactly(1)).WillOnce([](uint64_t scheduleId) {
EXPECT_EQ(scheduleId, testScheduleId);
return ResultCode::SUCCESS;
});
auto commandAttrs = MakeShared<Attributes>();
ASSERT_NE(commandAttrs, nullptr);
commandAttrs->SetUint32Value(Attributes::AttributeKey::ATTR_SCHEDULE_MODE, ENROLL);
ret = executorCallback->OnEndExecute(testScheduleId, commandAttrs);
ASSERT_EQ(ret, ResultCode::SUCCESS);
}
HWTEST_F(ExecutorUnitTest, UserAuthExecutor_OnEndExecute_Identify, TestSize.Level0)
{
static const uint64_t testScheduleId = 456;
shared_ptr<Executor> executor;
shared_ptr<ExecutorCallback> executorCallback;
shared_ptr<MockIAuthExecutorHdi> mockExecutorHdi;
sptr<MockIExecutorMessenger> mockMessenger;
int32_t ret = GetExecutorAndMockStub(executor, executorCallback, mockExecutorHdi, mockMessenger);
ASSERT_EQ(ret, ResultCode::SUCCESS);
EXPECT_CALL(*mockExecutorHdi, Cancel(_)).Times(Exactly(1)).WillOnce([](uint64_t scheduleId) {
EXPECT_EQ(scheduleId, testScheduleId);
return ResultCode::SUCCESS;
});
auto commandAttrs = MakeShared<Attributes>();
ASSERT_NE(commandAttrs, nullptr);
commandAttrs->SetUint32Value(Attributes::AttributeKey::ATTR_SCHEDULE_MODE, IDENTIFY);
ret = executorCallback->OnEndExecute(testScheduleId, commandAttrs);
ASSERT_EQ(ret, ResultCode::SUCCESS);
}
@ -856,7 +809,6 @@ HWTEST_F(ExecutorUnitTest, UserAuthExecutor_OnEndExecute_ErrorTest_001, TestSize
auto commandAttrs = MakeShared<Attributes>();
ASSERT_NE(commandAttrs, nullptr);
commandAttrs->SetUint32Value(Attributes::AttributeKey::ATTR_SCHEDULE_MODE, IDENTIFY);
ret = executorCallback->OnEndExecute(testScheduleId, commandAttrs);
ASSERT_EQ(ret, ResultCode::GENERAL_ERROR);
}
@ -876,7 +828,6 @@ HWTEST_F(ExecutorUnitTest, UserAuthExecutor_OnEndExecute_ErrorTest_002, TestSize
auto commandAttrs = MakeShared<Attributes>();
ASSERT_NE(commandAttrs, nullptr);
commandAttrs->SetUint32Value(Attributes::AttributeKey::ATTR_SCHEDULE_MODE, IDENTIFY);
// Error: Executor is disconnected
executor->OnHdiDisconnect();
ret = executorCallback->OnEndExecute(testScheduleId, commandAttrs);

View File

@ -18,6 +18,8 @@
#include "mock_authentication.h"
#include "mock_context.h"
#include "mock_schedule_node.h"
#include "schedule_node_impl.h"
#include "mock_resource_node.h"
using namespace OHOS::UserIAM::Common;
using namespace testing;
@ -439,6 +441,44 @@ HWTEST_F(SimpleAuthContextTest, SimpleAuthContextTest_OnScheduleStoped_005, Test
ASSERT_EQ(ret, true);
nodeCallback->OnScheduleStoped(testResultCode, result);
}
HWTEST_F(SimpleAuthContextTest, SimpleAuthContextTest_ContextFree, TestSize.Level1)
{
static const uint64_t testContestId = 2;
std::shared_ptr<MockAuthentication> mockAuth = MakeShared<MockAuthentication>();
ASSERT_NE(mockAuth, nullptr);
EXPECT_CALL(*mockAuth, Start(_, _))
.Times(Exactly(1))
.WillOnce([](std::vector<std::shared_ptr<ScheduleNode>> &scheduleList,
std::shared_ptr<ScheduleNodeCallback> callback) {
EXPECT_EQ(scheduleList.size(), 0U);
auto faceAllInOne = MockResourceNode::CreateWithExecuteIndex(1, FACE, ALL_IN_ONE);
auto builder = ScheduleNode::Builder::New(faceAllInOne, faceAllInOne);
builder->SetScheduleCallback(callback);
auto scheduleNode = builder->Build();
EXPECT_NE(scheduleNode, nullptr);
scheduleList.push_back(scheduleNode);
return true;
});
std::shared_ptr<MockContextCallback> contextCallback = MakeShared<MockContextCallback>();
ASSERT_NE(contextCallback, nullptr);
EXPECT_CALL(*contextCallback, OnResult(_, _)).Times(Exactly(1))
.WillOnce([](int32_t resultCode, const std::shared_ptr<Attributes> &finalResult) {
EXPECT_EQ(resultCode, ResultCode::BUSY);
});
std::weak_ptr<Context> weakContext;
{
std::shared_ptr<Context> context =
MakeShared<SimpleAuthContext>(testContestId, mockAuth, contextCallback);
ASSERT_NE(context, nullptr);
weakContext = context;
context->Start();
usleep(1000);
}
ASSERT_EQ(weakContext.lock(), nullptr);
}
} // namespace UserAuth
} // namespace UserIam
} // namespace OHOS