From 4352eaed1348493b16afcc3d59f6b657b587b87f Mon Sep 17 00:00:00 2001 From: Piotr Karczmarz Date: Thu, 16 Jan 2025 17:37:42 +0100 Subject: [PATCH 1/3] Updated authorization to the latest agent workflow. --- src/Cody.AgentTester/Program.cs | 2 +- .../Agent/Protocol/ExtensionConfiguration.cs | 7 +- src/Cody.Core/Agent/WebviewMessageHandler.cs | 12 +-- .../Infrastructure/ConfigurationService.cs | 2 - .../Infrastructure/ISecretStorageService.cs | 4 + .../Settings/IUserSettingsService.cs | 5 +- src/Cody.Core/Settings/UserSettingsService.cs | 17 +--- src/Cody.VisualStudio/CodyPackage.cs | 13 ++- .../Services/SecretStorageService.cs | 93 ++++++++++++++++--- 9 files changed, 103 insertions(+), 52 deletions(-) diff --git a/src/Cody.AgentTester/Program.cs b/src/Cody.AgentTester/Program.cs index f7a8ef3c..a579fce7 100644 --- a/src/Cody.AgentTester/Program.cs +++ b/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 diff --git a/src/Cody.Core/Agent/Protocol/ExtensionConfiguration.cs b/src/Cody.Core/Agent/Protocol/ExtensionConfiguration.cs index aa55a3df..2c1c165c 100644 --- a/src/Cody.Core/Agent/Protocol/ExtensionConfiguration.cs +++ b/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; } diff --git a/src/Cody.Core/Agent/WebviewMessageHandler.cs b/src/Cody.Core/Agent/WebviewMessageHandler.cs index 0153fca3..17439d3d 100644 --- a/src/Cody.Core/Agent/WebviewMessageHandler.cs +++ b/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; } diff --git a/src/Cody.Core/Infrastructure/ConfigurationService.cs b/src/Cody.Core/Infrastructure/ConfigurationService.cs index 5b1b74b6..9416225a 100644 --- a/src/Cody.Core/Infrastructure/ConfigurationService.cs +++ b/src/Cody.Core/Infrastructure/ConfigurationService.cs @@ -68,9 +68,7 @@ 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, diff --git a/src/Cody.Core/Infrastructure/ISecretStorageService.cs b/src/Cody.Core/Infrastructure/ISecretStorageService.cs index e4aae4bf..32fca9cf 100644 --- a/src/Cody.Core/Infrastructure/ISecretStorageService.cs +++ b/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; } } diff --git a/src/Cody.Core/Settings/IUserSettingsService.cs b/src/Cody.Core/Settings/IUserSettingsService.cs index ad8125ae..b8bb845d 100644 --- a/src/Cody.Core/Settings/IUserSettingsService.cs +++ b/src/Cody.Core/Settings/IUserSettingsService.cs @@ -7,12 +7,9 @@ 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; } } diff --git a/src/Cody.Core/Settings/UserSettingsService.cs b/src/Cody.Core/Settings/UserSettingsService.cs index 8d0e1c33..21c9b483 100644 --- a/src/Cody.Core/Settings/UserSettingsService.cs +++ b/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 { @@ -90,7 +76,6 @@ public string AccessToken if (!string.Equals(value, userToken, StringComparison.InvariantCulture)) { _secretStorage.AccessToken = value; - AuthorizationDetailsChanged?.Invoke(this, EventArgs.Empty); } } } diff --git a/src/Cody.VisualStudio/CodyPackage.cs b/src/Cody.VisualStudio/CodyPackage.cs index ac41d61c..ad28eac1 100644 --- a/src/Cody.VisualStudio/CodyPackage.cs +++ b/src/Cody.VisualStudio/CodyPackage.cs @@ -118,9 +118,9 @@ private void InitializeServices() VsVersionService = new VsVersionService(Logger); var vsSecretStorage = this.GetService(); - 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); @@ -202,7 +202,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); @@ -333,7 +338,7 @@ private void InitializeAgent() { try { - await TestServerConnection(UserSettingsService.ServerEndpoint); + await TestServerConnection(UserSettingsService.DefaultServerEndpoint); AgentClient.Start(); var clientConfig = ConfigurationService.GetClientInfo(); diff --git a/src/Cody.VisualStudio/Services/SecretStorageService.cs b/src/Cody.VisualStudio/Services/SecretStorageService.cs index 006dba19..bf74643f 100644 --- a/src/Cody.VisualStudio/Services/SecretStorageService.cs +++ b/src/Cody.VisualStudio/Services/SecretStorageService.cs @@ -1,50 +1,119 @@ +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); + Set(AccessTokenKey, value); } } } From cc781f6d375b587d6aa1c5f1bc538385c9e21a2f Mon Sep 17 00:00:00 2001 From: Piotr Karczmarz Date: Fri, 17 Jan 2025 15:56:08 +0100 Subject: [PATCH 2/3] Allow to set access token explicitly when running UI tests. --- src/Cody.Core/Infrastructure/ConfigurationService.cs | 7 +++++++ src/Cody.Core/Settings/IUserSettingsService.cs | 1 + src/Cody.Core/Settings/UserSettingsService.cs | 5 +++++ src/Cody.VisualStudio.Tests/PlaywrightTestsBase.cs | 1 + src/Cody.VisualStudio/Services/SecretStorageService.cs | 4 ++++ 5 files changed, 18 insertions(+) diff --git a/src/Cody.Core/Infrastructure/ConfigurationService.cs b/src/Cody.Core/Infrastructure/ConfigurationService.cs index 9416225a..1546bca6 100644 --- a/src/Cody.Core/Infrastructure/ConfigurationService.cs +++ b/src/Cody.Core/Infrastructure/ConfigurationService.cs @@ -75,6 +75,13 @@ public ExtensionConfiguration GetConfiguration() CustomConfiguration = GetCustomConfiguration() }; + if (_userSettingsService.ForceAccessTokenForUITests) + { +#pragma warning disable CS0618 // Type or member is obsolete + config.AccessToken = _userSettingsService.AccessToken; +#pragma warning restore CS0618 // Type or member is obsolete + } + return config; } diff --git a/src/Cody.Core/Settings/IUserSettingsService.cs b/src/Cody.Core/Settings/IUserSettingsService.cs index b8bb845d..753acd55 100644 --- a/src/Cody.Core/Settings/IUserSettingsService.cs +++ b/src/Cody.Core/Settings/IUserSettingsService.cs @@ -11,5 +11,6 @@ public interface IUserSettingsService string CustomConfiguration { get; set; } bool AcceptNonTrustedCert { get; set; } bool AutomaticallyTriggerCompletions { get; set; } + bool ForceAccessTokenForUITests { get; set; } } } diff --git a/src/Cody.Core/Settings/UserSettingsService.cs b/src/Cody.Core/Settings/UserSettingsService.cs index 21c9b483..d330df2c 100644 --- a/src/Cody.Core/Settings/UserSettingsService.cs +++ b/src/Cody.Core/Settings/UserSettingsService.cs @@ -72,6 +72,9 @@ 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)) { @@ -105,5 +108,7 @@ public bool AutomaticallyTriggerCompletions } set => Set(nameof(AutomaticallyTriggerCompletions), value.ToString()); } + + public bool ForceAccessTokenForUITests { get; set; } } } diff --git a/src/Cody.VisualStudio.Tests/PlaywrightTestsBase.cs b/src/Cody.VisualStudio.Tests/PlaywrightTestsBase.cs index 06fbf398..9cd3120e 100644 --- a/src/Cody.VisualStudio.Tests/PlaywrightTestsBase.cs +++ b/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; } diff --git a/src/Cody.VisualStudio/Services/SecretStorageService.cs b/src/Cody.VisualStudio/Services/SecretStorageService.cs index bf74643f..eee3bc78 100644 --- a/src/Cody.VisualStudio/Services/SecretStorageService.cs +++ b/src/Cody.VisualStudio/Services/SecretStorageService.cs @@ -113,7 +113,11 @@ public string AccessToken } set { + var oldAccessToken = AccessToken; Set(AccessTokenKey, value); + + if (oldAccessToken != value) + AuthorizationDetailsChanged?.Invoke(this, EventArgs.Empty); } } } From b0e8267f2988afc48bdfea650533a6cfa9129cc9 Mon Sep 17 00:00:00 2001 From: Piotr Karczmarz Date: Fri, 17 Jan 2025 16:38:27 +0100 Subject: [PATCH 3/3] Added setting default server endpoint for UI Tests. --- src/Cody.Core/Infrastructure/ConfigurationService.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Cody.Core/Infrastructure/ConfigurationService.cs b/src/Cody.Core/Infrastructure/ConfigurationService.cs index 1546bca6..51f65085 100644 --- a/src/Cody.Core/Infrastructure/ConfigurationService.cs +++ b/src/Cody.Core/Infrastructure/ConfigurationService.cs @@ -77,7 +77,10 @@ public ExtensionConfiguration GetConfiguration() 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 }