Fixup automatically created feed permissions

src/Microsoft.DotNet.Arcade.Sdk/tools/SdkTasks/PublishArtifactsInManifest.proj

@@ -75,16 +75,11 @@
     <TargetFramework>netcoreapp3.1</TargetFramework>
     <NETCORE_ENGINEERING_TELEMETRY>Publish</NETCORE_ENGINEERING_TELEMETRY>
 
-    <!-- Default publishing infra target is 2. -->
-    <PublishingInfraVersion Condition="'$(PublishingInfraVersion)' == ''">2</PublishingInfraVersion>
+    <!-- Default publishing infra target is 3. -->
+    <PublishingInfraVersion Condition="'$(PublishingInfraVersion)' == ''">3</PublishingInfraVersion>
   </PropertyGroup>
 
-  <Import Project="SetupTargetFeeds.proj" Condition="'$(PublishingInfraVersion)' == '2'" />
-
   <Target Name="Execute">
-    <CallTarget Targets="SetupTargetFeeds" Condition="'$(PublishingInfraVersion)' == '2'">
-      <Output TaskParameter="TargetOutputs" ItemName="TargetFeedConfig"/>
-    </CallTarget>
 
     <Error Condition="'$(ManifestsBasePath)' == ''" Text="ManifestsBasePath is empty. Please provide the full path to asset manifest(s) directory." />
     <Error Condition="'$(BlobBasePath)' == '' OR '$(PackageBasePath)' == ''" Text="A valid full path to BlobBasePath and PackageBasePath is required." />

src/Microsoft.DotNet.Arcade.Sdk/tools/SdkTasks/PublishSignedAssets.proj

@@ -11,12 +11,6 @@
       - AzdoTargetFeedPAT                       : Required token to publish assets to feeds
   -->
 
-  <PropertyGroup>
-    <TargetFramework>netcoreapp3.1</TargetFramework>
-    <NETCORE_ENGINEERING_TELEMETRY>Publish</NETCORE_ENGINEERING_TELEMETRY>
-    <AzureDevOpsOrg Condition="'$(AzureDevOpsOrg)' == ''">dnceng</AzureDevOpsOrg>
-  </PropertyGroup>
-
   <Target Name="Execute">
     <Error
       Condition="'$(IsInternalBuild)' == ''"
@@ -34,21 +28,33 @@
       Condition="'$(AzdoTargetFeedPAT)' == ''"
       Text="Parameters 'AzdoTargetFeedPAT' is empty." />
 
+    <PropertyGroup>
+      <TargetFramework>netcoreapp3.1</TargetFramework>
+      <NETCORE_ENGINEERING_TELEMETRY>Publish</NETCORE_ENGINEERING_TELEMETRY>
+      <AzureDevOpsOrg Condition="'$(AzureDevOpsOrg)' == ''">dnceng</AzureDevOpsOrg>
+      <AzureDevOpsProject Condition="'(AzureDevOpsProject)' == '' and '(AzureDevOpsOrg)' == 'dnceng' and '$(IsInternalBuild)' == 'true'">internal</AzureDevOpsProject>
+      <AzureDevOpsProject Condition="'(AzureDevOpsProject)' == '' and '(AzureDevOpsOrg)' == 'dnceng' and '$(IsInternalBuild)' == 'false'">public</AzureDevOpsProject>
+    </PropertyGroup>
+
+    <Error
+      Condition="'$(AzureDevOpsProject)' == ''"
+      Text="Parameters 'AzureDevOpsProject' is empty." />
+
     <!--Create the shipping feed-->
     <CreateAzureDevOpsFeed
-        IsInternal="$(IsInternalBuild)"
         AzureDevOpsPersonalAccessToken="$(AzdoTargetFeedPAT)"
         FeedName="$(FeedName)-shipping"
-        AzureDevOpsOrg="$(AzureDevOpsOrg)">
+        AzureDevOpsOrg="$(AzureDevOpsOrg)"
+        AzureDevOpsProject="$(AzureDevOpsProject)">
       <Output TaskParameter="TargetFeedURL" PropertyName="ShippingAzdoPackageFeedURL"/>
     </CreateAzureDevOpsFeed>
 
     <!--Create the nonshipping feed-->
     <CreateAzureDevOpsFeed
-        IsInternal="$(IsInternalBuild)"
         AzureDevOpsPersonalAccessToken="$(AzdoTargetFeedPAT)"
         FeedName="$(FeedName)-nonshipping"
-        AzureDevOpsOrg="$(AzureDevOpsOrg)">
+        AzureDevOpsOrg="$(AzureDevOpsOrg)"
+        AzureDevOpsProject="$(AzureDevOpsProject)">
       <Output TaskParameter="TargetFeedURL" PropertyName="NonShippingAzdoPackageFeedURL"/>
     </CreateAzureDevOpsFeed>
 

src/Microsoft.DotNet.Build.Tasks.Feed/src/AzureDevOpsArtifactFeed.cs

@@ -0,0 +1,67 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Collections.Generic;
+
+namespace Microsoft.DotNet.Build.Tasks.Feed
+{
+    /// <summary>
+    /// Represents the body of a request sent when creating a new feed.
+    /// </summary>
+    /// <remarks>>
+    /// When creating a new feed, we want to set up permissions based on the org and project.
+    /// Right now, only dnceng's public and internal projects are supported.
+    /// New feeds automatically get the feed administrators and project collection administrators as owners,
+    /// but we want to automatically add some additional permissions so that the build services can push to them,
+    /// and organization users can read from them.
+    ///
+    /// Note that there are two ways of providing read access to the feed:
+    /// 1. Providing explicit access in the permissions list to the "Project Collection Valid Users"
+    /// 2. Updating the Local feed view to allow 'collection' users access.
+    ///
+    /// The second is probably preferrable from a an AzDO pattern and usage standpoint. BUT the AzDO API has a drawback where
+    /// the create feed operation cannot create the local view with the appropriate access. Instead, it must be updated after the
+    /// feed is created. This would be fine except that updating a feed's permissions requires administrative permissions, while
+    /// creating a feed only requires contributor permissions. This would require passing around a PAT with management permissions,
+    /// instead of just r/w permissions, which is not ideal.
+    /// </remarks>
+    public class AzureDevOpsArtifactFeed
+    {
+        public AzureDevOpsArtifactFeed(string name, string organization, string project)
+        {
+            Name = name;
+            switch (organization)
+            {
+                case "dnceng":
+                    switch (project)
+                    {
+                        case "public":
+                        case "internal":
+                            Permissions = new List<AzureDevOpsFeedPermission>
+                            {
+                                // Project Collection Build Service
+                                new AzureDevOpsFeedPermission("Microsoft.TeamFoundation.ServiceIdentity;116cce53-b859-4624-9a95-934af41eccef:Build:7ea9116e-9fac-403d-b258-b31fcf1bb293", "contributor"),
+                                // internal Build Service
+                                new AzureDevOpsFeedPermission("Microsoft.TeamFoundation.ServiceIdentity;116cce53-b859-4624-9a95-934af41eccef:Build:b55de4ed-4b5a-4215-a8e4-0a0a5f71e7d8", "contributor"),
+                                // Project administrators
+                                new AzureDevOpsFeedPermission("Microsoft.TeamFoundation.Identity;S-1-9-1551374245-1349140002-2196814402-2899064621-3782482097-0-0-0-0-1", "administrator"),
+                                // Project Collection value users (see class comment for info)
+                                new AzureDevOpsFeedPermission("Microsoft.TeamFoundation.Identity;S-1-9-1551374245-3991166389-1514870082-2833517066-1601300440-0-0-0-0-3", "reader"),
+                            };
+                            break;
+                        default:
+                            throw new NotImplementedException($"Project '{project}' within organization '{organization}' contains no feed permissions information.");
+                    }
+                    break;
+                default:
+                    throw new NotImplementedException($"Organization '{organization}' contains no feed permissions information.");
+
+            }
+        }
+
+        public string Name { get; set; }
+
+        public List<AzureDevOpsFeedPermission> Permissions { get; private set; }
+    }
+}

src/Microsoft.DotNet.Build.Tasks.Feed/src/AzureDevOpsFeedPermission.cs

@@ -0,0 +1,18 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.DotNet.Build.Tasks.Feed
+{
+    public class AzureDevOpsFeedPermission
+    {
+        public AzureDevOpsFeedPermission(string identityDescriptor, string role)
+        {
+            IdentityDescriptor = identityDescriptor;
+            Role = role;
+        }
+
+        public string IdentityDescriptor { get; set; }
+
+        public string Role { get; set; }
+    }
+}

src/Microsoft.DotNet.Build.Tasks.Feed/src/CreateAzureDevOpsFeed.cs

@@ -23,8 +23,24 @@ public class CreateAzureDevOpsFeed : MSBuild.Task
         [Output]
         public string TargetFeedName { get; set; }
 
+        /// <summary>
+        /// Organization that the feed should be created in
+        /// </summary>
+        [Required]
+        public string AzureDevOpsOrg { get; set; }
+
+        /// <summary>
+        /// Project that that feed should be created in. The public/internal visibility
+        /// of this project will determine whether the feed is public.
+        /// </summary>
+        [Required]
+        public string AzureDevOpsProject { get; set; }
+
+        /// <summary>
+        /// Personal access token used to authorize to the API and create the feed
+        /// </summary>
         [Required]
-        public bool IsInternal { get; set; }
+        public string AzureDevOpsPersonalAccessToken { get; set; }
 
         public string RepositoryName { get; set; }
 
@@ -40,12 +56,9 @@ public class CreateAzureDevOpsFeed : MSBuild.Task
         /// </summary>
         public string ContentIdentifier { get; set; }
 
-        [Required]
-        public string AzureDevOpsPersonalAccessToken { get; set; }
-
-        public string AzureDevOpsFeedsApiVersion { get; set; } = "5.0-preview.1";
+        public string AzureDevOpsFeedsApiVersion { get; set; } = "5.1-preview.1";
 
-        public string AzureDevOpsOrg { get; set; } = "dnceng";
+        public string LocalViewVisibility { get; set; } = "collection";
 
         /// <summary>
         /// Number of characters from the commit SHA prefix that should be included in the feed name.
@@ -86,16 +99,19 @@ private async Task<bool> ExecuteAsync()
                 // or contain any of these: @ ~ ; { } ' + = , < > | / \ ? : & $ * " # [ ] %
                 string feedCompatibleRepositoryName = RepositoryName?.Replace('/', '-');
 
-                string accessType = IsInternal ? "internal" : "public";
-                string publicSegment = IsInternal ? string.Empty : "public/";
-                string accessId = IsInternal ? "int" : "pub";
+                // For clarity, and compatibility with existing infrastructure, we include the feed visibility tag.
+                // This serves two purposes:
+                // 1. In nuget.config files (and elsewhere), the name at a glance can identify its visibility
+                // 2. Existing automation has knowledge of "darc-int" and "darc-pub" for purposes of injecting authentication for internal builds
+                //    and managing the isolated feeds within the NuGet.config files.
+                string accessTag = GetFeedVisibilityTag(AzureDevOpsOrg, AzureDevOpsProject);
                 string extraContentInfo = !string.IsNullOrEmpty(ContentIdentifier) ? $"-{ContentIdentifier}" : "";
-                string baseFeedName = FeedName ?? $"darc-{accessId}{extraContentInfo}-{feedCompatibleRepositoryName}-{CommitSha.Substring(0, ShaUsableLength)}";
+                string baseFeedName = FeedName ?? $"darc-{accessTag}{extraContentInfo}-{feedCompatibleRepositoryName}-{CommitSha.Substring(0, ShaUsableLength)}";
                 string versionedFeedName = baseFeedName;
                 bool needsUniqueName = false;
                 int subVersion = 0;
 
-                Log.LogMessage(MessageImportance.High, $"Creating the new {accessType} Azure DevOps artifacts feed '{baseFeedName}'...");
+                Log.LogMessage(MessageImportance.High, $"Creating the new Azure DevOps artifacts feed '{baseFeedName}'...");
 
                 if (baseFeedName.Length > MaxLengthForAzDoFeedNames)
                 {
@@ -118,20 +134,24 @@ private async Task<bool> ExecuteAsync()
                             "Basic",
                             Convert.ToBase64String(Encoding.ASCII.GetBytes(string.Format("{0}:{1}", "", AzureDevOpsPersonalAccessToken))));
 
-                        AzureDevOpsArtifactFeed newFeed = new AzureDevOpsArtifactFeed(versionedFeedName, AzureDevOpsOrg);
+                        AzureDevOpsArtifactFeed newFeed = new AzureDevOpsArtifactFeed(versionedFeedName, AzureDevOpsOrg, AzureDevOpsProject);
 
-                        string body = JsonConvert.SerializeObject(newFeed, _serializerSettings);
+                        string createBody = JsonConvert.SerializeObject(newFeed, _serializerSettings);
 
-                        HttpRequestMessage postMessage = new HttpRequestMessage(HttpMethod.Post, $"{publicSegment}_apis/packaging/feeds");
-                        postMessage.Content = new StringContent(body, Encoding.UTF8, "application/json");
-                        HttpResponseMessage response = await client.SendAsync(postMessage);
+                        using HttpRequestMessage createFeedMessage = new HttpRequestMessage(HttpMethod.Post, $"{AzureDevOpsProject}/_apis/packaging/feeds");
+                        createFeedMessage.Content = new StringContent(createBody, Encoding.UTF8, "application/json");
+                        using HttpResponseMessage createFeedResponse = await client.SendAsync(createFeedMessage);
 
-                        if (response.StatusCode == HttpStatusCode.Created)
+                        if (createFeedResponse.StatusCode == HttpStatusCode.Created)
                         {
                             needsUniqueName = false;
                             baseFeedName = versionedFeedName;
+
+                            /// This is where we would potentially update the Local feed view with permissions to the organization's
+                            /// valid users. But, see <seealso cref="AzureDevOpsArtifactFeed"/> for more info on why this is not
+                            /// done this way.
                         }
-                        else if (response.StatusCode == HttpStatusCode.Conflict)
+                        else if (createFeedResponse.StatusCode == HttpStatusCode.Conflict)
                         {
                             versionedFeedName = $"{baseFeedName}-{++subVersion}";
                             needsUniqueName = true;
@@ -144,12 +164,12 @@ private async Task<bool> ExecuteAsync()
                         }
                         else
                         {
-                            throw new Exception($"Feed '{baseFeedName}' was not created. Request failed with status code {response.StatusCode}. Exception: {await response.Content.ReadAsStringAsync()}");
+                            throw new Exception($"Feed '{baseFeedName}' was not created. Request failed with status code {createFeedResponse.StatusCode}. Exception: {await createFeedResponse.Content.ReadAsStringAsync()}");
                         }
                     }
                 } while (needsUniqueName);
 
-                TargetFeedURL = $"https://pkgs.dev.azure.com/{AzureDevOpsOrg}/{publicSegment}_packaging/{baseFeedName}/nuget/v3/index.json";
+                TargetFeedURL = $"https://pkgs.dev.azure.com/{AzureDevOpsOrg}/{AzureDevOpsProject}/_packaging/{baseFeedName}/nuget/v3/index.json";
                 TargetFeedName = baseFeedName;
 
                 Log.LogMessage(MessageImportance.High, $"Feed '{TargetFeedURL}' created successfully!");
@@ -161,41 +181,31 @@ private async Task<bool> ExecuteAsync()
 
             return !Log.HasLoggedErrors;
         }
-    }
-
-    public class Permission
-    {
-        public Permission(string identityDescriptor, int role)
-        {
-            IdentityDescriptor = identityDescriptor;
-            Role = role;
-        }
 
-        public string IdentityDescriptor { get; set; }
-
-        public int Role { get; set; }
-    }
-
-    public class AzureDevOpsArtifactFeed
-    {
-        public AzureDevOpsArtifactFeed(string name, string organization)
+        /// <summary>
+        /// Returns a tag for feed visibility that will be added to the feed name
+        /// </summary>
+        /// <param name="organization">Organization containing the feed</param>
+        /// <param name="project">Project within <paramref name="organization"/> containing the feed</param>
+        /// <returns>Feed tag</returns>
+        /// <exception cref="NotImplementedException"></exception>
+        private string GetFeedVisibilityTag(string organization, string project)
         {
-            Name = name;
-            if (organization == "dnceng")
+            switch (organization)
             {
-                Permissions = new List<Permission>
-                {
-                    // Mimic the permissions added to a feed when created in the browser
-                    new Permission("Microsoft.TeamFoundation.ServiceIdentity;116cce53-b859-4624-9a95-934af41eccef:Build:b55de4ed-4b5a-4215-a8e4-0a0a5f71e7d8", 3),                      // Project Collection Build Service
-                    new Permission("Microsoft.TeamFoundation.ServiceIdentity;116cce53-b859-4624-9a95-934af41eccef:Build:7ea9116e-9fac-403d-b258-b31fcf1bb293", 3),                      // internal Build Service
-                    new Permission("Microsoft.TeamFoundation.Identity;S-1-9-1551374245-1349140002-2196814402-2899064621-3782482097-0-0-0-0-1", 4),                                      // Feed administrators
-                    new Permission("Microsoft.TeamFoundation.Identity;S-1-9-1551374245-1846651262-2896117056-2992157471-3474698899-1-2052915359-1158038602-2757432096-2854636005", 4)   // Feed administrators and contributors
-                };
+                case "dnceng":
+                    switch (project)
+                    {
+                        case "internal":
+                            return "int";
+                        case "public":
+                            return "pub";
+                        default:
+                            throw new NotImplementedException($"Project '{project}' within organization '{organization}' has no visibility mapping.");
+                    }
+                default:
+                    throw new NotImplementedException($"Organization '{organization}' has no visibility mapping.");
             }
         }
-
-        public string Name { get; set; }
-
-        public List<Permission> Permissions { get; private set; }
     }
 }

src/Microsoft.DotNet.Build.Tasks.Feed/src/model/SetupTargetFeedConfigV3.cs

@@ -30,6 +30,8 @@ public class SetupTargetFeedConfigV3 : SetupTargetFeedConfigBase
 
         public TaskLoggingHelper Log { get; }
 
+        public string AzureDevOpsOrg => "dnceng";
+
         public SetupTargetFeedConfigV3(
             TargetChannelConfig targetChannelConfig,
             bool isInternalBuild,
@@ -195,7 +197,8 @@ private void CreateStableSymbolsFeedIfNeeded()
                 var symbolsFeedTask = new CreateAzureDevOpsFeed()
                 {
                     BuildEngine = BuildEngine,
-                    IsInternal = IsInternalBuild,
+                    AzureDevOpsOrg = AzureDevOpsOrg,
+                    AzureDevOpsProject = IsInternalBuild ? "internal" : "public",
                     AzureDevOpsPersonalAccessToken = AzureDevOpsFeedsKey,
                     RepositoryName = RepositoryName,
                     CommitSha = CommitSha,
@@ -222,7 +225,8 @@ private void CreateStablePackagesFeedIfNeeded()
                 var packagesFeedTask = new CreateAzureDevOpsFeed()
                 {
                     BuildEngine = BuildEngine,
-                    IsInternal = IsInternalBuild,
+                    AzureDevOpsOrg = AzureDevOpsOrg,
+                    AzureDevOpsProject = IsInternalBuild ? "internal" : "public",
                     AzureDevOpsPersonalAccessToken = AzureDevOpsFeedsKey,
                     RepositoryName = RepositoryName,
                     CommitSha = CommitSha