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

Add Entra Id authentication support for installer download #5095

Merged
merged 12 commits into from
Dec 23, 2024

Conversation

yao-msft
Copy link
Contributor

@yao-msft yao-msft commented Dec 23, 2024

  • The entra id support for installer download reuses existing entra id code for rest source access (parsing, authenticating).
  • Added yaml manifest parsing for authentication to make manifest in parity with rest source. Added validation to disable Authentication in manifest for community repo.
  • Validated manually with private Azure blob storage hosted installer.
  • Added manifest tests and installer download authentication unit tests
Microsoft Reviewers: Open in CodeFlow

@yao-msft yao-msft marked this pull request as ready for review December 23, 2024 17:18
@yao-msft yao-msft requested a review from a team as a code owner December 23, 2024 17:18
@yao-msft
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yao-msft
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines failed to run 1 pipeline(s).

@yao-msft
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines failed to run 1 pipeline(s).

ryfu-msft
ryfu-msft previously approved these changes Dec 23, 2024
</data>
<data name="InstallerDownloadAuthenticationNotSupported" xml:space="preserve">
<value>Failed to download installer. This winget version does not support the installer download authentication method. Try upgrade to latest winget version.</value>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<value>Failed to download installer. This winget version does not support the installer download authentication method. Try upgrade to latest winget version.</value>
<value>Failed to download installer. This winget version does not support the installer download authentication method. Try upgrading to latest winget version.</value>

bool MicrosoftEntraIdAuthenticationInfo::operator<(const MicrosoftEntraIdAuthenticationInfo& other) const
{
// std::tie implements tuple comparison, wherein it checks the first item in the tuple,
// iff the first elements are equal, then the second element is used for comparison, and so on
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// iff the first elements are equal, then the second element is used for comparison, and so on
// if the first elements are equal, then the second element is used for comparison, and so on

bool AuthenticationInfo::operator<(const AuthenticationInfo& other) const
{
// std::tie implements tuple comparison, wherein it checks the first item in the tuple,
// iff the first elements are equal, then the second element is used for comparison, and so on
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// iff the first elements are equal, then the second element is used for comparison, and so on
// if the first elements are equal, then the second element is used for comparison, and so on

@@ -24,9 +26,9 @@ namespace AppInstaller::Authentication

AICLI_LOG(Core, Info, << "AuthenticationArguments values. Mode: " << AuthenticationModeToString(args.Mode) << ", Account: " << args.AuthenticationAccount);

if (info.Type == AuthenticationType::MicrosoftEntraId)
if (info.Type == AuthenticationType::MicrosoftEntraId || info.Type == AuthenticationType::MicrosoftEntraIdForAzureBlobStorage)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Should we create a semantic function for this similar to what you did for WebAccountAuthenticator?

Copy link
Contributor

@ryfu-msft ryfu-msft left a comment

Choose a reason for hiding this comment

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

:shipit:

@yao-msft yao-msft merged commit b49f784 into microsoft:master Dec 23, 2024
9 checks passed
@yao-msft yao-msft deleted the installeraad branch December 23, 2024 22:31
@Elonmusk678
Copy link

Elonmusk678 commented Dec 26, 2024

:shipit:

Hello

@Elonmusk678
Copy link

:shipit:

How are you

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