docs: add Windows User Credentials section to Credential-overview#2466
docs: add Windows User Credentials section to Credential-overview#2466AlexMnrs wants to merge 8 commits into
Conversation
Signed-off-by: alexmnrs <alexmnrs92@gmail.com>
- Document UserName vs GetNetworkCredential().UserName behavior - Table of supported username formats (FQDN, username-only, NetBIOS) - Note NetBIOS incompatibility with SQLPS/SQL 2016 - Recommendations and examples for credential creation - Reference issues dsccommunity#1223 and dsccommunity#1768 Addresses dsccommunity#1768
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds markdownlint suppressions to two existing credential overview sections and introduces a new “Windows User Credentials” section with username format guidance, recommendations, and PowerShell/DSC examples. ChangesWindows User Credentials documentation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/WikiSource/Credential-overview.md`:
- Around line 82-90: The ordered list in the Recommendations section uses mixed
numbering, which does not follow the required 1/1/1 style. Update the list in
the Markdown content so every ordered item in that block uses “1.” consistently,
keeping the existing text but changing the list markers only. Locate the fix in
the Recommendations section of the Credential-overview document.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4b2cca40-b3ab-45c0-93a9-2c1145f8892f
📒 Files selected for processing (4)
CHANGELOG.mdsource/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1source/DSCResources/DSC_SqlWindowsFirewall/en-US/DSC_SqlWindowsFirewall.strings.psd1source/WikiSource/Credential-overview.md
…tyle) Addresses CodeRabbit review comment on PR dsccommunity#2466
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
Great work, thank you! Just small comments.
| > without domain prefix. | ||
|
|
||
| This limitation was identified in [issue #1223](https://github.com/dsccommunity/SqlServerDsc/issues/1223) | ||
| where `GetNetworkCredential().UserName` was being used internally, causing |
There was a problem hiding this comment.
GetNetworkCredential().UserName is not used in any code, so this paragraph is incorrect and states past-tense which we do not need to document. Also, if this is still an issue it is not because of GetNetworkCredential().UserName and if this is kept it should instead state why it doesn't work today. 🤔
| **Always use `$Credential.UserName`** when passing the username to SMO connection | ||
| contexts (e.g., `ConnectAsUserName`). This preserves the domain format exactly | ||
| as the user entered it. |
There was a problem hiding this comment.
This is referencing properties of SMO types. For examples it should instead reference parameters of the commands or DSC resources. The reader will want to know how to use the commands and DSC resource in this module, not SMO.
| 2. **Avoid NetBIOS format** (`DOMAIN\user`) when targeting SQL Server 2016 or | ||
| environments where SQLPS may be used. | ||
| 3. **Use `$Credential.UserName`** (not `GetNetworkCredential().UserName`) in | ||
| any custom code that passes credentials to SMO connection contexts. |
There was a problem hiding this comment.
| any custom code that passes credentials to SMO connection contexts. | |
| any custom code that passes credentials to commands or DSC resources. |
| # FQDN format (recommended - works everywhere) | ||
| $password = ConvertTo-SecureString 'P@ssw0rd1' -AsPlainText -Force | ||
| $username = 'user@company.local' | ||
| $credential = New-Object -TypeName System.Management.Automation.PSCredential -ArgumentList $username, $password |
There was a problem hiding this comment.
We could also do with the short form (could be another example)
| $credential = New-Object -TypeName System.Management.Automation.PSCredential -ArgumentList $username, $password | |
| $credential = [PSCredential]::new($username, $password) |
| $username = 'user' | ||
| $credential = New-Object -TypeName System.Management.Automation.PSCredential -ArgumentList $username, $password |
There was a problem hiding this comment.
Set the password in this example too so it more clear, otherwise "username only" could be seen as a PSCredential without password, which is not possible.
| # NetBIOS format (AVOID for SQL 2016 / SQLPS) | ||
| $username = 'COMPANY\user' | ||
| $credential = New-Object -TypeName System.Management.Automation.PSCredential -ArgumentList $username, $password | ||
| ``` |
There was a problem hiding this comment.
I don't think we need to document how to NOT do it? It is already stated above, so we don't need an example for it?
| ``` | ||
|
|
||
| When using these credentials with SqlServerDsc resources, pass them via the | ||
| built-in `PsDscRunAsCredential` parameter: |
There was a problem hiding this comment.
Also add something like
| built-in `PsDscRunAsCredential` parameter: | |
| built-in `PsDscRunAsCredential`, credential or password parameters. This example shows the built-in `PsDscRunAsCredential` parameter: |
| <sup>_This section was informed by [issue #1223](https://github.com/dsccommunity/SqlServerDsc/issues/1223) | ||
| and [issue #1768](https://github.com/dsccommunity/SqlServerDsc/issues/1768)._</sup> |
There was a problem hiding this comment.
We don't need to mention issue #1768 as that is what this PR resolves.
| <sup>_This section was informed by [issue #1223](https://github.com/dsccommunity/SqlServerDsc/issues/1223) | |
| and [issue #1768](https://github.com/dsccommunity/SqlServerDsc/issues/1768)._</sup> | |
| <sup>_This section was informed by [issue #1223](https://github.com/dsccommunity/SqlServerDsc/issues/1223)._</sup> |
johlju
added
the
waiting for code fix
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2466 +/- ##
====================================
Coverage 91% 91%
====================================
Files 32 32
Lines 5353 5353
====================================
Hits 4904 4904
Misses 449 449
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
source/WikiSource/Credential-overview.md (2)
56-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd markdownlint-disable comment for the wide table.
The table header exceeds 80 characters. Per the Markdown style guidelines, disable the MD013 rule for this table block.
As per path instructions: "Disable MD013 rule by adding a comment for tables/code blocks exceeding 80 characters".
📝 Proposed fix
+<!-- markdownlint-disable MD013 --> | Format | Example | Works with SQLPS (SQL 2016) | Works with SqlServer module | | ------ | ------- | --------------------------- | --------------------------- | | **FQDN (UPN)** | `user@domain.local` | ✅ Yes | ✅ Yes | | **Username only** | `user` | ✅ Yes | ✅ Yes | | **NetBIOS** | `DOMAIN\user` | ❌ **No** | ✅ Yes | +<!-- markdownlint-enable MD013 -->🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/WikiSource/Credential-overview.md` around lines 56 - 60, The wide markdown table in Credential-overview.md triggers MD013 because the header exceeds 80 characters; add a markdownlint disable comment for MD013 around this specific table block. Use the existing table near the credential format examples and ensure the disable applies only to that table, then re-enable afterward if needed.Source: Path instructions
48-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse bold for parameter names and italic for module/product names.
Several text formatting guidelines are not followed in the changed text. Please update the following:
- Line 48:
_SqlServerDsc_(module name)- Line 50:
**Credential**,**UserName**,**Password**(parameter names in text)- Line 63:
_SQLPS_(module name)- Line 64:
_SQL Server_(product name)- Line 69:
_SQL Server_(product name)- Line 90–92:
**PsDscRunAsCredential**,**credential**,**password**(parameter names in text)As per path instructions: "Parameters: bold" and "Resource/module/product names: italic".
📝 Proposed fix
-When using Windows user credentials (i.e., `LoginType = 'WindowsUser'`) with -SqlServerDsc resources, there are important considerations regarding the +When using Windows user credentials (i.e., `LoginType = 'WindowsUser'`) with +_SqlServerDsc_ resources, there are important considerations regarding the username format used in credential, username, and password parameters. +username format used in **Credential**, **UserName**, and **Password** parameters. -When using these credentials with SqlServerDsc resources, pass them via the -built-in `PsDscRunAsCredential`, credential or password parameters. This -example shows the built-in `PsDscRunAsCredential` parameter: +When using these credentials with _SqlServerDsc_ resources, pass them via the +built-in **PsDscRunAsCredential**, **credential** or **password** parameters. This +example shows the built-in **PsDscRunAsCredential** parameter:-### Recommendations +### Recommendations 1. **Prefer FQDN format** (`user@domain.local`) for maximum compatibility across - all SQL Server versions and PowerShell modules. + all _SQL Server_ versions and PowerShell modules. 1. **Avoid NetBIOS format** (`DOMAIN\user`) when targeting SQL Server 2016 or - environments where SQLPS may be used. + environments where _SQLPS_ may be used.Also applies to: 50-50, 63-64, 69-69, 90-92
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/WikiSource/Credential-overview.md` at line 48, Update the affected prose in Credential-overview.md to follow the formatting rules: wrap module/product names such as SqlServerDsc, SQLPS, and SQL Server in italics, and wrap parameter names such as Credential, UserName, Password, PsDscRunAsCredential, credential, and password in bold. Make the edits in the referenced paragraphs so the existing text reads consistently with the “Parameters: bold” and “Resource/module/product names: italic” guidance.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@source/WikiSource/Credential-overview.md`:
- Around line 56-60: The wide markdown table in Credential-overview.md triggers
MD013 because the header exceeds 80 characters; add a markdownlint disable
comment for MD013 around this specific table block. Use the existing table near
the credential format examples and ensure the disable applies only to that
table, then re-enable afterward if needed.
- Line 48: Update the affected prose in Credential-overview.md to follow the
formatting rules: wrap module/product names such as SqlServerDsc, SQLPS, and SQL
Server in italics, and wrap parameter names such as Credential, UserName,
Password, PsDscRunAsCredential, credential, and password in bold. Make the edits
in the referenced paragraphs so the existing text reads consistently with the
“Parameters: bold” and “Resource/module/product names: italic” guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1df4d141-2335-4fd9-a26c-0ebc633a30ce
📒 Files selected for processing (1)
source/WikiSource/Credential-overview.md
|
Thanks for the careful review, @johlju. I pushed an update that addresses the wording and examples: removed the incorrect GetNetworkCredential()/SMO explanation, kept the guidance focused on commands and DSC resources, switched the credential examples to [PSCredential]::new(), removed the NetBIOS anti-example, and trimmed the footer to only reference #1223. I also updated the branch with current main. |
2 participants
Summary
source/WikiSource/Credential-overview.md$Credential.UserNamevs$Credential.GetNetworkCredential().UserNamebehavior differenceValidation
Closes #1768
This change is