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

Support Microsoft Entra Id authentication for rest source #4123

Merged
merged 12 commits into from Feb 1, 2024

Conversation

yao-msft
Copy link
Contributor

@yao-msft yao-msft commented Jan 31, 2024

Change

This pr adds Microsoft Entra Id support for rest source. To achieve this, rest source /information contract is updated with below. The /information endpoint should never use authentication as it is the first handshake between winget client and rest source.

// "Authentication": {
//     "AuthenticationType": "microsoftEntraId",
//     "MicrosoftEntraIdAuthenticationInfo" : {
//         "Resource": "Resource",
//         "Scope" : "test"
//     }
// }

Rest source interface will read this info and acquire authentication using WebAccountManager OS api during all following source interaction (/packageManifests, /manifestSearch)

CLI arguments updated to specify authentication behavior

COM interface updated to specify authentication behavior from caller and provide authentication info of package catalog to caller

Validation

  • Due to lack of e2e rest source tests and complex in getting real Microsoft Entra Id token. E2e validation is manually performed. I tested cli (packaged and unpackaged), InProcCom and OutOfProcCom . Both interactive and silent flow works correctly. I setup a test rest source with Microsoft Entra Id protection (based on tenants) and it's working as expected
  • Added a bunch of unit tests: AuthenticationInfo parsing, RestClient and RestInterface creation, and RestInterface GetManifest and Search operation with override Microsoft Entra Id token
Microsoft Reviewers: Open in CodeFlow

@yao-msft yao-msft requested a review from a team as a code owner January 31, 2024 23:55
@denelon
Copy link
Contributor

denelon commented Feb 1, 2024

@yao-msft
Copy link
Contributor Author

yao-msft commented Feb 1, 2024

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

auto authenticatedAccount = responseData.WebAccount();

// Check token's corresponding account matches user input if applicable.
if (m_authArgs.AuthenticationAccount.empty() || Utility::CaseInsensitiveEquals(m_authArgs.AuthenticationAccount, Utility::ConvertToUTF8(authenticatedAccount.UserName())))
Copy link
Contributor

Choose a reason for hiding this comment

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

m_authArgs.AuthenticationAccount.empty()

Does an empty authenticationAccount mean that it succeeded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Empty m_authArgs.AuthenticationAccount means user or caller did not specify which account to use. So we consider it as authentication success as long as WebAccountmanager returns success.

@yao-msft yao-msft merged commit 3bcd69d into microsoft:master Feb 1, 2024
8 checks passed
@yao-msft yao-msft deleted the restaad branch February 1, 2024 21:40
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.

None yet

3 participants