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

Asser-M365DSCBlueprint: add additional Auth Meth to Asser-M365DSCBlueprint #1792

Closed
wants to merge 1 commit into from

Conversation

Timsto
Copy link
Contributor

@Timsto Timsto commented Mar 1, 2022

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

Copy link
Member

@ykuijs ykuijs left a comment

Choose a reason for hiding this comment

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

Could you please also update the changelog to reflect this change?

@ykuijs
Copy link
Member

ykuijs commented Mar 8, 2022

@NikCharlebois Can you please also check this PR? Not 100% if this will work for this function.

@NikCharlebois
Copy link
Collaborator

@Timsto This function should not handle credentials. It is purely meant to retrieve the domain if service principal if passed. If credentials are used, then determining the domain is very easy and straightforward:

$domain = $credentials.UserName.Split('@')[1]

@andikrueger
Copy link
Collaborator

@NikCharlebois and @Timsto Are there any updates to this PR?

At the moment there are two tasks open:

  • Update Changelog
  • Remove Credential parameter from Get-M365DSCTenantDomain

Comment on lines +1156 to +1159
[Parameter()]
[System.Management.Automation.PSCredential]
$Credential,

Copy link
Collaborator

Choose a reason for hiding this comment

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

as mentioned in #1792 (comment) this function should not handle any Credentials.

@NikCharlebois
Copy link
Collaborator

@Timsto have you had a chance to look at the comments? Should we keep this PR open or close it?

@ykuijs
Copy link
Member

ykuijs commented Mar 3, 2023

Closing this PR due to inactivity, but code will be adopted in a different PR I am working on.

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