Skip to content

Commit

Permalink
Improve the logic around schedule application in LockEndpoint. (proje…
Browse files Browse the repository at this point in the history 
…ct-chip#31946)

The old logic had several bugs:

* For kYearDayScheduleUser users, there was no schedule enforcement at all,
  since the outer "if" never tested true.
* For kWeekDayScheduleUser users, there was no schedule enforcement at all,
  since the inner "if" never tested true.
* For kScheduleRestrictedUser users, access was allowed if there was any
  schedule, weekday or yearday, that granted access.  But the intent is that
  access should be disallowed if there are year day schedules and none of them
  grants access, no matter what's going on with weekday.  And vice versa.
  • Loading branch information
@bzbarsky-apple @raul-marquez-csa
bzbarsky-apple authored and raul-marquez-csa committed Feb 16, 2024
1 parent d3e895f commit ee226ca
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 27 deletions.
12 changes: 10 additions & 2 deletions examples/lock-app/lock-common/include/LockEndpoint.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,16 @@ class LockEndpoint
OperationSourceEnum opSource = OperationSourceEnum::kUnspecified);
const char * lockStateToString(DlLockState lockState) const;

bool weekDayScheduleInAction(uint16_t userIndex) const;
bool yearDayScheduleInAction(uint16_t userIndex) const;
// Returns true if week day schedules should apply to the user, there are
// schedules defined for the user, and access is not currently allowed by
// those schedules. The outparam indicates whether there were in fact any
// year day schedules defined for the user.
bool weekDayScheduleForbidsAccess(uint16_t userIndex, bool * haveSchedule) const;
// Returns true if year day schedules should apply to the user, there are
// schedules defined for the user, and access is not currently allowed by
// those schedules. The outparam indicates whether there were in fact any
// year day schedules defined for the user.
bool yearDayScheduleForbidsAccess(uint16_t userIndex, bool * haveSchedule) const;

static void OnLockActionCompleteCallback(chip::System::Layer *, void * callbackContext);

Expand Down
73 changes: 48 additions & 25 deletions examples/lock-app/lock-common/src/LockEndpoint.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,19 +488,20 @@ bool LockEndpoint::setLockState(const Nullable<chip::FabricIndex> & fabricIdx, c
auto userIndex = static_cast<uint8_t>(user - mLockUsers.begin());

// Check if schedules affect the user
if ((user->userType == UserTypeEnum::kScheduleRestrictedUser || user->userType == UserTypeEnum::kWeekDayScheduleUser) &&
!weekDayScheduleInAction(userIndex))
bool haveWeekDaySchedules = false;
bool haveYearDaySchedules = false;
if (weekDayScheduleForbidsAccess(userIndex, &haveWeekDaySchedules) ||
yearDayScheduleForbidsAccess(userIndex, &haveYearDaySchedules) ||
// Also disallow access for a user that's supposed to have _some_
// schedule but doesn't have any
(user->userType == UserTypeEnum::kScheduleRestrictedUser && !haveWeekDaySchedules && !haveYearDaySchedules))
{
if ((user->userType == UserTypeEnum::kScheduleRestrictedUser || user->userType == UserTypeEnum::kYearDayScheduleUser) &&
!yearDayScheduleInAction(userIndex))
{
ChipLogDetail(Zcl,
"Lock App: associated user is not allowed to operate the lock due to schedules"
"[endpointId=%d,userIndex=%u]",
mEndpointId, userIndex);
err = OperationErrorEnum::kRestricted;
return false;
}
ChipLogDetail(Zcl,
"Lock App: associated user is not allowed to operate the lock due to schedules"
"[endpointId=%d,userIndex=%u]",
mEndpointId, userIndex);
err = OperationErrorEnum::kRestricted;
return false;
}
ChipLogProgress(
Zcl,
Expand Down Expand Up @@ -561,12 +562,23 @@ void LockEndpoint::OnLockActionCompleteCallback(chip::System::Layer *, void * ca
}
}

bool LockEndpoint::weekDayScheduleInAction(uint16_t userIndex) const
bool LockEndpoint::weekDayScheduleForbidsAccess(uint16_t userIndex, bool * haveSchedule) const
{
*haveSchedule = std::any_of(mWeekDaySchedules[userIndex].begin(), mWeekDaySchedules[userIndex].end(),
[](const WeekDaysScheduleInfo & s) { return s.status == DlScheduleStatus::kOccupied; });

const auto & user = mLockUsers[userIndex];
if (user.userType != UserTypeEnum::kScheduleRestrictedUser && user.userType != UserTypeEnum::kWeekDayScheduleUser)
{
return true;
// Weekday schedules don't apply to this user.
return false;
}

if (user.userType == UserTypeEnum::kScheduleRestrictedUser && !*haveSchedule)
{
// It's valid to not have any schedules of a given type; on its own this
// does not prevent access.
return false;
}

chip::System::Clock::Milliseconds64 cTMs;
Expand All @@ -575,7 +587,7 @@ bool LockEndpoint::weekDayScheduleInAction(uint16_t userIndex) const
{
ChipLogError(Zcl, "Lock App: unable to get current time to check user schedules [endpointId=%d,error=%d (%s)]", mEndpointId,
chipError.AsInteger(), chipError.AsString());
return false;
return true;
}
time_t unixEpoch = std::chrono::duration_cast<chip::System::Clock::Seconds32>(cTMs).count();

Expand All @@ -585,8 +597,9 @@ bool LockEndpoint::weekDayScheduleInAction(uint16_t userIndex) const
auto currentTime =
calendarTime.tm_hour * chip::kSecondsPerHour + calendarTime.tm_min * chip::kSecondsPerMinute + calendarTime.tm_sec;

// Second, check the week day schedules.
return std::any_of(
// Now check whether any schedule allows the current time. If it does,
// access is not forbidden.
return !std::any_of(
mWeekDaySchedules[userIndex].begin(), mWeekDaySchedules[userIndex].end(),
[currentTime, calendarTime](const WeekDaysScheduleInfo & s) {
auto startTime = s.schedule.startHour * chip::kSecondsPerHour + s.schedule.startMinute * chip::kSecondsPerMinute;
Expand All @@ -596,12 +609,22 @@ bool LockEndpoint::weekDayScheduleInAction(uint16_t userIndex) const
});
}

bool LockEndpoint::yearDayScheduleInAction(uint16_t userIndex) const
bool LockEndpoint::yearDayScheduleForbidsAccess(uint16_t userIndex, bool * haveSchedule) const
{
*haveSchedule = std::any_of(mYearDaySchedules[userIndex].begin(), mYearDaySchedules[userIndex].end(),
[](const YearDayScheduleInfo & sch) { return sch.status == DlScheduleStatus::kOccupied; });

const auto & user = mLockUsers[userIndex];
if (user.userType != UserTypeEnum::kScheduleRestrictedUser && user.userType != UserTypeEnum::kYearDayScheduleUser)
{
return true;
return false;
}

if (user.userType == UserTypeEnum::kScheduleRestrictedUser && !*haveSchedule)
{
// It's valid to not have any schedules of a given type; on its own this
// does not prevent access.
return false;
}

chip::System::Clock::Milliseconds64 cTMs;
Expand All @@ -610,7 +633,7 @@ bool LockEndpoint::yearDayScheduleInAction(uint16_t userIndex) const
{
ChipLogError(Zcl, "Lock App: unable to get current time to check user schedules [endpointId=%d,error=%d (%s)]", mEndpointId,
chipError.AsInteger(), chipError.AsString());
return false;
return true;
}
auto unixEpoch = std::chrono::duration_cast<chip::System::Clock::Seconds32>(cTMs).count();
uint32_t chipEpoch = 0;
Expand All @@ -623,11 +646,11 @@ bool LockEndpoint::yearDayScheduleInAction(uint16_t userIndex) const
return false;
}

return std::any_of(mYearDaySchedules[userIndex].begin(), mYearDaySchedules[userIndex].end(),
[chipEpoch](const YearDayScheduleInfo & sch) {
return sch.status == DlScheduleStatus::kOccupied && sch.schedule.localStartTime <= chipEpoch &&
chipEpoch <= sch.schedule.localEndTime;
});
return !std::any_of(mYearDaySchedules[userIndex].begin(), mYearDaySchedules[userIndex].end(),
[chipEpoch](const YearDayScheduleInfo & sch) {
return sch.status == DlScheduleStatus::kOccupied && sch.schedule.localStartTime <= chipEpoch &&
chipEpoch <= sch.schedule.localEndTime;
});
}

const char * LockEndpoint::lockStateToString(DlLockState lockState) const
Expand Down

0 comments on commit ee226ca

Please sign in to comment.