Skip to content

KC-1315, KC-1329: Fix share-folder record expiration and ROE handling#2168

Merged
amangalampalli-ks merged 9 commits into
share-folder-roe-fixfrom
share-folder-roe-fix-int
Jul 9, 2026
Merged

KC-1315, KC-1329: Fix share-folder record expiration and ROE handling#2168
amangalampalli-ks merged 9 commits into
share-folder-roe-fixfrom
share-folder-roe-fix-int

Conversation

@sshrushanth-ks

@sshrushanth-ks sshrushanth-ks commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes share-folder behavior for KC-1315 and KC-1329. Folder access (-e, -p, -o, --expire-in/at, -roe) and record permissions (-r, -d, -s) are now independent flag groups. Expiration applies only to folder user/team access, not per-record timers. Remove with -e revokes folder access only and no longer removes records from the folder.

Changes

KC-1315 — Expiration

  • --expire-in / --expire-at apply only to folder user/team protos (SharedFolderUpdateUser / SharedFolderUpdateTeam), not SharedFolderUpdateRecord
  • Record protos from -r update can_edit / can_share only (no expiration or ROE on records)
  • Fixed short expiration validation (e.g. 1mi) using millisecond-precision timestamps
  • Sync-down before share updates when expiration is set

KC-1329 — Remove semantics

  • -a remove with -e revokes folder access only; does not call sharedFolderRemoveRecord
  • -r / -d / -s can be combined with remove to update record permissions in the same request
  • Owner/participant self-removal prompts and validation (backup manager required for owner self-remove)

Validation & safety

  • -d / -s require a record target (-r, -r *, or -r @existing)
  • -r with a specific record UID/name must already be in the folder; rejects foreign records instead of linking them via sharedFolderAddRecord
  • Case-insensitive folder user lookup for -e

Reliability

  • Explicit -p / -o always update folder user permissions even when -r is present
  • Skip redundant folder user updates only when sharing another record with unchanged permissions and no expiration
  • Include API status in user-share failure logs

Help & tests

  • Reorganized share-folder help into folder access vs record permissions groups
  • Document that -o is mutually exclusive with --expire-at / --expire-in (matches Vault behavior)
  • Expanded unit tests in test_command_register.py

Flag model

Area Flags Purpose
Folder access -a, -e, -p, -o, --expire-in/at, -roe Grant/remove user or team folder access
Record permissions -r, -d, -s Update can edit / can share for records already in the folder

-p/-o and -d/-s are independent within their groups.

@datadog-keeper-security

This comment has been minimized.

@sshrushanth-ks sshrushanth-ks marked this pull request as ready for review June 24, 2026 10:48
@sshrushanth-ks sshrushanth-ks self-assigned this Jun 24, 2026
Comment thread keepercommander/commands/register.py Outdated
@sshrushanth-ks sshrushanth-ks changed the title KC-1315: Fix share-folder record expiration and ROE handling KC-1315, KC-1329: Fix share-folder record expiration and ROE handling Jun 28, 2026
@amangalampalli-ks amangalampalli-ks force-pushed the share-folder-roe-fix-int branch 2 times, most recently from dcd973e to 4820349 Compare July 3, 2026 05:20
@amangalampalli-ks amangalampalli-ks force-pushed the share-folder-roe-fix-int branch from ae1f3ed to 8d8fcf6 Compare July 6, 2026 12:49
@amangalampalli-ks amangalampalli-ks changed the base branch from share-folder-roe-fix to release July 6, 2026 13:44
@amangalampalli-ks amangalampalli-ks changed the base branch from release to share-folder-roe-fix July 6, 2026 13:44
@amangalampalli-ks amangalampalli-ks self-assigned this Jul 6, 2026
@amangalampalli-ks amangalampalli-ks requested review from sali-ks and removed request for amangalampalli-ks July 6, 2026 13:54
sshrushanth-ks and others added 8 commits July 9, 2026 19:40
…g re-shares

When share-folder was used with -r and --expire-in, expiration was applied to
SharedFolderUpdateRecord, which caused the record to be removed from the
owner's vault when the timer expired. Route per-record expiration and -roe
through the record share API (revoke then re-grant) instead, keep folder user
updates for access only, sync before granting, and skip redundant user updates
when sharing additional records to the same recipient.
@amangalampalli-ks amangalampalli-ks force-pushed the share-folder-roe-fix-int branch from 7e69ec0 to 0663292 Compare July 9, 2026 14:11
@amangalampalli-ks amangalampalli-ks merged commit bddbb3d into share-folder-roe-fix Jul 9, 2026
4 checks passed
sk-keeper pushed a commit that referenced this pull request Jul 9, 2026
…#2168)

* Fix share-folder record expiration removing owner records and breaking re-shares

When share-folder was used with -r and --expire-in, expiration was applied to
SharedFolderUpdateRecord, which caused the record to be removed from the
owner's vault when the timer expired. Route per-record expiration and -roe
through the record share API (revoke then re-grant) instead, keep folder user
updates for access only, sync before granting, and skip redundant user updates
when sharing additional records to the same recipient.

* Share-folder: expire folder and record access together; log expiry in output

* updated test file

* Fix share-folder remove vault deletion; clean up access grant/remove logs

* Fix share folder expire in and -r combination

* Separate folder and record flag usage to fix multiple remove related issue

* Fix -p and -o flags and 1mi expiry

* Restrict outside records to be shared via -r

* Update help

---------

Co-authored-by: amangalampalli-ks <amangalampalli@keepersecurity.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants