Browse Source

[release/8.0] Improve dev-certs export error message (#58470)

* Improve dev-certs export error message

During a recent security review of the dev-certs tool, we observed that on export it would create a directory that was potentially world-readable (e.g. based on permissions inherited from the parent directory).  We decided it would be more appropriate to let users make the decision of who should have access to the directory.  Unfortunately, this removal of functionality broke some app authors' workflows.  When dev-certs is run directly, the `--verbose` output makes it clear what went wrong and what needs to happen, but the non-verbose output that appears when another tool does the export is less helpful.  This change introduces a new top-level error state for an export failure caused by a non-existent target directory to make it clearer how to fix broken workflows.

The behavior changed in #57108, which included a backport of #56985, and shipped in 8.0.10.

For #58330

* Improve error test
Andrew Casey 1 year ago
parent
commit
b74d4a6c08

+ 1 - 0
src/Shared/CertificateGeneration/CertificateManager.cs

@@ -328,6 +328,7 @@ internal abstract class CertificateManager
                 var exportDir = Path.GetDirectoryName(path);
                 if (!string.IsNullOrEmpty(exportDir) && !Directory.Exists(exportDir))
                 {
+                    result = EnsureCertificateResult.ErrorExportingTheCertificateToNonExistentDirectory;
                     throw new InvalidOperationException($"The directory '{exportDir}' does not exist.  Choose permissions carefully when creating it.");
                 }
 

+ 1 - 0
src/Shared/CertificateGeneration/EnsureCertificateResult.cs

@@ -10,6 +10,7 @@ internal enum EnsureCertificateResult
     ErrorCreatingTheCertificate,
     ErrorSavingTheCertificateIntoTheCurrentUserPersonalStore,
     ErrorExportingTheCertificate,
+    ErrorExportingTheCertificateToNonExistentDirectory,
     FailedToTrustTheCertificate,
     PartiallyFailedToTrustTheCertificate,
     UserCancelledTrustStep,

+ 1 - 1
src/Tools/FirstRunCertGenerator/test/CertificateManagerTests.cs

@@ -373,7 +373,7 @@ public class CertificateManagerTests : IClassFixture<CertFixture>
             .EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), Path.Combine("NoSuchDirectory", CertificateName));
 
         // Assert
-        Assert.Equal(EnsureCertificateResult.ErrorExportingTheCertificate, result);
+        Assert.Equal(EnsureCertificateResult.ErrorExportingTheCertificateToNonExistentDirectory, result);
     }
 
     [Fact]

+ 4 - 0
src/Tools/dotnet-dev-certs/src/Program.cs

@@ -425,6 +425,10 @@ internal sealed class Program
             case EnsureCertificateResult.ErrorExportingTheCertificate:
                 reporter.Warn("There was an error exporting the HTTPS developer certificate to a file.");
                 return ErrorExportingTheCertificate;
+            case EnsureCertificateResult.ErrorExportingTheCertificateToNonExistentDirectory:
+                // A distinct warning is useful, but a distinct error code is probably not.
+                reporter.Warn("There was an error exporting the HTTPS developer certificate to a file. Please create the target directory before exporting. Choose permissions carefully when creating it.");
+                return ErrorExportingTheCertificate;
             case EnsureCertificateResult.PartiallyFailedToTrustTheCertificate:
                 // A distinct warning is useful, but a distinct error code is probably not.
                 reporter.Warn("There was an error trusting the HTTPS developer certificate. It will be trusted by some clients but not by others.");