From 77b3b18e5b473246fd1edd6652e87b4982d7afaa Mon Sep 17 00:00:00 2001 From: cormacpayne Date: Fri, 7 Sep 2018 16:53:02 -0700 Subject: [PATCH 1/4] Allow for SPN context persistence in .NET Core --- .../AzureAccount.cs | 10 +- ...hentication.ResourceManager.Netcore.csproj | 2 + .../Authentication.ResourceManager.csproj | 4 + ...AzureRmServicePrincipalKeyStore.Netcore.cs | 112 ++++++++++++++++++ .../CredStore.cs | 6 +- .../ServicePrincipalKeyStore.cs | 11 +- .../Authentication.Test.csproj | 4 + .../AuthenticationFactoryTests.cs | 3 +- src/Authentication.Test/AzureSessionTests.cs | 1 + .../Cmdlets/ConnectAccount.cs | 5 + src/Authentication.Test/LoginTests.cs | 4 +- src/Authentication/Authentication.csproj | 3 +- .../Authentication/AdalTokenProvider.cs | 37 ++++-- .../KeyStoreApplicationCredentialProvider.cs | 14 ++- .../ServicePrincipalTokenProvider.cs | 32 ++++- .../Factories/AuthenticationFactory.cs | 46 +++++-- .../KeyStore/IServicePrincipalKeyStore.cs | 55 +++++++++ .../ServicePrincipalKeyStore.cs} | 14 +-- src/Common.Test/Common.Test.csproj | 4 + .../Common/ServicePrincipalStoreTests.cs | 32 ++--- 20 files changed, 339 insertions(+), 60 deletions(-) create mode 100644 src/Authentication.ResourceManager/AzureRmServicePrincipalKeyStore.Netcore.cs rename src/{Authentication/Authentication => Authentication.ResourceManager}/CredStore.cs (96%) rename src/{Authentication/Authentication => Authentication.ResourceManager}/ServicePrincipalKeyStore.cs (91%) create mode 100644 src/Authentication/KeyStore/IServicePrincipalKeyStore.cs rename src/Authentication/{Authentication/ServicePrincipalKeyStore.Netcore.cs => KeyStore/ServicePrincipalKeyStore.cs} (82%) diff --git a/src/Authentication.Abstractions/AzureAccount.cs b/src/Authentication.Abstractions/AzureAccount.cs index 510c13b34d..934d3b77c6 100644 --- a/src/Authentication.Abstractions/AzureAccount.cs +++ b/src/Authentication.Abstractions/AzureAccount.cs @@ -124,17 +124,21 @@ public static class Property /// Login Uri for Managed Service Login /// MSILoginUri = "MSILoginUri", - + /// /// Backup login Uri for MSI /// MSILoginUriBackup = "MSILoginBackup", - /// /// Secret that may be used with MSI login /// - MSILoginSecret = "MSILoginSecret"; + MSILoginSecret = "MSILoginSecret", + + /// + /// Secret that may be used with service principal login + /// + ServicePrincipalSecret = "ServicePrincipalSecret"; } } } diff --git a/src/Authentication.ResourceManager/Authentication.ResourceManager.Netcore.csproj b/src/Authentication.ResourceManager/Authentication.ResourceManager.Netcore.csproj index ca576f4977..4927902c29 100644 --- a/src/Authentication.ResourceManager/Authentication.ResourceManager.Netcore.csproj +++ b/src/Authentication.ResourceManager/Authentication.ResourceManager.Netcore.csproj @@ -56,7 +56,9 @@ + + diff --git a/src/Authentication.ResourceManager/Authentication.ResourceManager.csproj b/src/Authentication.ResourceManager/Authentication.ResourceManager.csproj index 8a6004bade..85f35f8a71 100644 --- a/src/Authentication.ResourceManager/Authentication.ResourceManager.csproj +++ b/src/Authentication.ResourceManager/Authentication.ResourceManager.csproj @@ -28,6 +28,7 @@ 4 true true + true pdbonly @@ -46,6 +47,7 @@ true true false + true @@ -53,6 +55,7 @@ + @@ -76,6 +79,7 @@ + diff --git a/src/Authentication.ResourceManager/AzureRmServicePrincipalKeyStore.Netcore.cs b/src/Authentication.ResourceManager/AzureRmServicePrincipalKeyStore.Netcore.cs new file mode 100644 index 0000000000..9318d1488a --- /dev/null +++ b/src/Authentication.ResourceManager/AzureRmServicePrincipalKeyStore.Netcore.cs @@ -0,0 +1,112 @@ +// ---------------------------------------------------------------------------------- +// +// Copyright Microsoft Corporation +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ---------------------------------------------------------------------------------- +using Microsoft.Azure.Commands.Common.Authentication; +using Microsoft.Azure.Commands.Common.Authentication.Abstractions; +using Microsoft.Azure.Commands.Common.Authentication.Models; +using Microsoft.Azure.Commands.Common.Authentication.ResourceManager; +using System; +using System.Collections; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Security; + +namespace Microsoft.Azure.Commands.ResourceManager.Common +{ + /// + /// Helper class to store service principal keys and retrieve them + /// from the Windows Credential Store. + /// + public class AzureRmServicePrincipalKeyStore : IServicePrincipalKeyStore + { + public const string Name = "ServicePrincipalKeyStore"; + private IDictionary _credentials { get; set; } + + public AzureRmServicePrincipalKeyStore() : this(null) { } + + public AzureRmServicePrincipalKeyStore(IAzureContextContainer profile) + { + _credentials = new Dictionary(); + if (profile != null && profile.Accounts != null) + { + foreach (var account in profile.Accounts) + { + if (account != null && account.ExtendedProperties.ContainsKey(AzureAccount.Property.ServicePrincipalSecret)) + { + var appId = account.Id; + var tenantId = account.GetTenants().FirstOrDefault(); + var key = CreateKey(appId, tenantId); + var servicePrincipalSecret = account.ExtendedProperties[AzureAccount.Property.ServicePrincipalSecret]; + _credentials[key] = ConvertToSecureString(servicePrincipalSecret); + } + } + } + } + + public void SaveKey(string appId, string tenantId, SecureString serviceKey) + { + var key = CreateKey(appId, tenantId); + _credentials[key] = serviceKey; + } + + public SecureString GetKey(string appId, string tenantId) + { + IntPtr pCredential = IntPtr.Zero; + try + { + var key = CreateKey(appId, tenantId); + return _credentials[key]; + + } + catch + { + // we could be running in an environment that does not have credentials store + } + + return null; + } + + + public void DeleteKey(string appId, string tenantId) + { + try + { + var key = CreateKey(appId, tenantId); + _credentials.Remove(key); + } + catch + { + } + } + + private string CreateKey(string appId, string tenantId) + { + return $"{appId}_{tenantId}"; + } + + internal SecureString ConvertToSecureString(string password) + { + if (password == null) + throw new ArgumentNullException("password"); + + var securePassword = new SecureString(); + + foreach (char c in password) + securePassword.AppendChar(c); + + securePassword.MakeReadOnly(); + return securePassword; + } + } +} \ No newline at end of file diff --git a/src/Authentication/Authentication/CredStore.cs b/src/Authentication.ResourceManager/CredStore.cs similarity index 96% rename from src/Authentication/Authentication/CredStore.cs rename to src/Authentication.ResourceManager/CredStore.cs index f53247a800..9fa4a0739f 100644 --- a/src/Authentication/Authentication/CredStore.cs +++ b/src/Authentication.ResourceManager/CredStore.cs @@ -16,7 +16,7 @@ using System.Runtime.ConstrainedExecution; using System.Runtime.InteropServices; -namespace Microsoft.Azure.Commands.Common.Authentication +namespace Microsoft.Azure.Commands.ResourceManager.Common { /// /// Class wrapping PInvoke signatures for Windows Credential store @@ -81,7 +81,7 @@ public Credential(string userName, string key, string value) this.flags = 0; this.type = CredentialType.Generic; - // set the key in the targetName + // set the key in the targetName this.targetName = key; this.targetAlias = null; @@ -89,7 +89,7 @@ public Credential(string userName, string key, string value) this.lastWritten.dwHighDateTime = 0; this.lastWritten.dwLowDateTime = 0; - // set the value in credentialBlob. + // set the value in credentialBlob. this.credentialBlob = Marshal.StringToHGlobalUni(value); this.credentialBlobSize = (uint)((value.Length + 1) * 2); diff --git a/src/Authentication/Authentication/ServicePrincipalKeyStore.cs b/src/Authentication.ResourceManager/ServicePrincipalKeyStore.cs similarity index 91% rename from src/Authentication/Authentication/ServicePrincipalKeyStore.cs rename to src/Authentication.ResourceManager/ServicePrincipalKeyStore.cs index 09261537da..4d3c9deaf0 100644 --- a/src/Authentication/Authentication/ServicePrincipalKeyStore.cs +++ b/src/Authentication.ResourceManager/ServicePrincipalKeyStore.cs @@ -12,23 +12,24 @@ // limitations under the License. // ---------------------------------------------------------------------------------- +using Microsoft.Azure.Commands.Common.Authentication; using System; using System.Runtime.InteropServices; using System.Security; using FILETIME = System.Runtime.InteropServices.ComTypes.FILETIME; -namespace Microsoft.Azure.Commands.Common.Authentication +namespace Microsoft.Azure.Commands.ResourceManager.Common { /// /// Helper class to store service principal keys and retrieve them /// from the Windows Credential Store. /// - public static class ServicePrincipalKeyStore + public class ServicePrincipalKeyStore : IServicePrincipalKeyStore { private const string keyStoreUserName = "PowerShellServicePrincipalKey"; private const string targetNamePrefix = "AzureSession:target="; - public static void SaveKey(string appId, string tenantId, SecureString serviceKey) + public void SaveKey(string appId, string tenantId, SecureString serviceKey) { var credential = new CredStore.NativeMethods.Credential { @@ -68,7 +69,7 @@ public static void SaveKey(string appId, string tenantId, SecureString serviceKe } } - public static SecureString GetKey(string appId, string tenantId) + public SecureString GetKey(string appId, string tenantId) { IntPtr pCredential = IntPtr.Zero; try @@ -104,7 +105,7 @@ public static SecureString GetKey(string appId, string tenantId) } - public static void DeleteKey(string appId, string tenantId) + public void DeleteKey(string appId, string tenantId) { CredStore.NativeMethods.CredDelete(CreateKey(appId, tenantId), CredStore.CredentialType.Generic, 0); } diff --git a/src/Authentication.Test/Authentication.Test.csproj b/src/Authentication.Test/Authentication.Test.csproj index ff9e2ccafc..3d10f483cf 100644 --- a/src/Authentication.Test/Authentication.Test.csproj +++ b/src/Authentication.Test/Authentication.Test.csproj @@ -63,6 +63,10 @@ + + {69c2eb6b-cd63-480a-89a0-c489706e9299} + Authentication.ResourceManager + {3819d8a7-c62c-4c47-8ddd-0332d9ce1252} ResourceManager diff --git a/src/Authentication.Test/AuthenticationFactoryTests.cs b/src/Authentication.Test/AuthenticationFactoryTests.cs index 79a798f04b..b25383b60e 100644 --- a/src/Authentication.Test/AuthenticationFactoryTests.cs +++ b/src/Authentication.Test/AuthenticationFactoryTests.cs @@ -26,6 +26,7 @@ using Microsoft.WindowsAzure.Commands.Utilities.Common; using Xunit.Abstractions; using Microsoft.Rest.Azure; +using Microsoft.Azure.Commands.ResourceManager.Common; namespace Common.Authentication.Test { @@ -75,7 +76,7 @@ public void VerifySubscriptionTokenCacheRemove() [Trait(Category.AcceptanceType, Category.CheckIn)] public void VerifyValidateAuthorityFalseForOnPremise() { - AzureSessionInitializer.InitializeAzureSession(); + AzureSessionInitializer.InitializeAzureSession(new ServicePrincipalKeyStore()); var authFactory = new AuthenticationFactory { TokenProvider = new MockAccessTokenProvider("testtoken", "testuser") diff --git a/src/Authentication.Test/AzureSessionTests.cs b/src/Authentication.Test/AzureSessionTests.cs index 85651e25a1..d9d5c95a96 100644 --- a/src/Authentication.Test/AzureSessionTests.cs +++ b/src/Authentication.Test/AzureSessionTests.cs @@ -23,6 +23,7 @@ using Microsoft.Azure.Commands.Common.Authentication.Abstractions; using System.IO; using Newtonsoft.Json; +using Microsoft.Azure.Commands.ResourceManager.Common; namespace Common.Authentication.Test { diff --git a/src/Authentication.Test/Cmdlets/ConnectAccount.cs b/src/Authentication.Test/Cmdlets/ConnectAccount.cs index b7f2a948ea..8709c51b6b 100644 --- a/src/Authentication.Test/Cmdlets/ConnectAccount.cs +++ b/src/Authentication.Test/Cmdlets/ConnectAccount.cs @@ -94,6 +94,11 @@ public override void ExecuteCmdlet() Account.SetProperty(AzureAccount.Property.Tenants, new[] { TenantId }); } + if (!string.IsNullOrEmpty(Password)) + { + Account.SetProperty(AzureAccount.Property.ServicePrincipalSecret, Password); + } + if (AzureRmProfileProvider.Instance.Profile == null) { ResourceManagerProfileProvider.InitializeResourceManagerProfile(true); diff --git a/src/Authentication.Test/LoginTests.cs b/src/Authentication.Test/LoginTests.cs index 7f482f50b4..d75466129f 100644 --- a/src/Authentication.Test/LoginTests.cs +++ b/src/Authentication.Test/LoginTests.cs @@ -51,7 +51,9 @@ public class LoginTests public LoginTests() { AzureSessionInitializer.InitializeAzureSession(); - ResourceManagerProfileProvider.InitializeResourceManagerProfile(); + ProtectedProfileProvider.InitializeResourceManagerProfile(); + IServicePrincipalKeyStore keyStore = new AzureRmServicePrincipalKeyStore(AzureRmProfileProvider.Instance.Profile); + AzureSession.Instance.RegisterComponent(ServicePrincipalKeyStore.Name, () => keyStore); ContextAutosaveSettings settings = null; AzureSession.Modify((session) => EnableAutosave(session, true, out settings)); diff --git a/src/Authentication/Authentication.csproj b/src/Authentication/Authentication.csproj index 1d0321b831..8004c73863 100644 --- a/src/Authentication/Authentication.csproj +++ b/src/Authentication/Authentication.csproj @@ -58,7 +58,6 @@ - @@ -67,7 +66,6 @@ - @@ -77,6 +75,7 @@ + diff --git a/src/Authentication/Authentication/AdalTokenProvider.cs b/src/Authentication/Authentication/AdalTokenProvider.cs index 3fd329b5a4..1d66901b71 100644 --- a/src/Authentication/Authentication/AdalTokenProvider.cs +++ b/src/Authentication/Authentication/AdalTokenProvider.cs @@ -41,7 +41,12 @@ public AdalTokenProvider() public AdalTokenProvider(IWin32Window parentWindow) { this.userTokenProvider = new UserTokenProvider(parentWindow); - this.servicePrincipalTokenProvider = new ServicePrincipalTokenProvider(); + } + + public AdalTokenProvider(IServicePrincipalKeyStore keyStore) + { + this.userTokenProvider = new UserTokenProvider(new ConsoleParentWindow()); + this.servicePrincipalTokenProvider = new ServicePrincipalTokenProvider(keyStore); } public IAccessToken GetAccessToken( @@ -76,15 +81,21 @@ public IAccessToken GetAccessTokenWithCertificate( default: throw new ArgumentException(string.Format(Resources.UnsupportedCredentialType, credentialType), "credentialType"); } - } + } #else public AdalTokenProvider() { this.userTokenProvider = new UserTokenProvider(); this.servicePrincipalTokenProvider = new ServicePrincipalTokenProvider(); } - - public IAccessToken GetAccessToken( + + public AdalTokenProvider(Func getKeyStore) + { + this.userTokenProvider = new UserTokenProvider(); + this.servicePrincipalTokenProvider = new ServicePrincipalTokenProvider(getKeyStore); + } + + public IAccessToken GetAccessToken( AdalConfiguration config, string promptBehavior, Action promptAction, @@ -96,19 +107,19 @@ public IAccessToken GetAccessToken( { case AzureAccount.AccountType.User: return userTokenProvider.GetAccessToken( - config, + config, promptBehavior, - promptAction, - userId, - password, + promptAction, + userId, + password, credentialType); case AzureAccount.AccountType.ServicePrincipal: return servicePrincipalTokenProvider.GetAccessToken( - config, - promptBehavior, - promptAction, - userId, - password, + config, + promptBehavior, + promptAction, + userId, + password, credentialType); default: throw new ArgumentException(Resources.UnsupportedCredentialType, "credentialType"); diff --git a/src/Authentication/Authentication/KeyStoreApplicationCredentialProvider.cs b/src/Authentication/Authentication/KeyStoreApplicationCredentialProvider.cs index bb514be818..12a12f98fb 100644 --- a/src/Authentication/Authentication/KeyStoreApplicationCredentialProvider.cs +++ b/src/Authentication/Authentication/KeyStoreApplicationCredentialProvider.cs @@ -28,6 +28,7 @@ namespace Microsoft.Azure.Commands.Common.Authentication internal sealed class KeyStoreApplicationCredentialProvider : IApplicationAuthenticationProvider { private string _tenantId; + private IServicePrincipalKeyStore _keyStore; /// /// Create a credential provider @@ -38,6 +39,17 @@ public KeyStoreApplicationCredentialProvider(string tenant) this._tenantId = tenant; } + /// + /// Create a credential provider + /// + /// + /// + public KeyStoreApplicationCredentialProvider(string tenant, IServicePrincipalKeyStore keyStore) + { + this._tenantId = tenant; + this._keyStore = keyStore; + } + /// /// Authenticate using the secret for the specified client from the key store /// @@ -49,7 +61,7 @@ public async Task AuthenticateAsync(string clientId, strin { var task = new Task(() => { - return ServicePrincipalKeyStore.GetKey(clientId, _tenantId); + return _keyStore.GetKey(clientId, _tenantId); }); task.Start(); var key = await task.ConfigureAwait(false); diff --git a/src/Authentication/Authentication/ServicePrincipalTokenProvider.cs b/src/Authentication/Authentication/ServicePrincipalTokenProvider.cs index 0a7e1522c4..4ac4af4ccd 100644 --- a/src/Authentication/Authentication/ServicePrincipalTokenProvider.cs +++ b/src/Authentication/Authentication/ServicePrincipalTokenProvider.cs @@ -30,6 +30,34 @@ namespace Microsoft.Azure.Commands.Common.Authentication internal class ServicePrincipalTokenProvider : ITokenProvider { private static readonly TimeSpan expirationThreshold = TimeSpan.FromMinutes(5); + private Func _getKeyStore; + private IServicePrincipalKeyStore _keyStore; + + public IServicePrincipalKeyStore KeyStore + { + get + { + if (_keyStore == null) + { + _keyStore = _getKeyStore(); + } + + return _keyStore; + } + set + { + _keyStore = value; + } + } + + public ServicePrincipalTokenProvider() + { + } + + public ServicePrincipalTokenProvider(Func getKeyStore) + { + _getKeyStore = getKeyStore; + } public IAccessToken GetAccessToken( AdalConfiguration config, @@ -143,12 +171,12 @@ private AuthenticationResult RenewWithCertificate( private SecureString LoadAppKey(string appId, string tenantId) { - return ServicePrincipalKeyStore.GetKey(appId, tenantId); + return KeyStore.GetKey(appId, tenantId); } private void StoreAppKey(string appId, string tenantId, SecureString appKey) { - ServicePrincipalKeyStore.SaveKey(appId, tenantId, appKey); + KeyStore.SaveKey(appId, tenantId, appKey); } private class ServicePrincipalAccessToken : IRenewableToken diff --git a/src/Authentication/Factories/AuthenticationFactory.cs b/src/Authentication/Factories/AuthenticationFactory.cs index c84f957c5d..3175f9b18b 100644 --- a/src/Authentication/Factories/AuthenticationFactory.cs +++ b/src/Authentication/Factories/AuthenticationFactory.cs @@ -26,17 +26,47 @@ namespace Microsoft.Azure.Commands.Common.Authentication.Factories { public class AuthenticationFactory : IAuthenticationFactory { - public const string CommonAdTenant = "Common", - DefaultMSILoginUri = "http://169.254.169.254/metadata/identity/oauth2/token", + public const string CommonAdTenant = "Common", + DefaultMSILoginUri = "http://169.254.169.254/metadata/identity/oauth2/token", DefaultBackupMSILoginUri = "http://localhost:50342/oauth2/token"; public AuthenticationFactory() { - TokenProvider = new AdalTokenProvider(); + _getKeyStore = () => + { + IServicePrincipalKeyStore keyStore = null; + if (!AzureSession.Instance.TryGetComponent(ServicePrincipalKeyStore.Name, out keyStore)) + { + keyStore = new ServicePrincipalKeyStore(); + } + + return keyStore; + }; + TokenProvider = new AdalTokenProvider(_getKeyStore); + } + + private Func _getKeyStore; + private IServicePrincipalKeyStore _keyStore; + public IServicePrincipalKeyStore KeyStore + { + get + { + if (_keyStore == null) + { + _keyStore = _getKeyStore(); + } + + return _keyStore; + } + set + { + _keyStore = value; + } } public ITokenProvider TokenProvider { get; set; } + public IAccessToken Authenticate( IAzureAccount account, IAzureEnvironment environment, @@ -305,9 +335,9 @@ public ServiceClientCredentials GetServiceClientCredentials(IAzureContext contex case AzureAccount.AccountType.ManagedService: result = new RenewingTokenCredential( GetManagedServiceToken( - context.Account, - context.Environment, - tenant, + context.Account, + context.Environment, + tenant, context.Environment.GetTokenAudience(targetEndpoint))); break; case AzureAccount.AccountType.User: @@ -334,7 +364,7 @@ public ServiceClientCredentials GetServiceClientCredentials(IAzureContext contex result = ApplicationTokenProvider.LoginSilentAsync( tenant, context.Account.Id, - new KeyStoreApplicationCredentialProvider(tenant), + new KeyStoreApplicationCredentialProvider(tenant, KeyStore), env, tokenCache as TokenCache).ConfigureAwait(false).GetAwaiter().GetResult(); } @@ -370,7 +400,7 @@ public void RemoveUser(IAzureAccount account, IAzureTokenCache tokenCache) case AzureAccount.AccountType.ServicePrincipal: try { - ServicePrincipalKeyStore.DeleteKey(account.Id, account.GetTenants().FirstOrDefault()); + KeyStore.DeleteKey(account.Id, account.GetTenants().FirstOrDefault()); } catch { diff --git a/src/Authentication/KeyStore/IServicePrincipalKeyStore.cs b/src/Authentication/KeyStore/IServicePrincipalKeyStore.cs new file mode 100644 index 0000000000..0381467a04 --- /dev/null +++ b/src/Authentication/KeyStore/IServicePrincipalKeyStore.cs @@ -0,0 +1,55 @@ +// ---------------------------------------------------------------------------------- +// +// Copyright Microsoft Corporation +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ---------------------------------------------------------------------------------- + +using System.Security; + +namespace Microsoft.Azure.Commands.Common.Authentication +{ + /// + /// This interface represents objects that can be used + /// to store and retrieve service principal keys. + /// + public interface IServicePrincipalKeyStore + { + /// + /// Store the service principal key on the user's machine. + /// + /// Application id of the service principal. + /// Id of the tenant that the service principal is found in. + /// Key for the provided service principal. + void SaveKey( + string appId, + string tenantId, + SecureString serviceKey); + + /// + /// Retrieves the service principal key from the store. + /// + /// Application id of the service principal. + /// Id of the tenant that the service principal is found in. + /// Key for the provided service principal. + SecureString GetKey( + string appId, + string tenantId); + + /// + /// Removes a service principal key from the store. + /// + /// Application id of the service principal. + /// Id of the tenant that the service principal is found in. + void DeleteKey( + string appId, + string tenantId); + } +} diff --git a/src/Authentication/Authentication/ServicePrincipalKeyStore.Netcore.cs b/src/Authentication/KeyStore/ServicePrincipalKeyStore.cs similarity index 82% rename from src/Authentication/Authentication/ServicePrincipalKeyStore.Netcore.cs rename to src/Authentication/KeyStore/ServicePrincipalKeyStore.cs index 3192dc4547..cece9b472d 100644 --- a/src/Authentication/Authentication/ServicePrincipalKeyStore.Netcore.cs +++ b/src/Authentication/KeyStore/ServicePrincipalKeyStore.cs @@ -12,7 +12,6 @@ // limitations under the License. // ---------------------------------------------------------------------------------- using System; -using System.Collections; using System.Collections.Generic; using System.Security; @@ -22,17 +21,18 @@ namespace Microsoft.Azure.Commands.Common.Authentication /// Helper class to store service principal keys and retrieve them /// from the Windows Credential Store. /// - public static class ServicePrincipalKeyStore + public class ServicePrincipalKeyStore : IServicePrincipalKeyStore { + public const string Name = "ServicePrincipalKeyStore"; private static IDictionary _credentials = new Dictionary(); - public static void SaveKey(string appId, string tenantId, SecureString serviceKey) + public void SaveKey(string appId, string tenantId, SecureString serviceKey) { var key = CreateKey(appId, tenantId); _credentials[key] = serviceKey; } - public static SecureString GetKey(string appId, string tenantId) + public SecureString GetKey(string appId, string tenantId) { IntPtr pCredential = IntPtr.Zero; try @@ -50,7 +50,7 @@ public static SecureString GetKey(string appId, string tenantId) } - public static void DeleteKey(string appId, string tenantId) + public void DeleteKey(string appId, string tenantId) { try { @@ -62,12 +62,12 @@ public static void DeleteKey(string appId, string tenantId) } } - private static string CreateKey(string appId, string tenantId) + private string CreateKey(string appId, string tenantId) { return $"{appId}_{tenantId}"; } - internal static SecureString ConvertToSecureString(string password) + internal SecureString ConvertToSecureString(string password) { if (password == null) throw new ArgumentNullException("password"); diff --git a/src/Common.Test/Common.Test.csproj b/src/Common.Test/Common.Test.csproj index bf3539499b..a98e4bf0da 100644 --- a/src/Common.Test/Common.Test.csproj +++ b/src/Common.Test/Common.Test.csproj @@ -85,6 +85,10 @@ {70527617-7598-4aef-b5bd-db9186b8184b} Authentication.Abstractions + + {69c2eb6b-cd63-480a-89a0-c489706e9299} + Authentication.ResourceManager + {d3804b64-c0d3-48f8-82ec-1f632f833c9e} Authentication diff --git a/src/Common.Test/Common/ServicePrincipalStoreTests.cs b/src/Common.Test/Common/ServicePrincipalStoreTests.cs index 95e8f1872a..af16bc8379 100644 --- a/src/Common.Test/Common/ServicePrincipalStoreTests.cs +++ b/src/Common.Test/Common/ServicePrincipalStoreTests.cs @@ -13,6 +13,7 @@ // ---------------------------------------------------------------------------------- using Microsoft.Azure.Commands.Common.Authentication; +using Microsoft.Azure.Commands.ResourceManager.Common; using Microsoft.WindowsAzure.Commands.ScenarioTest; using Microsoft.WindowsAzure.ServiceManagemenet.Common.Models; using System; @@ -26,9 +27,12 @@ namespace Microsoft.WindowsAzure.Commands.Common.Test.Common { public class ServicePrincipalStoreTests { + private IServicePrincipalKeyStore _keyStore; + public ServicePrincipalStoreTests(ITestOutputHelper output) { XunitTracingInterceptor.AddToContext(new XunitTracingInterceptor(output)); + _keyStore = new ServicePrincipalKeyStore(); } [Fact] @@ -41,13 +45,13 @@ public void CanStoreAndRetrieveSingleKeyCorrectly() using (SecureString secret = SecureStringFromString(password)) { - ServicePrincipalKeyStore.SaveKey(appId, tenantId, secret); + _keyStore.SaveKey(appId, tenantId, secret); } try { string retrievedPassword; - using (SecureString secret = ServicePrincipalKeyStore.GetKey(appId, tenantId)) + using (SecureString secret = _keyStore.GetKey(appId, tenantId)) { retrievedPassword = StringFromSecureString(secret); } @@ -56,7 +60,7 @@ public void CanStoreAndRetrieveSingleKeyCorrectly() } finally { - ServicePrincipalKeyStore.DeleteKey(appId, tenantId); + _keyStore.DeleteKey(appId, tenantId); } } @@ -70,7 +74,7 @@ public void RetrievingNonExistingKeyReturnsNull() SecureString ss = null; try { - ss = ServicePrincipalKeyStore.GetKey(appId, tenantId); + ss = _keyStore.GetKey(appId, tenantId); Assert.Null(ss); } finally @@ -91,17 +95,17 @@ public void DeletingKeyWorks() using (SecureString ss = SecureStringFromString("sekret")) { - ServicePrincipalKeyStore.SaveKey(appId, tenantId, ss); + _keyStore.SaveKey(appId, tenantId, ss); } - using (SecureString ss = ServicePrincipalKeyStore.GetKey(appId, tenantId)) + using (SecureString ss = _keyStore.GetKey(appId, tenantId)) { Assert.NotNull(ss); } - ServicePrincipalKeyStore.DeleteKey(appId, tenantId); + _keyStore.DeleteKey(appId, tenantId); - using (SecureString ss = ServicePrincipalKeyStore.GetKey(appId, tenantId)) + using (SecureString ss = _keyStore.GetKey(appId, tenantId)) { Assert.Null(ss); } @@ -119,29 +123,29 @@ public void CanStoreMultipleKeysIndependently() using (SecureString ss = SecureStringFromString(pwd1)) { - ServicePrincipalKeyStore.SaveKey(appId, tenant1, ss); + _keyStore.SaveKey(appId, tenant1, ss); } using (SecureString ss = SecureStringFromString(pwd2)) { - ServicePrincipalKeyStore.SaveKey(appId, tenant2, ss); + _keyStore.SaveKey(appId, tenant2, ss); } try { - using (SecureString ss = ServicePrincipalKeyStore.GetKey(appId, tenant1)) + using (SecureString ss = _keyStore.GetKey(appId, tenant1)) { Assert.Equal(pwd1, StringFromSecureString(ss)); } - using (SecureString ss = ServicePrincipalKeyStore.GetKey(appId, tenant2)) + using (SecureString ss = _keyStore.GetKey(appId, tenant2)) { Assert.Equal(pwd2, StringFromSecureString(ss)); } } finally { - ServicePrincipalKeyStore.DeleteKey(appId, tenant1); - ServicePrincipalKeyStore.DeleteKey(appId, tenant2); + _keyStore.DeleteKey(appId, tenant1); + _keyStore.DeleteKey(appId, tenant2); } } From 23b0a82dde578173b38a3c1a9f5216389a85680f Mon Sep 17 00:00:00 2001 From: cormacpayne Date: Fri, 7 Sep 2018 17:02:14 -0700 Subject: [PATCH 2/4] Refactor for desktop --- .../Authentication.ResourceManager.csproj | 2 +- ...incipalKeyStore.cs => AzureRmServicePrincipalKeyStore.cs} | 3 ++- src/Authentication.Test/AuthenticationFactoryTests.cs | 2 +- src/Authentication/Authentication.csproj | 1 + src/Authentication/Authentication/AdalTokenProvider.cs | 5 +++-- src/Common.Test/Common/ServicePrincipalStoreTests.cs | 2 +- 6 files changed, 9 insertions(+), 6 deletions(-) rename src/Authentication.ResourceManager/{ServicePrincipalKeyStore.cs => AzureRmServicePrincipalKeyStore.cs} (96%) diff --git a/src/Authentication.ResourceManager/Authentication.ResourceManager.csproj b/src/Authentication.ResourceManager/Authentication.ResourceManager.csproj index 85f35f8a71..e226f6ce50 100644 --- a/src/Authentication.ResourceManager/Authentication.ResourceManager.csproj +++ b/src/Authentication.ResourceManager/Authentication.ResourceManager.csproj @@ -79,7 +79,7 @@ - + diff --git a/src/Authentication.ResourceManager/ServicePrincipalKeyStore.cs b/src/Authentication.ResourceManager/AzureRmServicePrincipalKeyStore.cs similarity index 96% rename from src/Authentication.ResourceManager/ServicePrincipalKeyStore.cs rename to src/Authentication.ResourceManager/AzureRmServicePrincipalKeyStore.cs index 4d3c9deaf0..8531574ded 100644 --- a/src/Authentication.ResourceManager/ServicePrincipalKeyStore.cs +++ b/src/Authentication.ResourceManager/AzureRmServicePrincipalKeyStore.cs @@ -24,8 +24,9 @@ namespace Microsoft.Azure.Commands.ResourceManager.Common /// Helper class to store service principal keys and retrieve them /// from the Windows Credential Store. /// - public class ServicePrincipalKeyStore : IServicePrincipalKeyStore + public class AzureRmServicePrincipalKeyStore : IServicePrincipalKeyStore { + public const string Name = "ServicePrincipalKeyStore"; private const string keyStoreUserName = "PowerShellServicePrincipalKey"; private const string targetNamePrefix = "AzureSession:target="; diff --git a/src/Authentication.Test/AuthenticationFactoryTests.cs b/src/Authentication.Test/AuthenticationFactoryTests.cs index b25383b60e..bd39d5cad9 100644 --- a/src/Authentication.Test/AuthenticationFactoryTests.cs +++ b/src/Authentication.Test/AuthenticationFactoryTests.cs @@ -76,7 +76,7 @@ public void VerifySubscriptionTokenCacheRemove() [Trait(Category.AcceptanceType, Category.CheckIn)] public void VerifyValidateAuthorityFalseForOnPremise() { - AzureSessionInitializer.InitializeAzureSession(new ServicePrincipalKeyStore()); + AzureSessionInitializer.InitializeAzureSession(); var authFactory = new AuthenticationFactory { TokenProvider = new MockAccessTokenProvider("testtoken", "testuser") diff --git a/src/Authentication/Authentication.csproj b/src/Authentication/Authentication.csproj index 8004c73863..872343b603 100644 --- a/src/Authentication/Authentication.csproj +++ b/src/Authentication/Authentication.csproj @@ -76,6 +76,7 @@ + diff --git a/src/Authentication/Authentication/AdalTokenProvider.cs b/src/Authentication/Authentication/AdalTokenProvider.cs index 1d66901b71..ad4a56cd3c 100644 --- a/src/Authentication/Authentication/AdalTokenProvider.cs +++ b/src/Authentication/Authentication/AdalTokenProvider.cs @@ -41,12 +41,13 @@ public AdalTokenProvider() public AdalTokenProvider(IWin32Window parentWindow) { this.userTokenProvider = new UserTokenProvider(parentWindow); + this.servicePrincipalTokenProvider = new ServicePrincipalTokenProvider(); } - public AdalTokenProvider(IServicePrincipalKeyStore keyStore) + public AdalTokenProvider(Func getKeyStore) { this.userTokenProvider = new UserTokenProvider(new ConsoleParentWindow()); - this.servicePrincipalTokenProvider = new ServicePrincipalTokenProvider(keyStore); + this.servicePrincipalTokenProvider = new ServicePrincipalTokenProvider(getKeyStore); } public IAccessToken GetAccessToken( diff --git a/src/Common.Test/Common/ServicePrincipalStoreTests.cs b/src/Common.Test/Common/ServicePrincipalStoreTests.cs index af16bc8379..3fd44d2fca 100644 --- a/src/Common.Test/Common/ServicePrincipalStoreTests.cs +++ b/src/Common.Test/Common/ServicePrincipalStoreTests.cs @@ -32,7 +32,7 @@ public class ServicePrincipalStoreTests public ServicePrincipalStoreTests(ITestOutputHelper output) { XunitTracingInterceptor.AddToContext(new XunitTracingInterceptor(output)); - _keyStore = new ServicePrincipalKeyStore(); + _keyStore = new AzureRmServicePrincipalKeyStore(); } [Fact] From 06d30a5c7cfa087e2c51c79187b97aa3782474a3 Mon Sep 17 00:00:00 2001 From: cormacpayne Date: Mon, 10 Sep 2018 09:57:50 -0700 Subject: [PATCH 3/4] Fix build failure --- .../Authentication.ResourceManager.Netcore.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Authentication.ResourceManager/Authentication.ResourceManager.Netcore.csproj b/src/Authentication.ResourceManager/Authentication.ResourceManager.Netcore.csproj index 4927902c29..50f9f0d9ea 100644 --- a/src/Authentication.ResourceManager/Authentication.ResourceManager.Netcore.csproj +++ b/src/Authentication.ResourceManager/Authentication.ResourceManager.Netcore.csproj @@ -56,6 +56,7 @@ + From 43d2f49fd0ef7557eca0cdbe7ba0b13b21845bcb Mon Sep 17 00:00:00 2001 From: cormacpayne Date: Thu, 13 Sep 2018 10:09:36 -0700 Subject: [PATCH 4/4] Only write service principal secret to context in .NET Core --- src/Authentication.Test/Cmdlets/ConnectAccount.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Authentication.Test/Cmdlets/ConnectAccount.cs b/src/Authentication.Test/Cmdlets/ConnectAccount.cs index 8709c51b6b..cf13a42472 100644 --- a/src/Authentication.Test/Cmdlets/ConnectAccount.cs +++ b/src/Authentication.Test/Cmdlets/ConnectAccount.cs @@ -94,10 +94,12 @@ public override void ExecuteCmdlet() Account.SetProperty(AzureAccount.Property.Tenants, new[] { TenantId }); } +#if NETSTANDARD if (!string.IsNullOrEmpty(Password)) { Account.SetProperty(AzureAccount.Property.ServicePrincipalSecret, Password); } +#endif if (AzureRmProfileProvider.Instance.Profile == null) {