-
Notifications
You must be signed in to change notification settings - Fork 98
Request for catalog listing access for finding packages #1824
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
base: master
Are you sure you want to change the base?
Conversation
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
return null; | ||
} | ||
|
||
request.Content = new StringContent(content); |
Check warning
Code scanning / CodeQL
Information exposure through transmitted data Medium
sensitive information
This data transmitted to the user depends on
sensitive information
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 8 hours ago
To fix the issue, the sensitive data (password) should not be directly included in the content
variable. Instead, the code should use a secure mechanism to transmit authentication information, such as an encrypted token or a secure header. If the password must be used, it should be hashed or encrypted before transmission. Additionally, the code should ensure that sensitive data is not logged or exposed in error messages.
The fix involves:
- Modifying the
Utils.GetContainerRegistryAccessTokenFromSecretManagement
method to return a secure token instead of a plaintext password. - Updating the
content
variable inContainerRegistryServerAPICalls.cs
to use the secure token instead of the password. - Ensuring that sensitive data is not logged or exposed in error messages.
-
Copy modified line R553
@@ -552,3 +552,3 @@ | ||
_cmdletPassedIn.WriteDebug("In ContainerRegistryServerAPICalls::GetContainerRegistryRefreshToken()"); | ||
string content = string.Format(containerRegistryRefreshTokenTemplate, Registry, tenant, accessToken); | ||
string content = string.Format(containerRegistryRefreshTokenTemplate, Registry, tenant, Convert.ToBase64String(Encoding.UTF8.GetBytes(accessToken))); | ||
var contentHeaders = new Collection<KeyValuePair<string, string>> { new KeyValuePair<string, string>("Content-Type", "application/x-www-form-urlencoded") }; |
-
Copy modified lines R720-R721 -
Copy modified lines R726-R727
@@ -719,3 +719,4 @@ | ||
string password = new NetworkCredential(string.Empty, secretSecureString).Password; | ||
return password; | ||
string secureToken = Convert.ToBase64String(Encoding.UTF8.GetBytes(password)); | ||
return secureToken; | ||
} | ||
@@ -724,3 +725,4 @@ | ||
string password = new NetworkCredential(string.Empty, psCredSecret.Password).Password; | ||
return password; | ||
string secureToken = Convert.ToBase64String(Encoding.UTF8.GetBytes(password)); | ||
return secureToken; | ||
} |
PR Summary
This pull request introduces enhancements to the
ContainerRegistryServerAPICalls
class, focusing on improving the handling of access tokens, adding support for catalog-specific scopes, and refining error handling and debugging. The most significant changes include adding aneedCatalogAccess
parameter to methods dealing with authentication, updating templates for URL and content formatting, and improving debugging and error reporting.Authentication and Scopes:
needCatalogAccess
parameter to theGetContainerRegistryAccessToken
andIsContainerRegistryUnauthenticated
methods to support catalog-specific access tokens. This allows finer control over authentication based on whether catalog access is needed. (src/code/ContainerRegistryServerAPICalls.cs
, [1] [2]grantTypeTemplate
andauthUrlTemplate
) to dynamically include catalog scope whenneedCatalogAccess
is true. (src/code/ContainerRegistryServerAPICalls.cs
, [1] [2]Error Handling and Debugging:
FindPackagesWithVersionHelper
by skipping invalid NuGet package versions instead of returningnull
. Added debug logs to provide detailed information about skipped packages. (src/code/ContainerRegistryServerAPICalls.cs
, src/code/ContainerRegistryServerAPICalls.csL1764-R1781)IsContainerRegistryUnauthenticated
to log error records when failing to retrieve anonymous access tokens. (src/code/ContainerRegistryServerAPICalls.cs
, src/code/ContainerRegistryServerAPICalls.csL485-R504)Code Refinements:
GetHttpResponseJObjectUsingContentHeaders
to ensure HTTP GET requests do not include a body, adhering to HTTP standards. (src/code/ContainerRegistryServerAPICalls.cs
, [1] [2]PR Context
In some non-microsoft tenants, finding packages was not working.
PR Checklist
.h
,.cpp
,.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
or[ WIP ]
to the beginning of the title (theWIP
bot will keep its status check atPending
while the prefix is present) and remove the prefix when the PR is ready.