fix issue that error happens if there's no write permission to user profile path

erich-wang · erich-wang · commit 4b391f10064b · 2020-11-04T15:51:16.000+08:00
diff --git a/src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs b/src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs
@@ -416,7 +416,11 @@ public override void ExecuteCmdlet()
                        }
                        else
                        {
-                           WriteWarning(Resources.SuggestToUseDeviceCodeAuth);
+                           if (ParameterSetName == UserParameterSet && UseDeviceAuthentication == false)
+                           {
+                               //Display only if user is using Interactive auth
+                               WriteWarning(Resources.SuggestToUseDeviceCodeAuth);
+                           }
                            WriteDebug(ex.ToString());
                            throw;
                        }
@@ -474,23 +478,24 @@ private void SetContextWithOverwritePrompt(Action<AzureRmProfile, RMProfileClien
                     originalShouldRefreshContextsFromCache = profile.ShouldRefreshContextsFromCache;
                     profile.ShouldRefreshContextsFromCache = false;
                 }
-            if (!CheckForExistingContext(profile, name)
-                || Force.IsPresent
-                || ShouldContinue(string.Format(Resources.ReplaceContextQuery, name),
-                string.Format(Resources.ReplaceContextCaption, name)))
-            {
-                ModifyContext((prof, client) => setContextAction(prof, client, name));
+                if (!CheckForExistingContext(profile, name)
+                    || Force.IsPresent
+                    || ShouldContinue(string.Format(Resources.ReplaceContextQuery, name),
+                    string.Format(Resources.ReplaceContextCaption, name)))
+                {
+                    ModifyContext((prof, client) => setContextAction(prof, client, name));
+                }
             }
-        }
             finally
             {
-                if(profile != null && originalShouldRefreshContextsFromCache.HasValue)
+                if (profile != null && originalShouldRefreshContextsFromCache.HasValue)
                 {
                     profile.ShouldRefreshContextsFromCache = originalShouldRefreshContextsFromCache.Value;
                 }
             }
         }
 
+        //This method may throw exception because of permission issue, exception should be handled from caller
         private static IAzureContextContainer GetAzureContextContainer()
         {
             var provider = new ProtectedProfileProvider();
@@ -507,7 +512,7 @@ public void OnImport()
             {
 #endif
                 AzureSessionInitializer.InitializeAzureSession();
-                AzureSessionInitializer.MigrateAdalCache(AzureSession.Instance, GetAzureContextContainer);
+                AzureSessionInitializer.MigrateAdalCache(AzureSession.Instance, GetAzureContextContainer, WriteInitializationWarnings);
 #if DEBUG
                 if (!TestMockSupport.RunningMocked)
                 {
@@ -525,11 +530,21 @@ public void OnImport()
                     autoSaveEnabled = localAutosave;
                 }
 
-                if (autoSaveEnabled && !SharedTokenCacheProvider.SupportCachePersistence(out string message))
+                try
+                {
+                    if (autoSaveEnabled && !SharedTokenCacheProvider.SupportCachePersistence(out string message))
+                    {
+                        // If token cache persistence is not supported, fall back to plain text persistence, and print a warning
+                        // We cannot just throw an exception here because this is called when importing the module
+                        WriteInitializationWarnings(Resources.TokenCacheEncryptionNotSupportedWithFallback);
+                    }
+                }
+                catch(Exception ex)
                 {
-                    // If token cache persistence is not supported, fall back to plain text persistence, and print a warning
-                    // We cannot just throw an exception here because this is called when importing the module
-                    WriteInitializationWarnings(Resources.TokenCacheEncryptionNotSupportedWithFallback);
+                    //Likely the exception is related permission, fall back context save mode to process
+                    autoSaveEnabled = false;
+                    AzureSession.Instance.ARMContextSaveMode = ContextSaveMode.Process;
+                    WriteInitializationWarnings(Resources.FallbackContextSaveModeDueCacheCheckError.FormatInvariant(ex.Message));
                 }
 
                 if(!InitializeProfileProvider(autoSaveEnabled))
diff --git a/src/Accounts/Accounts/ChangeLog.md b/src/Accounts/Accounts/ChangeLog.md
@@ -18,6 +18,7 @@
         - Additional information about change #1
 -->
 ## Upcoming Release
+* Fixed an issue that error happens if user profile path is inaccessible
 * Fixed an issue causing `Connect-AzAccount -KeyVaultAccessToken` not working [#13127]
 * Fixed null reference and method case insensitive in `Invoke-AzRestMethod`
 
diff --git a/src/Accounts/Accounts/Common/AzureContextModificationCmdlet.cs b/src/Accounts/Accounts/Common/AzureContextModificationCmdlet.cs
@@ -127,12 +127,9 @@ internal bool InitializeProfileProvider(bool useAutoSaveProfile = false)
                     {
                         ProtectedProfileProvider.InitializeResourceManagerProfile();
                     }
-                    catch (SystemException e)
+                    catch (Exception e)
                     {
-                        if (!(e is IOException) && !(e is UnauthorizedAccessException))
-                        {
-                            throw e;
-                        }
+                        //Likely the exception is related to IO or permission, fallback to Process save mode
                         WriteInitializationWarnings(string.Format(Resources.ProfileFileNotAccessible, e.Message));
                         ResourceManagerProfileProvider.InitializeResourceManagerProfile(true);
                     }
diff --git a/src/Accounts/Accounts/Properties/Resources.Designer.cs b/src/Accounts/Accounts/Properties/Resources.Designer.cs
diff --git a/src/Accounts/Accounts/Properties/Resources.resx b/src/Accounts/Accounts/Properties/Resources.resx
@@ -496,10 +496,10 @@
     <value>Context autosave is not supported in current environment. Please disable it using 'Disable-AzContextSave'.</value>
   </data>
   <data name="TokenCacheEncryptionNotSupportedWithFallback" xml:space="preserve">
-    <value>Token cache encryption is not supported in current environment and token cache will be fallen back as plain text.</value>
+    <value>INITIALIZATION: Token cache encryption is not supported in current environment and token cache will be fallen back as plain text.</value>
   </data>
   <data name="ProfileFileNotAccessible" xml:space="preserve">
-    <value>Fail to access profile file and will try to use process ContextAutosaveSetting mode. Detailed error: '{0}'</value>
+    <value>INITIALIZATION: Fail to access profile file and will try to use process ContextAutosaveSetting mode. Detailed error: '{0}'</value>
   </data>
   <data name="AzurePowerShellFeedback" xml:space="preserve">
     <value>The Azure PowerShell team is listening, please let us know how we are doing: {0}.</value>
@@ -519,4 +519,7 @@
   <data name="SuggestToUseDeviceCodeAuth" xml:space="preserve">
     <value>Please run 'Connect-AzAccount -DeviceCode' if browser is not supported in this session.</value>
   </data>
+  <data name="FallbackContextSaveModeDueCacheCheckError" xml:space="preserve">
+    <value>INITIALIZATION: Fallback context save mode to process because of error during checking token cache persistence: {0}.</value>
+  </data>
 </root>
diff --git a/src/Accounts/Authentication/AzureSessionInitializer.cs b/src/Accounts/Authentication/AzureSessionInitializer.cs
@@ -99,39 +99,46 @@ static bool MigrateSettings(IDataStore store, string oldProfileDirectory, string
             return false;
         }
 
-        public static void MigrateAdalCache(IAzureSession session, Func<IAzureContextContainer> getContextContainer)
+        public static void MigrateAdalCache(IAzureSession session, Func<IAzureContextContainer> getContextContainer, Action<string> writeWarning)
         {
-            if (session.ARMContextSaveMode == ContextSaveMode.Process)
+            try
             {
-                // Don't attempt to migrate if context autosave is disabled
-                return;
-            }
+                if (session.ARMContextSaveMode == ContextSaveMode.Process)
+                {
+                    // Don't attempt to migrate if context autosave is disabled
+                    return;
+                }
 
-            var adalCachePath = Path.Combine(session.ProfileDirectory, "TokenCache.dat");
-            var msalCachePath = Path.Combine(session.TokenCacheDirectory, "msal.cache");
-            var store = session.DataStore;
-            if (!store.FileExists(adalCachePath) || store.FileExists(msalCachePath))
-            {
-                // Return if
-                // (1) The ADAL cache doesn't exist (nothing to migrate), or
-                // (2) The MSAL cache does exist (don't override existing cache)
-                return;
-            }
+                var adalCachePath = Path.Combine(session.ProfileDirectory, "TokenCache.dat");
+                var msalCachePath = Path.Combine(session.TokenCacheDirectory, "msal.cache");
+                var store = session.DataStore;
+                if (!store.FileExists(adalCachePath) || store.FileExists(msalCachePath))
+                {
+                    // Return if
+                    // (1) The ADAL cache doesn't exist (nothing to migrate), or
+                    // (2) The MSAL cache does exist (don't override existing cache)
+                    return;
+                }
 
-            byte[] adalData;
-            try
-            {
-                adalData = File.ReadAllBytes(adalCachePath);
-            }
-            catch
-            {
-                // Return if there was an error converting the ADAL data safely
-                return;
-            }
+                byte[] adalData;
+                try
+                {
+                    adalData = File.ReadAllBytes(adalCachePath);
+                }
+                catch
+                {
+                    // Return if there was an error converting the ADAL data safely
+                    return;
+                }
 
-            if(adalData != null && adalData.Length > 0)
+                if (adalData != null && adalData.Length > 0)
+                {
+                    new AdalTokenMigrator(adalData, getContextContainer).MigrateFromAdalToMsal();
+                }
+            }
+            catch(Exception e)
             {
-                new AdalTokenMigrator(adalData, getContextContainer).MigrateFromAdalToMsal();
+                writeWarning(Resources.FailedToMigrateAdal2Msal.FormatInvariant(e.Message));
             }
         }
 
diff --git a/src/Accounts/Authentication/Properties/Resources.Designer.cs b/src/Accounts/Authentication/Properties/Resources.Designer.cs
diff --git a/src/Accounts/Authentication/Properties/Resources.resx b/src/Accounts/Authentication/Properties/Resources.resx
@@ -352,13 +352,13 @@
   <data name="UnableToLaunchBrowser" xml:space="preserve">
     <value>Unable to launch a browser for authorization code login. Reverting to device code login.</value>
   </data>
-  <data name="EnvironmentNotFound" xml:space="preserve">
-    <value>The environment name '{0}' is not found.</value>
-  </data>
   <data name="AutosaveNotSupportedWithSuggestion" xml:space="preserve">
     <value>Context autosave is not supported on current environment. Please disable it using 'Disable-AzContextSave'.</value>
   </data>
   <data name="MissingPasswordAndNoCache" xml:space="preserve">
     <value>Password is missing and no cache found for the current user.</value>
   </data>
+  <data name="FailedToMigrateAdal2Msal" xml:space="preserve">
+    <value>INITIALZATION: Failed to migrate ADAL token to MSAL token with error : {0}</value>
+  </data>
 </root>

Original file line number	Diff line number	Diff line change
`@@ -416,7 +416,11 @@ public override void ExecuteCmdlet()`
`416`	`416`	`}`
`417`	`417`	`else`
`418`	`418`	`{`
`419`		`- WriteWarning(Resources.SuggestToUseDeviceCodeAuth);`
	`419`	`+ if (ParameterSetName == UserParameterSet && UseDeviceAuthentication == false)`
	`420`	`+ {`
	`421`	`+ //Display only if user is using Interactive auth`
	`422`	`+ WriteWarning(Resources.SuggestToUseDeviceCodeAuth);`
	`423`	`+ }`
`420`	`424`	`WriteDebug(ex.ToString());`
`421`	`425`	`throw;`
`422`	`426`	`}`
`@@ -474,23 +478,24 @@ private void SetContextWithOverwritePrompt(Action<AzureRmProfile, RMProfileClien`
`474`	`478`	`originalShouldRefreshContextsFromCache = profile.ShouldRefreshContextsFromCache;`
`475`	`479`	`profile.ShouldRefreshContextsFromCache = false;`
`476`	`480`	`}`
`477`		`- if (!CheckForExistingContext(profile, name)`
`478`		`- \|\| Force.IsPresent`
`479`		`- \|\| ShouldContinue(string.Format(Resources.ReplaceContextQuery, name),`
`480`		`- string.Format(Resources.ReplaceContextCaption, name)))`
`481`		`- {`
`482`		`- ModifyContext((prof, client) => setContextAction(prof, client, name));`
	`481`	`+ if (!CheckForExistingContext(profile, name)`
	`482`	`+ \|\| Force.IsPresent`
	`483`	`+ \|\| ShouldContinue(string.Format(Resources.ReplaceContextQuery, name),`
	`484`	`+ string.Format(Resources.ReplaceContextCaption, name)))`
	`485`	`+ {`
	`486`	`+ ModifyContext((prof, client) => setContextAction(prof, client, name));`
	`487`	`+ }`
`483`	`488`	`}`
`484`		`- }`
`485`	`489`	`finally`
`486`	`490`	`{`
`487`		`- if(profile != null && originalShouldRefreshContextsFromCache.HasValue)`
	`491`	`+ if (profile != null && originalShouldRefreshContextsFromCache.HasValue)`
`488`	`492`	`{`
`489`	`493`	`profile.ShouldRefreshContextsFromCache = originalShouldRefreshContextsFromCache.Value;`
`490`	`494`	`}`
`491`	`495`	`}`
`492`	`496`	`}`
`493`	`497`
	`498`	`+ //This method may throw exception because of permission issue, exception should be handled from caller`
`494`	`499`	`private static IAzureContextContainer GetAzureContextContainer()`
`495`	`500`	`{`
`496`	`501`	`var provider = new ProtectedProfileProvider();`
`@@ -507,7 +512,7 @@ public void OnImport()`
`507`	`512`	`{`
`508`	`513`	`#endif`
`509`	`514`	`AzureSessionInitializer.InitializeAzureSession();`
`510`		`- AzureSessionInitializer.MigrateAdalCache(AzureSession.Instance, GetAzureContextContainer);`
	`515`	`+ AzureSessionInitializer.MigrateAdalCache(AzureSession.Instance, GetAzureContextContainer, WriteInitializationWarnings);`
`511`	`516`	`#if DEBUG`
`512`	`517`	`if (!TestMockSupport.RunningMocked)`
`513`	`518`	`{`
`@@ -525,11 +530,21 @@ public void OnImport()`
`525`	`530`	`autoSaveEnabled = localAutosave;`
`526`	`531`	`}`
`527`	`532`
`528`		`- if (autoSaveEnabled && !SharedTokenCacheProvider.SupportCachePersistence(out string message))`
	`533`	`+ try`
	`534`	`+ {`
	`535`	`+ if (autoSaveEnabled && !SharedTokenCacheProvider.SupportCachePersistence(out string message))`
	`536`	`+ {`
	`537`	`+ // If token cache persistence is not supported, fall back to plain text persistence, and print a warning`
	`538`	`+ // We cannot just throw an exception here because this is called when importing the module`
	`539`	`+ WriteInitializationWarnings(Resources.TokenCacheEncryptionNotSupportedWithFallback);`
	`540`	`+ }`
	`541`	`+ }`
	`542`	`+ catch(Exception ex)`
`529`	`543`	`{`
`530`		`- // If token cache persistence is not supported, fall back to plain text persistence, and print a warning`
`531`		`- // We cannot just throw an exception here because this is called when importing the module`
`532`		`- WriteInitializationWarnings(Resources.TokenCacheEncryptionNotSupportedWithFallback);`
	`544`	`+ //Likely the exception is related permission, fall back context save mode to process`
	`545`	`+ autoSaveEnabled = false;`
	`546`	`+ AzureSession.Instance.ARMContextSaveMode = ContextSaveMode.Process;`
	`547`	`+ WriteInitializationWarnings(Resources.FallbackContextSaveModeDueCacheCheckError.FormatInvariant(ex.Message));`
`533`	`548`	`}`
`534`	`549`
`535`	`550`	`if(!InitializeProfileProvider(autoSaveEnabled))`
Original file line number	Diff line number	Diff line change
`@@ -127,12 +127,9 @@ internal bool InitializeProfileProvider(bool useAutoSaveProfile = false)`
`127`	`127`	`{`
`128`	`128`	`ProtectedProfileProvider.InitializeResourceManagerProfile();`
`129`	`129`	`}`
`130`		`- catch (SystemException e)`
	`130`	`+ catch (Exception e)`
`131`	`131`	`{`
`132`		`- if (!(e is IOException) && !(e is UnauthorizedAccessException))`
`133`		`- {`
`134`		`- throw e;`
`135`		`- }`
	`132`	`+ //Likely the exception is related to IO or permission, fallback to Process save mode`
`136`	`133`	`WriteInitializationWarnings(string.Format(Resources.ProfileFileNotAccessible, e.Message));`
`137`	`134`	`ResourceManagerProfileProvider.InitializeResourceManagerProfile(true);`
`138`	`135`	`}`