Skip to content

Add NSF support for PAM config/rotation and KSM; drop duplicate rotation sync#212

Open
mtyagi-ks wants to merge 9 commits into
pam-nsf-supportfrom
pam-nsf-support-int
Open

Add NSF support for PAM config/rotation and KSM; drop duplicate rotation sync#212
mtyagi-ks wants to merge 9 commits into
pam-nsf-supportfrom
pam-nsf-support-int

Conversation

@mtyagi-ks

Copy link
Copy Markdown
Contributor

Summary

  • Extend PAM and KSM flows to resolve/use Nested Shared Folder (Keeper Drive) records for list/config and rotation setup.
  • Ingest classic + NSF record rotation data from the normal vault sync into PAM sqlite, and remove the extra dedicated sync_down used only for rotations.
  • Update protobufs and PAM config new/edit paths needed for NSF-aware PAM configuration.

Test plan

  • pam config list / pam config new / pam config edit work for NSF PAM configs
  • pam rotation list / info resolve NSF pamUser, config, and resource titles/UIDs
  • pam rotation edit against NSF pamUser (confirm revision behavior after sync)
  • Confirm no separate vault sync is triggered only to refresh record rotations
  • KSM list/share paths still work for NSF folders/records
  • Existing classic (non-NSF) PAM/KSM flows unchanged

return None
folder_uid = folder_uids[0]
if folder_uid == nsf_management.ROOT_FOLDER_UID:
return _PamFolderRef('My Drive', folder_uid)

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.

change it to 'My Vault'

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.

addressed

record_uid = nsf_management.resolve_nsf_record_uid(vault, identifier)
except nsf_management.NsfError:
record_uid = None
if not record_uid:

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.

move this check before checking for records, move it to line 242

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.

identifier is already validated above, so moving if not record_uid before get_record would be dead code. The post-resolve None check is the one that matters and should stay after resolve

return None

def _find_nsf_pam_configurations(self, vault: vault_online.VaultOnline):
"""Yield TypedRecord PAM configs stored under NSF / Keeper Drive."""

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.

remove drive mention from anywhere in the code

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.

addressed

common_parser.add_argument('--shared-folder', '-sf', dest='shared_folder_uid', action='store',
help='Share Folder where this PAM Configuration is stored. Should be one of the folders to '
'which the gateway has access to.')
help='Classic shared folder or NSF / Keeper Drive folder (UID or name) where this PAM '

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.

remove keeper drive mention

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.

addressed

record_uid = nsf_management.resolve_nsf_record_uid(vault, identifier)
except nsf_management.NsfError:
record_uid = None
if not record_uid:

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.

move record uid check to line 1200

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.

identifier is already validated above, so moving if not record_uid before get_record would be dead code. The post-resolve None check is the one that matters and should stay after resolve

resource_id = utils.base64_url_encode(rri.resourceUid)
resource_ok = is_resource_ok(resource_id, vault, configuration_uid)
logger.info(f"Admin Resource Uid: {resource_id if resource_ok else 'FAIL'}")
# Always print the router resource UID (Commander parity). Do not

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.

remove this comment

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.

addressed

if not self._auth or not self._auth.auth_context.is_enterprise_admin:
return
if self._enterprise_loader is None:
"""Copy rotations from the last vault sync into :attr:`pam_plugin` sqlite (no extra sync_down)."""

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.

remove this comment

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.

addressed

self._record_rotations.put_entities(_pam_rotation_to_domain(r) for r in row_list)

def sync_down(self, *, reload: bool = False) -> None:
"""Refresh controllers from PAM API; reload rotations from local sqlite only."""

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.

remove this comment

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.

addressed

if not uids_to_update:
classic_uids_to_readd.append(uid)

if classic_uids_to_readd:

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.

correct typo from definition till here

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.

addressed

def consume_rotations_cleared(self) -> bool:
"""Return and clear the 'last sync cleared rotations' flag for PAM sqlite replace."""
cleared = self._pending_rotation_replace
self._pending_rotation_replace = False

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 clears the cache and rebuild from sqlite?

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.

this only returns and clears the _pending_rotation_replace flag (one-shot). The in-memory rotation cache is cleared during sync_down / _ingest_record_rotations. PAM sqlite replace vs merge is decided later by the CLI using this flag as replace_all.

*,
is_editable: bool = False,
request_sync: bool = True) -> Dict[str, Any]:
"""Share an NSF (Keeper Drive) folder with a KSM application via AT_APPLICATION.

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.

remove keeper drive

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.

addressed

request_sync: bool = True) -> Dict[str, Any]:
"""Share an NSF (Keeper Drive) folder with a KSM application via AT_APPLICATION.

Classic vault/app_share_add rejects Drive folders; this uses folders/v3/access_update.

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.

remove this comment

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.

addressed

Comment thread keepercli-package/src/keepercli/commands/pam/pam_config.py
return sf_uid


if vault.nsf_data is None:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

add log message No nsf data

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.

addressed

Comment thread keepercli-package/src/keepercli/commands/pam/pam_config.py Outdated
Comment thread keepercli-package/src/keepercli/commands/pam/pam_config.py
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.

3 participants