Updated authorization to the latest agent workflow. (#197)

CLOSE
https://linear.app/sourcegraph/issue/CODY-4620/cody-does-not-stay-signed-in-after-closing-visual-studio

1. Server endpoint and access token is no longer set via `ExtensionConfiguration`, because in a specific scenario, it could cause invalidating an access token when VS is restarted (probably agent related bug when migrated to the new secret storage driven sign in workflow).
2. Fixed NRE in the `SecretStorageService` when `Set()` method is called with empty/null key (not allowed by `IVsCredentialStorageService` implementation).
3. Moved event `AuthorizationDetailsChanged` from `UserSettingsService` to `SecretStorageService`
  - introduced `IsEndpoint()` as a heuristic logic to detect if the agent changed account and/or access token.
4. Removed code handling `signout/signin` commands from `HandleAuthCommand()` - the agent handles this workflows themselves by accessing secret storage.
5. Added logging to `SecretStorageService` and try/catch statements.
6. Updated UI tests infrastructure to allow setting access token explicitly during run-time.

## Test plan

1. Test sign in / sign out workflows
7. Check if it's still in logged state when Visual Studio is restarted

src/Cody.AgentTester/Program.cs

@@ -30,7 +30,7 @@ static async Task Main(string[] args)
             var portNumber = int.TryParse(devPort, out int port) ? port : 3113;
 
             var logger = new Logger();
-            var secretStorageService = new SecretStorageService(new FakeSecretStorageProvider());
+            var secretStorageService = new SecretStorageService(new FakeSecretStorageProvider(), logger);
             var settingsService = new UserSettingsService(new MemorySettingsProvider(), secretStorageService, logger);
             var editorService = new FileService(new FakeServiceProvider(), logger);
             var options = new AgentClientOptions

src/Cody.Core/Agent/Protocol/ExtensionConfiguration.cs

@@ -1,12 +1,15 @@
+using System;
 using System.Collections.Generic;
 
 namespace Cody.Core.Agent.Protocol
 {
     public class ExtensionConfiguration
     {
-        public string ServerEndpoint { get; set; }
+        [Obsolete("Setting the property is obsolete. The agent supports changing it using UI, and use secret storage.")]
+        public string ServerEndpoint { get; set; } 
         public string Proxy { get; set; }
-        public string AccessToken { get; set; }
+        [Obsolete("Setting the property is obsolete. The agent supports changing it using UI, and use secret storage.")]
+        public string AccessToken { get; set; } 
 
         public string AnonymousUserID { get; set; }
 

src/Cody.Core/Agent/WebviewMessageHandler.cs

@@ -47,18 +47,8 @@ public bool HandleMessage(string message)
 
         private bool HandleAuthCommand(dynamic json)
         {
-            if (json.authKind == "signout")
-            {
-                _settingsService.AccessToken = string.Empty;
-            }
-            else if (json.authKind == "signin")
-            {
-                var token = json.value;
-                var endpoint = json.endpoint;
+            // login/logout handled by the agent via accessing secret storage 
 
-                _settingsService.ServerEndpoint = endpoint;
-                _settingsService.AccessToken = token;
-            }
             // Always return false to allow the request to be forwarded to the agent.
             return false;
         }

src/Cody.Core/Infrastructure/ConfigurationService.cs

@@ -69,15 +69,23 @@ public ExtensionConfiguration GetConfiguration()
             var config = new ExtensionConfiguration
             {
                 AnonymousUserID = _userSettingsService.AnonymousUserID,
-                ServerEndpoint = _userSettingsService.ServerEndpoint,
                 Proxy = null,
-                AccessToken = _userSettingsService.AccessToken,
                 AutocompleteAdvancedProvider = null,
                 Debug = Configuration.AgentDebug,
                 VerboseDebug = Configuration.AgentVerboseDebug,
                 CustomConfiguration = GetCustomConfiguration()
             };
 
+            if (_userSettingsService.ForceAccessTokenForUITests)
+            {
+                _logger.Debug($"Detected {nameof(_userSettingsService.ForceAccessTokenForUITests)}");
+
+#pragma warning disable CS0618 // Type or member is obsolete
+                config.ServerEndpoint = _userSettingsService.DefaultServerEndpoint;
+                config.AccessToken = _userSettingsService.AccessToken;
+#pragma warning restore CS0618 // Type or member is obsolete
+            }
+
             return config;
         }
 

src/Cody.Core/Infrastructure/ISecretStorageService.cs

@@ -1,3 +1,5 @@
+using System;
+
 namespace Cody.Core.Infrastructure
 {
     public interface ISecretStorageService
@@ -6,5 +8,7 @@ public interface ISecretStorageService
         string Get(string key);
         void Delete(string key);
         string AccessToken { get; set; }
+
+        event EventHandler AuthorizationDetailsChanged;
     }
 }

src/Cody.Core/Settings/IUserSettingsService.cs

@@ -7,12 +7,10 @@ public interface IUserSettingsService
         string AnonymousUserID { get; set; }
 
         string AccessToken { get; set; }
-        string ServerEndpoint { get; set; }
+        string DefaultServerEndpoint { get; }
         string CustomConfiguration { get; set; }
         bool AcceptNonTrustedCert { get; set; }
         bool AutomaticallyTriggerCompletions { get; set; }
-
-
-        event EventHandler AuthorizationDetailsChanged;
+        bool ForceAccessTokenForUITests { get; set; }
     }
 }

src/Cody.Core/Settings/UserSettingsService.cs

@@ -10,8 +10,6 @@ public class UserSettingsService : IUserSettingsService
         private readonly ISecretStorageService _secretStorage;
         private readonly ILog _logger;
 
-        public event EventHandler AuthorizationDetailsChanged;
-
         public UserSettingsService(IUserSettingsProvider settingsProvider, ISecretStorageService secretStorage, ILog log)
         {
             _settingsProvider = settingsProvider;
@@ -55,19 +53,7 @@ public string AnonymousUserID
             set => Set(nameof(AnonymousUserID), value);
         }
 
-        public string ServerEndpoint
-        {
-            get => GetOrDefault(nameof(ServerEndpoint), "https://sourcegraph.com/");
-            set
-            {
-                var endpoint = GetOrDefault(nameof(ServerEndpoint));
-                if (!string.Equals(value, endpoint, StringComparison.InvariantCulture))
-                {
-                    Set(nameof(ServerEndpoint), value);
-                    AuthorizationDetailsChanged?.Invoke(this, EventArgs.Empty);
-                }
-            }
-        }
+        public string DefaultServerEndpoint => "https://sourcegraph.com/";
 
         public string AccessToken
         {
@@ -86,11 +72,13 @@ public string AccessToken
             }
             set
             {
+                if (!ForceAccessTokenForUITests)
+                    throw new Exception("Setting access token explicitly only allowed when running UI tests!");
+
                 var userToken = _secretStorage.AccessToken;
                 if (!string.Equals(value, userToken, StringComparison.InvariantCulture))
                 {
                     _secretStorage.AccessToken = value;
-                    AuthorizationDetailsChanged?.Invoke(this, EventArgs.Empty);
                 }
             }
         }
@@ -120,5 +108,7 @@ public bool AutomaticallyTriggerCompletions
             }
             set => Set(nameof(AutomaticallyTriggerCompletions), value.ToString());
         }
+
+        public bool ForceAccessTokenForUITests { get; set; }
     }
 }

src/Cody.VisualStudio.Tests/PlaywrightTestsBase.cs

@@ -58,6 +58,7 @@ private async Task InitializeAsync()
                 if (accessToken != null)
                 {
                     WriteLog("Using Access Token.");
+                    CodyPackage.UserSettingsService.ForceAccessTokenForUITests = true;
                     CodyPackage.UserSettingsService.AccessToken = accessToken;
                 }
 

src/Cody.VisualStudio/CodyPackage.cs

@@ -118,9 +118,9 @@ private void InitializeServices()
             VsVersionService = new VsVersionService(Logger);
 
             var vsSecretStorage = this.GetService<SVsCredentialStorageService, IVsCredentialStorageService>();
-            SecretStorageService = new SecretStorageService(vsSecretStorage);
+            SecretStorageService = new SecretStorageService(vsSecretStorage, Logger);
             UserSettingsService = new UserSettingsService(new UserSettingsProvider(this), SecretStorageService, Logger);
-            UserSettingsService.AuthorizationDetailsChanged += AuthorizationDetailsChanged;
+            SecretStorageService.AuthorizationDetailsChanged += AuthorizationDetailsChanged;
 
             ConfigurationService = new ConfigurationService(VersionService, VsVersionService, SolutionService, UserSettingsService, Logger);
 
@@ -203,7 +203,12 @@ private async void AuthorizationDetailsChanged(object sender, EventArgs eventArg
         {
             try
             {
-                Logger.Debug($"Changing authorization details ...");
+                Logger.Debug($"Checking authorization status ...");
+                if (ConfigurationService == null || AgentService == null)
+                {
+                    Logger.Debug("Not changed.");
+                    return;
+                }
 
                 var config = ConfigurationService.GetConfiguration();
                 var status = await AgentService.ConfigurationChange(config);
@@ -334,7 +339,7 @@ private void InitializeAgent()
                 {
                     try
                     {
-                        await TestServerConnection(UserSettingsService.ServerEndpoint);
+                        await TestServerConnection(UserSettingsService.DefaultServerEndpoint);
                         AgentClient.Start();
 
                         var clientConfig = ConfigurationService.GetClientInfo();

src/Cody.VisualStudio/Services/SecretStorageService.cs

@@ -1,50 +1,123 @@
+using System;
 using Cody.Core.Infrastructure;
+using Cody.Core.Logging;
 using Microsoft.VisualStudio.Shell.Connected.CredentialStorage;
 
 namespace Cody.VisualStudio.Services
 {
     public class SecretStorageService : ISecretStorageService
     {
-        private readonly IVsCredentialStorageService secretStorageService;
+        private readonly IVsCredentialStorageService _secretStorageService;
+        private readonly ILog _logger;
         private readonly string AccessTokenKey = "cody.access-token";
         private readonly string FeatureName = "Cody.VisualStudio";
         private readonly string UserName = "CodyAgent";
         private readonly string Type = "token";
 
-        public SecretStorageService(IVsCredentialStorageService secretStorageService)
+        public event EventHandler AuthorizationDetailsChanged;
+
+        public SecretStorageService(IVsCredentialStorageService secretStorageService, ILog logger)
         {
-            this.secretStorageService = secretStorageService;
+            _secretStorageService = secretStorageService;
+            _logger = logger;
         }
 
         public string Get(string key)
         {
-            var credentialKey = this.secretStorageService.CreateCredentialKey(FeatureName, key, UserName, Type);
-            var credential = this.secretStorageService.Retrieve(credentialKey);
-            return credential?.TokenValue;
+            try
+            {
+                var credentialKey = _secretStorageService.CreateCredentialKey(FeatureName, key, UserName, Type);
+                var credential = _secretStorageService.Retrieve(credentialKey);
+
+                var value = credential?.TokenValue;
+                //_logger.Debug($"Get '{key}':{value}");
+
+                if (IsEndpoint(key))
+                    AuthorizationDetailsChanged?.Invoke(this, EventArgs.Empty);
+
+                return value;
+            }
+            catch (Exception ex)
+            {
+                _logger.Error($"Cannot get key: '{key}'", ex);
+            }
+
+            return null;
 
         }
 
+        private bool IsEndpoint(string key)
+        {
+            try
+            {
+                var isUri = IsValidUri(key);
+                if (isUri)
+                {
+                    _logger.Debug($"Detected Uri:'{key}'");
+                    return true;
+                }
+            }
+            catch (Exception ex)
+            {
+                _logger.Error("Failed.", ex);
+            }
+
+            return false;
+        }
+        private bool IsValidUri(string uriString)
+        {
+            return Uri.TryCreate(uriString, UriKind.Absolute, out _);
+        }
+
         public void Set(string key, string value)
         {
-            var credentialKey = this.secretStorageService.CreateCredentialKey(FeatureName, key, UserName, Type);
-            this.secretStorageService.Add(credentialKey, value);
+            try
+            {
+                if (string.IsNullOrEmpty(value))
+                {
+                    // cannot be an empty string ("") or start with the null character
+                    value = "NULL";
+                }
+
+                var credentialKey = _secretStorageService.CreateCredentialKey(FeatureName, key, UserName, Type);
+                _secretStorageService.Add(credentialKey, value);
+
+                //_logger.Debug($"Set '{key}':{value}");
+            }
+            catch (Exception ex)
+            {
+                _logger.Error($"Cannot set key: '{key}'", ex);
+            }
         }
 
         public void Delete(string key)
         {
-            var credentialKey = this.secretStorageService.CreateCredentialKey(FeatureName, key, UserName, Type);
-            this.secretStorageService.Remove(credentialKey);
+            try
+            {
+                var credentialKey = _secretStorageService.CreateCredentialKey(FeatureName, key, UserName, Type);
+                _secretStorageService.Remove(credentialKey);
+
+                _logger.Debug($"Remove '{key}'");
+            }
+            catch (Exception ex)
+            {
+                _logger.Error($"Cannot delete key: '{key}'", ex);
+            }
         }
 
         public string AccessToken
         {
             get
             {
-                return this.Get(AccessTokenKey);
+                return Get(AccessTokenKey);
             }
             set
             {
-                this.Set(AccessTokenKey, value);
+                var oldAccessToken = AccessToken;
+                Set(AccessTokenKey, value);
+
+                if (oldAccessToken != value)
+                    AuthorizationDetailsChanged?.Invoke(this, EventArgs.Empty);
             }
         }
     }