Skip to content

Improved connect-pnponline cert auth to work with persist login and also added cmdlet to list persist login cmdlets - #5463

Open
gautamdsheth wants to merge 6 commits into
devfrom
feat/persistlogin-apponly
Open

Improved connect-pnponline cert auth to work with persist login and also added cmdlet to list persist login cmdlets#5463
gautamdsheth wants to merge 6 commits into
devfrom
feat/persistlogin-apponly

Conversation

@gautamdsheth

Copy link
Copy Markdown
Collaborator

Type

  • Bug Fix
  • New Feature
  • Sample

Related Issues?

Fixes NA

What is in this Pull Request ?

Added persist login support for cert auth
Added cmdlet to list persist login
Added verbose message for creds

…lso added cmdlet to list persist login cmdlets
Copilot AI balanced review requested due to automatic review settings September 5, 2026 15:18

Copilot AI left a comment

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.

🟡 Changes recommended

Environment-variable credential persistence currently crashes, and failed secure cache initialization leaves a persistent unusable registration.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds persisted certificate-based authentication, cache inspection, and credential diagnostics.

Changes:

  • Supports app-only certificate token caching.
  • Adds Get-PnPPersistedLogin.
  • Updates authentication documentation and verbose credential reporting.
File summaries
File Description
src/Commands/Model/TokenCacheConfiguration.cs Adds authentication type metadata.
src/Commands/Base/PnPConnection.cs Implements app-only cache lifecycle.
src/Commands/Base/GetPersistedLogin.cs Adds cache-listing cmdlet.
src/Commands/Base/ConnectOnline.cs Enables certificate persistence and credential diagnostics.
pages/articles/persistedlogin.md Documents persisted authentication.
pages/articles/credentialmanagement.md Documents credential diagnostics.
documentation/Get-PnPPersistedLogin.md Adds cmdlet reference.
documentation/Connect-PnPOnline.md Updates parameter documentation.
CHANGELOG.md Adds release notes.
Review details
  • Files reviewed: 9/9 changed files
  • Comments generated: 7
  • Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Commands/Base/ConnectOnline.cs
Comment thread src/Commands/Base/PnPConnection.cs Outdated
Comment thread CHANGELOG.md
Comment thread documentation/Connect-PnPOnline.md
Comment thread documentation/Get-PnPPersistedLogin.md
Comment thread pages/articles/credentialmanagement.md Outdated
Comment thread pages/articles/persistedlogin.md Outdated

Copilot AI left a comment

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.

🔵 Needs a closer look

Cross-platform secure token persistence and cache deletion require human runtime validation despite the implementation being statically coherent.

Review details
  • Files reviewed: 9/9 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread pages/articles/credentialmanagement.md Outdated
Comment thread src/Commands/Base/ConnectOnline.cs Outdated

Copilot AI left a comment

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.

🔵 Needs a closer look

Cross-platform secure token persistence and cache deletion require human runtime validation.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/Commands/Base/PnPConnection.cs:1176

  • When an existing app-only registration triggers this path (cacheRequested is false but CacheEnabled is true), the settings entry remains enabled, so saying persistence “was not enabled” is inaccurate and obscures why every subsequent certificate connection keeps retrying the cache. Use wording that applies both to first-time enablement and reuse.
  • Files reviewed: 9/9 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

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.

2 participants