Skip to content
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.

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

xWebSite: skip certificate thumbprint and store name check when SslFlags are valid #600

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

changbowen
Copy link

@changbowen changbowen commented Nov 15, 2021

Pull Request (PR) description

SslFlags value 2 and 3 suggest the use of CCS (Central Certificate Store). Checking for certificate thumbprint and store name should be skipped when CCS is being used.

In IIS Management Console the SSL certificate field would be disabled when CCS is enabled.

This Pull Request (PR) fixes the following issues

Fixes #199

Task list

  • Added an entry to the change log under the Unreleased section of the
    file CHANGELOG.md. Entry should say what was changed and how that
    affects users (if applicable), and reference the issue being resolved
    (if applicable).
  • Resource documentation added/updated in README.md.
  • Resource parameter descriptions added/updated in README.md, schema.mof
    and comment-based help.
  • Comment-based help added/updated.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Community Testing Guidelines.
  • Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
  • New/changed code adheres to DSC Community Style Guidelines.

This change is Reviewable

Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on @changbowen and @johlju)

a discussion (no related file):
Please add a unit test for this change: https://github.com/dsccommunity/xWebAdministration/blob/673d20f7a73d2e6bfdd0715a5628bd7ffa708d5f/tests/Unit/MSFT_xWebsite.Tests.ps1#L2273


a discussion (no related file):
Please update the CHANGELOG.md with an entry for this change.



source/DSCResources/MSFT_xWebSite/MSFT_xWebSite.psm1, line 1439 at r1 (raw file):

                {
                    if ([Environment]::OSVersion.Version -lt '6.2' -or $binding.SslFlags -notin @('2', '3'))
                    {

We should indent the code inside the brace.

@johlju johlju self-requested a review November 15, 2021 11:09
@johlju johlju added the waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. label Nov 15, 2021
@changbowen
Copy link
Author

Indent added.
Sorry I'm not really familiar with the workflow.. had a hard time understanding the unit tests.. Am I supposed to make changes to the unit test and change log files?

@johlju
Copy link
Member

johlju commented Nov 15, 2021

Yes to both. You should add a unit test that verifies that the new code path works, and if existing tests fail then either the code change must be revised or existing unit tests revised.

The CHANGELOG.md is used as the release notes for the next release. By adding a new entry under the Unreleased-section it will be automatically used in the release notes.

@johlju
Copy link
Member

johlju commented Jun 8, 2022

We have renamed the resource, removeing 'x', so please rebase this PR.

@david-garcia-garcia
Copy link

Works if you use: CertificateThumbprint = 'Allow'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author.
Projects
None yet
3 participants