Skip to content

zigbee-lock: centralize credential/user table updates and tighten addCredential/updateCredential validation #3331

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hcarter-775 merged 4 commits into from
Nov 11, 2026
Merged

zigbee-lock: centralize credential/user table updates and tighten addCredential/updateCredential validation #3331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4192673
zigbee-lock: simplify reported and commanded credential management
hcarter-775 Nov 9, 2026
4bc3602
remove unnecessary user_index == 0 check
hcarter-775 Nov 9, 2026
af2903c
limit one credential per user on add
hcarter-775 Nov 10, 2026
1cbcfe5
fix comment
hcarter-775 Nov 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 24 additions & 5 deletions drivers/SmartThings/zigbee-lock/src/lock_handlers/capabilities.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,15 +127,27 @@ function CapabilityHandlers.add_credential(driver, device, command)
return
end

-- A userIndex of 0 means "auto-assign the next available slot"
local user_index = command.args.userIndex == 0 and tables.next_index(device, "users") or command.args.userIndex
local cred_index = command.args.userIndex == 0 and tables.next_index(device, "credentials") or command.args.userIndex
-- userIndex 0 means "create a new user for this credential" (its index is chosen when the lock confirms the
-- code). Any other userIndex must name an existing user that does not already have a credential (for now,
-- each user may have at most one credential).
local user_index = command.args.userIndex
if user_index ~= 0 and (not tables.find_entry(device, "users", user_index)
or tables.find_entry_by(device, "credentials", "userIndex", user_index)) then
lock_utils.emit_command_result(device, capabilities.lockCredentials, consts.LOCK_CREDENTIALS.ADD, consts.COMMAND_RESULT.INVALID_COMMAND)
return
end

if #(tables.get_state(device, "credentials") or {}) >= tables.get_max_entries(device, "credentials") then
local credentials_full = #(tables.get_state(device, "credentials") or {}) >= tables.get_max_entries(device, "credentials")
-- if we are being asked to create a new user (index 0), this also needs a free user index.
local users_full = (user_index == 0) and #(tables.get_state(device, "users") or {}) >= tables.get_max_entries(device, "users")
if credentials_full or users_full then

@ctowns ctowns Nov 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is inconsistent with our plan to allow credentials to be created even if the users table is full. I don't think it's a blocker, but it looks like we have two different policies for handling this case dependent on whether the add originated from a keypad or from the capability command. Worth revisiting in the follow up ticket, but I wouldn't consider this a blocker here.

@hcarter-775 hcarter-775 Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is closer to the expectation, where we don't allow a cred to be made if it can't have an associated user. I think the other case is the "problem", since we can't know that ahead of time, but yes, they both speak to the deeper problem at hand.

lock_utils.emit_command_result(device, capabilities.lockCredentials, consts.LOCK_CREDENTIALS.ADD, consts.COMMAND_RESULT.RESOURCE_EXHAUSTED)
return
end

-- the credential always takes the next available slot.
local cred_index = tables.next_index(device, "credentials", 1)

-- Set busy state and attempt to set the PIN on-device.
lock_utils.set_busy_state(device, consts.LOCK_CREDENTIALS.ADD, {
userIndex = user_index,
Expand All @@ -157,11 +169,18 @@ function CapabilityHandlers.update_credential(driver, device, command)
return
end

if not tables.find_entry(device, "credentials", command.args.credentialIndex) then
local existing = tables.find_entry(device, "credentials", command.args.credentialIndex)
if not existing then
lock_utils.emit_command_result(device, capabilities.lockCredentials, consts.LOCK_CREDENTIALS.UPDATE, consts.COMMAND_RESULT.FAILURE)
return
end

-- a credential cannot be moved to another user: the given userIndex must already own this credential
if existing.userIndex ~= command.args.userIndex then
lock_utils.emit_command_result(device, capabilities.lockCredentials, consts.LOCK_CREDENTIALS.UPDATE, consts.COMMAND_RESULT.INVALID_COMMAND)
return
end

lock_utils.set_busy_state(device, consts.LOCK_CREDENTIALS.UPDATE, {
userIndex = command.args.userIndex,
credentialIndex = command.args.credentialIndex,
Expand Down
80 changes: 12 additions & 68 deletions drivers/SmartThings/zigbee-lock/src/lock_handlers/zigbee_responses.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,9 @@ function ZigbeeHandlers.set_pin_code_response(driver, device, zb_rx)
}

-- apply result based on response and identify command result status
local result_status
local result_status, user_index
if set_pin_code_status == SetCodeStatus.SUCCESS then
if command_in_progress == consts.LOCK_CREDENTIALS.ADD then
tables.add_entry(device, "users", {
userIndex = credential_args_in_use.userIndex,
userName = "Guest " .. credential_args_in_use.userIndex, -- default
userType = "guest", -- default
})
result_status = tables.add_entry(device, "credentials", {
userIndex = credential_args_in_use.userIndex,
credentialIndex = credential_args_in_use.credentialIndex,
credentialType = credential_args_in_use.credentialType,
credentialName = "Guest " .. credential_args_in_use.userIndex, -- default
})
elseif command_in_progress == consts.LOCK_CREDENTIALS.UPDATE then
result_status = consts.COMMAND_RESULT.SUCCESS
end
result_status, user_index = lock_utils.apply_commanded_credential(device, command_in_progress, credential_args_in_use)
elseif RESPONSE_RESULT_MAP[set_pin_code_status] then
result_status = RESPONSE_RESULT_MAP[set_pin_code_status]
else
Expand All @@ -62,7 +48,7 @@ function ZigbeeHandlers.set_pin_code_response(driver, device, zb_rx)
-- emit command result
if command_in_progress then
local additional_info = result_status == consts.COMMAND_RESULT.SUCCESS and {
userIndex = credential_args_in_use.userIndex,
userIndex = user_index,
credentialIndex = credential_args_in_use.credentialIndex,
} or nil
lock_utils.emit_command_result(device, capabilities.lockCredentials, command_in_progress, result_status, additional_info)
Expand Down Expand Up @@ -157,18 +143,7 @@ function ZigbeeHandlers.get_pin_code_response(driver, device, zb_rx)
-- Only add entries for slots that are actually occupied on the lock.
local user_status = zb_rx.body.zcl_body.user_status.value
if user_status == clusters.DoorLock.types.DrlkUserStatus.OCCUPIED_ENABLED then
-- If an entry already exists at this index, this will be a no-op.
tables.add_entry(device, "users", {
userIndex = user_id,
userName = "Guest " .. user_id,
userType = "guest",
})
tables.add_entry(device, "credentials", {
userIndex = user_id,
credentialIndex = user_id,
credentialType = consts.CRED_TYPE_PIN,
credentialName = "Guest " .. user_id,
})
lock_utils.add_reported_credential(device, user_id)
-- reset the consecutive unoccupied codes counter since we found an occupied code
device:set_field(consts.SYNC.CONSECUTIVE_UNOCCUPIED_CODES, 0)
else
Expand Down Expand Up @@ -223,23 +198,9 @@ function ZigbeeHandlers.programming_event_notification(driver, device, zb_rx)
local credential_args_in_use = device:get_field(consts.DRIVER_STATE.CREDENTIAL_ARGS_IN_USE) or {}
if command_in_progress and credential_args_in_use.credentialIndex == user_id then
local result_status
if event_code == ProgramEventCode.PIN_CODE_ADDED then
if command_in_progress == consts.LOCK_CREDENTIALS.ADD then
tables.add_entry(device, "users", {
userIndex = credential_args_in_use.userIndex,
userName = "Guest " .. credential_args_in_use.userIndex, -- default
userType = "guest", -- default
})
result_status = tables.add_entry(device, "credentials", {
userIndex = credential_args_in_use.userIndex,
credentialIndex = credential_args_in_use.credentialIndex,
credentialType = credential_args_in_use.credentialType,
credentialName = credential_args_in_use.credentialName, -- optional
})
elseif command_in_progress == consts.LOCK_CREDENTIALS.UPDATE then
-- some devices emit PIN_CODE_ADDED even for UPDATE events
result_status = consts.COMMAND_RESULT.SUCCESS
end
local user_index = credential_args_in_use.userIndex
if event_code == ProgramEventCode.PIN_CODE_ADDED then -- note: some devices emit PIN_CODE_ADDED even for UPDATE events
result_status, user_index = lock_utils.apply_commanded_credential(device, command_in_progress, credential_args_in_use)
elseif event_code == ProgramEventCode.PIN_CODE_DELETED and (command_in_progress == consts.LOCK_CREDENTIALS.DELETE or command_in_progress == consts.LOCK_USERS.DELETE) then
result_status = tables.delete_entry(device, "credentials", credential_args_in_use.credentialIndex)
-- the deleteUser command injects a deleteCredential command, so ensure the lockUsers command results is emitted in this case.
Expand All @@ -252,42 +213,25 @@ function ZigbeeHandlers.programming_event_notification(driver, device, zb_rx)
end
end
elseif event_code == ProgramEventCode.PIN_CODE_CHANGED and command_in_progress == consts.LOCK_CREDENTIALS.UPDATE then
result_status = consts.COMMAND_RESULT.SUCCESS
result_status, user_index = lock_utils.apply_commanded_credential(device, command_in_progress, credential_args_in_use)
end

if result_status then
lock_utils.emit_command_result(device,
capabilities.lockCredentials,
command_in_progress,
result_status,
{ userIndex = credential_args_in_use.userIndex, credentialIndex = credential_args_in_use.credentialIndex }
{ userIndex = user_index, credentialIndex = credential_args_in_use.credentialIndex }
)
lock_utils.clear_busy_state(device)
return
end
end

-- handle the case where we receive a programming event notification for a code we've just deleted,
-- handle the case where we receive a programming event notification for a code we didn't send a command for
if event_code == ProgramEventCode.PIN_CODE_ADDED then
-- if no "addCredential" command is in progress, check if the credential is already stored locally.
local credential = tables.find_entry(device, "credentials", user_id)
if not credential then
-- check what user slot we have to associate this with and add a default user
local next_index = tables.next_index(device, "users")
if next_index and next_index <= tables.get_max_entries(device, "users") then
Comment thread
ctowns marked this conversation as resolved.
Resolved
tables.add_entry(device, "users", {
userIndex = next_index,
userName = "Guest " .. next_index,
userType = "guest",
})
tables.add_entry(device, "credentials", {
userIndex = next_index,
credentialIndex = user_id,
credentialType = consts.CRED_TYPE_PIN,
credentialName = "Guest " .. next_index,
})
end
end
-- if no "addCredential" command is in progress, ensure the credential is represented locally
lock_utils.add_reported_credential(device, user_id)
elseif event_code == ProgramEventCode.PIN_CODE_DELETED then
-- check if a credential exists locally for this user id, then
-- try to delete the entries in our tables corresponding to this code.
Expand Down
9 changes: 5 additions & 4 deletions drivers/SmartThings/zigbee-lock/src/lock_utils/tables.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,16 @@ function table_utils.find_all_entries_by(device, table_name, key, value)
return matches
end

-- Return the lowest positive integer not yet used as the match_key in the named table.
-- Used to auto-assign the next available slot for a new entry.
function table_utils.next_index(device, table_name)
-- Return the lowest integer, starting at the table's minimum, not yet used as the
-- match_key in the named table. Used to auto-assign the next available slot for a new entry.
-- An optional `start` overrides the table's minimum, e.g. to skip a reserved slot.
function table_utils.next_index(device, table_name, start)
local def = resolve_table_def(device, table_name)
if not def then return 1 end
local t = table_utils.get_state(device, def) or {}
local occupied = {}
for _, entry in ipairs(t) do occupied[entry[def.match_key]] = true end
local idx = 1
local idx = start or def.minimum
while occupied[idx] do idx = idx + 1 end
return idx
end
Expand Down
72 changes: 72 additions & 0 deletions drivers/SmartThings/zigbee-lock/src/lock_utils/utils.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
-- Copyright 2026 SmartThings, Inc.
-- Licensed under the Apache License, Version 2.0

local tables = require "lock_utils.tables"
local consts = require "lock_utils.constants"

local lock_utils = {}
Expand Down Expand Up @@ -52,6 +53,77 @@ function lock_utils.sync_device_state(device)
end


-- [[ CREDENTIAL TABLE MANAGEMENT ]] --

-- Add a credential and, optionally, a new user for it. `args.userIndex` is the credential's user.
-- When `add_user` is true the user is created too, so a user never exists without a credential; when false
-- the credential is attached to the already-existing user `args.userIndex`.
-- The credential is added first; the user is only added if that succeeded, so a failed or
-- colliding credential add leaves no user behind. Returns the result status of the credential add.
function lock_utils.add_credential_and_user(device, args, add_user)
local name = "Guest " .. args.userIndex
local credential_status = tables.add_entry(device, "credentials", {
userIndex = args.userIndex,
credentialIndex = args.credentialIndex,
credentialType = args.credentialType or consts.CRED_TYPE_PIN,
credentialName = args.credentialName or name,

@tpmanley tpmanley Nov 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm not mistaken using credentialName is a change in behavior that isn't called out in the description. I haven't checked - is this how we handle it in the other lock drivers as well?

@hcarter-775 hcarter-775 Nov 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, but we should. It is an optional field that is currently basically not utilized. If it is used, it can effectively pass a userName to a newly created user without explicitly calling update. Description updated.

})
if add_user and credential_status == consts.COMMAND_RESULT.SUCCESS then
tables.add_entry(device, "users", {
Comment thread
tpmanley marked this conversation as resolved.
Resolved
userIndex = args.userIndex,
userName = args.credentialName or name,
userType = "guest",
})
end
return credential_status
end

-- Apply the arguments of an updateCredential command to the credentials table.
-- Returns the command result status and the credential's userIndex.
function lock_utils.update_commanded_credential(device, credential_args)
local existing = tables.find_entry(device, "credentials", credential_args.credentialIndex)
if not existing then return consts.COMMAND_RESULT.FAILURE end
local name = credential_args.credentialName
if name ~= nil and name ~= existing.credentialName then
local status = tables.update_entry(device, "credentials", credential_args.credentialIndex, { credentialName = name })
if status ~= consts.COMMAND_RESULT.SUCCESS then return status end
end
return consts.COMMAND_RESULT.SUCCESS, existing.userIndex
end

-- Per command: update the tables after the lock confirmed a set-credential command we sent.
-- Returns the command result status, or nil for any other command, plus the affected userIndex.
-- For addCredential, a userIndex of 0 creates a new user (its index is chosen here); otherwise the credential
-- is attached to the existing user with that index.
function lock_utils.apply_commanded_credential(device, command_in_progress, credential_args)
if command_in_progress == consts.LOCK_CREDENTIALS.ADD then
local user_index = credential_args.userIndex
if (user_index == 0) then user_index = tables.next_index(device, "users") end
local user_not_found = not tables.find_entry(device, "users", user_index)
local status = lock_utils.add_credential_and_user(device, {
userIndex = user_index,
credentialIndex = credential_args.credentialIndex,
credentialType = credential_args.credentialType,
credentialName = credential_args.credentialName,
}, user_not_found)
return status, status == consts.COMMAND_RESULT.SUCCESS and user_index or nil
elseif command_in_progress == consts.LOCK_CREDENTIALS.UPDATE then
return lock_utils.update_commanded_credential(device, credential_args)
end
end

-- Per report: ensure a user and PIN credential exist for a code the lock reports at `credential_index`
-- without any command from us (sync, or a programming event). If a credential already exists there it
-- is left in place
function lock_utils.add_reported_credential(device, credential_index)
if tables.find_entry(device, "credentials", credential_index) then return end
lock_utils.add_credential_and_user(device, {
userIndex = tables.next_index(device, "users"),
credentialIndex = credential_index,
}, true)
end


-- [[ COMMAND RESULT STATE MANAGEMENT ]] --

function lock_utils.emit_command_result(device, capability, command_name, status_code, additional_info)
Expand Down
Loading
Loading