diff --git a/src/Microsoft.SqlTools.ServiceLayer/Security/CreateLoginData.cs b/src/Microsoft.SqlTools.ServiceLayer/Security/LoginData.cs similarity index 100% rename from src/Microsoft.SqlTools.ServiceLayer/Security/CreateLoginData.cs rename to src/Microsoft.SqlTools.ServiceLayer/Security/LoginData.cs diff --git a/src/Microsoft.SqlTools.ServiceLayer/Security/SecurityService.cs b/src/Microsoft.SqlTools.ServiceLayer/Security/SecurityService.cs index 80e4c562..b477d1d3 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Security/SecurityService.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Security/SecurityService.cs @@ -168,7 +168,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Security #region "User Handlers" - private UserPrototypeNew InitUserNew(CDataContainer dataContainer) + private UserPrototype InitUserNew(CDataContainer dataContainer) { // this.DataContainer = context; // this.parentDbUrn = new Urn(this.DataContainer.ParentUrn); @@ -193,7 +193,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Security dataContainer.Server.GetSmoObject(dataContainer.ObjectUrn) as User); } - UserPrototypeNew currentUserPrototype = userPrototypeFactory.GetUserPrototype(currentUserType); + UserPrototype currentUserPrototype = userPrototypeFactory.GetUserPrototype(currentUserType); return currentUserPrototype; } @@ -310,7 +310,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Security public void UserMemberships_OnRunNow(object sender, CDataContainer dataContainer) { - UserPrototypeNew currentPrototype = UserPrototypeFactory.GetInstance(dataContainer).CurrentPrototype; + UserPrototype currentPrototype = UserPrototypeFactory.GetInstance(dataContainer).CurrentPrototype; //In case the UserGeneral/OwnedSchemas pages are loaded, //those will takes care of applying membership changes also. @@ -336,7 +336,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Security /// public void UserOwnedSchemas_OnRunNow(object sender, CDataContainer dataContainer) { - UserPrototypeNew currentPrototype = UserPrototypeFactory.GetInstance(dataContainer).CurrentPrototype; + UserPrototype currentPrototype = UserPrototypeFactory.GetInstance(dataContainer).CurrentPrototype; //In case the UserGeneral/Membership pages are loaded, //those will takes care of applying schema ownership changes also. diff --git a/src/Microsoft.SqlTools.ServiceLayer/Security/UserData.cs b/src/Microsoft.SqlTools.ServiceLayer/Security/UserData.cs index caca6623..ec840add 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Security/UserData.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Security/UserData.cs @@ -9,6 +9,7 @@ using System.Security; using Microsoft.SqlServer.Management.Smo; using Microsoft.SqlServer.Management.Sdk.Sfc; using Microsoft.SqlTools.ServiceLayer.Management; +using System.Linq; namespace Microsoft.SqlTools.ServiceLayer.Security { @@ -72,13 +73,13 @@ namespace Microsoft.SqlTools.ServiceLayer.Security /// Object have exhaustive list of data elements which are required for creating /// any type of database user. /// - public class UserPrototypeDataNew + public class UserPrototypeData { public string name = string.Empty; public UserType userType = UserType.SqlUser; public bool isSystemObject = false; - public Dictionary isSchemaOwned = null; - public Dictionary isMember = null; + public Dictionary isSchemaOwned; + public Dictionary isMember; public AuthenticationType authenticationType = AuthenticationType.Instance; public string mappedLoginName = string.Empty; @@ -94,13 +95,13 @@ namespace Microsoft.SqlTools.ServiceLayer.Security /// /// Used for creating clone of a UserPrototypeData. /// - private UserPrototypeDataNew() + private UserPrototypeData() { this.isSchemaOwned = new Dictionary(); this.isMember = new Dictionary(); } - public UserPrototypeDataNew(CDataContainer context) + public UserPrototypeData(CDataContainer context) { this.isSchemaOwned = new Dictionary(); this.isMember = new Dictionary(); @@ -115,9 +116,9 @@ namespace Microsoft.SqlTools.ServiceLayer.Security this.LoadSchemaData(context); } - public UserPrototypeDataNew Clone() + public UserPrototypeData Clone() { - UserPrototypeDataNew result = new UserPrototypeDataNew(); + UserPrototypeData result = new UserPrototypeData(); result.asymmetricKeyName = this.asymmetricKeyName; result.authenticationType = this.authenticationType; @@ -133,20 +134,28 @@ namespace Microsoft.SqlTools.ServiceLayer.Security result.isOldPasswordRequired = this.isOldPasswordRequired; result.userType = this.userType; - foreach (string key in this.isMember.Keys) + foreach (string key in this.isMember?.Keys ?? Enumerable.Empty()) { - result.isMember[key] = this.isMember[key]; + if (result.isMember?.ContainsKey(key) == true + && this.isMember?.ContainsKey(key) == true) + { + result.isMember[key] = this.isMember[key]; + } } - foreach (string key in this.isSchemaOwned.Keys) + foreach (string key in this.isSchemaOwned?.Keys ?? Enumerable.Empty()) { - result.isSchemaOwned[key] = this.isSchemaOwned[key]; + if (result.isSchemaOwned?.ContainsKey(key) == true + && this.isSchemaOwned?.ContainsKey(key) == true) + { + result.isSchemaOwned[key] = this.isSchemaOwned[key]; + } } return result; } - public bool HasSameValueAs(UserPrototypeDataNew other) + public bool HasSameValueAs(UserPrototypeData other) { bool result = (this.asymmetricKeyName == other.asymmetricKeyName) && @@ -163,12 +172,14 @@ namespace Microsoft.SqlTools.ServiceLayer.Security (this.isOldPasswordRequired == other.isOldPasswordRequired) && (this.userType == other.userType); - result = result && this.isMember.Keys.Count == other.isMember.Keys.Count; + result = result && this.isMember?.Keys.Count == other.isMember?.Keys.Count; if (result) { - foreach (string key in this.isMember.Keys) + foreach (string key in this.isMember?.Keys ?? Enumerable.Empty()) { - if (this.isMember[key] != other.isMember[key]) + if (this.isMember?.ContainsKey(key) == true + && other.isMember?.ContainsKey(key) == true + && this.isMember[key] != other.isMember[key]) { result = false; break; @@ -198,7 +209,11 @@ namespace Microsoft.SqlTools.ServiceLayer.Security /// private void LoadUserData(CDataContainer context) { - User existingUser = context.Server.GetSmoObject(new Urn(context.ObjectUrn)) as User; + User? existingUser = context.Server.GetSmoObject(new Urn(context.ObjectUrn)) as User; + if (existingUser == null) + { + return; + } this.name = existingUser.Name; this.mappedLoginName = existingUser.Login; @@ -229,7 +244,12 @@ namespace Microsoft.SqlTools.ServiceLayer.Security Urn objUrn = new Urn(context.ObjectUrn); Urn databaseUrn = objUrn.Parent; - Database parentDb = context.Server.GetSmoObject(databaseUrn) as Database; + Database? parentDb = context.Server.GetSmoObject(databaseUrn) as Database; + if (parentDb == null) + { + return; + } + User existingUser = context.Server.Databases[parentDb.Name].Users[objUrn.GetNameForType("User")]; foreach (DatabaseRole dbRole in parentDb.Roles) @@ -258,7 +278,12 @@ namespace Microsoft.SqlTools.ServiceLayer.Security Urn objUrn = new Urn(context.ObjectUrn); Urn databaseUrn = objUrn.Parent; - Database parentDb = context.Server.GetSmoObject(databaseUrn) as Database; + Database? parentDb = context.Server.GetSmoObject(databaseUrn) as Database; + if (parentDb == null) + { + return; + } + User existingUser = context.Server.Databases[parentDb.Name].Users[objUrn.GetNameForType("User")]; if (!SqlMgmtUtils.IsYukonOrAbove(context.Server) @@ -285,15 +310,15 @@ namespace Microsoft.SqlTools.ServiceLayer.Security /// /// Prototype object for creating or altering users /// - internal class UserPrototypeNew : IUserPrototype + internal class UserPrototype : IUserPrototype { - protected UserPrototypeDataNew originalState = null; - protected UserPrototypeDataNew currentState = null; + protected UserPrototypeData originalState; + protected UserPrototypeData currentState; - private List schemaNames = null; - private List roleNames = null; + private List schemaNames; + private List roleNames; private bool exists = false; - private Database parent = null; + private Database parent; public bool IsRoleMembershipChangesApplied { get; set; } //default is false public bool IsSchemaOwnershipChangesApplied { get; set; } //default is false @@ -412,23 +437,28 @@ namespace Microsoft.SqlTools.ServiceLayer.Security /// Constructor /// /// The context for the dialog - public UserPrototypeNew(CDataContainer context, - UserPrototypeDataNew current, - UserPrototypeDataNew original) + public UserPrototype(CDataContainer context, + UserPrototypeData current, + UserPrototypeData original) { this.currentState = current; this.originalState = original; - this.exists = !context.IsNewObject; - this.parent = context.Server.GetSmoObject(new Urn(context.ParentUrn)) as Database; - - this.PopulateRoles(); - this.PopulateSchemas(); + + Database? parent = context.Server.GetSmoObject(new Urn(context.ParentUrn)) as Database; + if (parent == null) + { + throw new ArgumentException("Context ParentUrn is invalid"); + } + this.parent = parent; + + this.roleNames = this.PopulateRoles(); + this.schemaNames = this.PopulateSchemas(); } - private void PopulateRoles() + private List PopulateRoles() { - this.roleNames = new List(); + var roleNames = new List(); foreach (DatabaseRole dbRole in this.parent.Roles) { @@ -438,22 +468,24 @@ namespace Microsoft.SqlTools.ServiceLayer.Security this.roleNames.Add(dbRole.Name); } } + return roleNames; } - private void PopulateSchemas() + private List PopulateSchemas() { - this.schemaNames = new List(); + var schemaNames = new List(); if (!SqlMgmtUtils.IsYukonOrAbove(this.parent.Parent) || this.parent.CompatibilityLevel <= CompatibilityLevel.Version80) { - return; + throw new ArgumentException("Unsupported server version"); } foreach (Schema sch in this.parent.Schemas) { this.schemaNames.Add(sch.Name); - } + } + return schemaNames; } public bool IsYukonOrLater @@ -466,9 +498,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Security public User ApplyChanges() { - User user = null; - - user = this.GetUser(); + User user = this.GetUser(); if (this.ChangesExist()) { @@ -504,50 +534,56 @@ namespace Microsoft.SqlTools.ServiceLayer.Security private void ApplySchemaOwnershipChanges(User user) { - IEnumerator> enumerator = this.currentState.isSchemaOwned.GetEnumerator(); - enumerator.Reset(); - - String nullString = null; - - while (enumerator.MoveNext()) + IEnumerator>? enumerator = this.currentState.isSchemaOwned?.GetEnumerator(); + if (enumerator != null) { - string schemaName = enumerator.Current.Key.ToString(); - bool userIsOwner = (bool)enumerator.Current.Value; + enumerator.Reset(); - if (((bool)this.originalState.isSchemaOwned[schemaName]) != userIsOwner) + String? nullString = null; + + while (enumerator.MoveNext()) { - System.Diagnostics.Debug.Assert(!this.Exists || userIsOwner, "shouldn't have to unset ownership for new users"); + string schemaName = enumerator.Current.Key.ToString(); + bool userIsOwner = (bool)enumerator.Current.Value; - Schema schema = this.parent.Schemas[schemaName]; - schema.Owner = userIsOwner ? user.Name : nullString; - schema.Alter(); + if (this.originalState.isSchemaOwned?[schemaName] != userIsOwner) + { + System.Diagnostics.Debug.Assert(!this.Exists || userIsOwner, "shouldn't have to unset ownership for new users"); + + Schema schema = this.parent.Schemas[schemaName]; + schema.Owner = userIsOwner ? user.Name : nullString; + schema.Alter(); + } } } } private void ApplyRoleMembershipChanges(User user) { - IEnumerator> enumerator = this.currentState.isMember.GetEnumerator(); - enumerator.Reset(); - - while (enumerator.MoveNext()) + IEnumerator>? enumerator = this.currentState.isMember?.GetEnumerator(); + if (enumerator != null) { - string roleName = enumerator.Current.Key; - bool userIsMember = (bool)enumerator.Current.Value; + enumerator.Reset(); - if (((bool)this.originalState.isMember[roleName]) != userIsMember) + while (enumerator.MoveNext()) { - System.Diagnostics.Debug.Assert(this.Exists || userIsMember, "shouldn't have to unset membership for new users"); + string roleName = enumerator.Current.Key; + bool userIsMember = enumerator.Current.Value; - DatabaseRole role = this.parent.Roles[roleName]; + if (this.originalState.isMember?[roleName] != userIsMember) + { + System.Diagnostics.Debug.Assert(this.Exists || userIsMember, "shouldn't have to unset membership for new users"); - if (userIsMember) - { - role.AddMember(user.Name); - } - else - { - role.DropMember(user.Name); + DatabaseRole role = this.parent.Roles[roleName]; + + if (userIsMember) + { + role.AddMember(user.Name); + } + else + { + role.DropMember(user.Name); + } } } } @@ -577,13 +613,13 @@ namespace Microsoft.SqlTools.ServiceLayer.Security public User GetUser() { - User result = null; + User result; // if we think we exist, get the SMO user object if (this.Exists) { result = this.parent.Users[this.originalState.name]; - result.Refresh(); + result?.Refresh(); System.Diagnostics.Debug.Assert(0 == String.Compare(this.originalState.name, this.currentState.name, StringComparison.Ordinal), "name of existing user has changed"); if (result == null) @@ -613,7 +649,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Security } } - internal class UserPrototypeWithDefaultSchema : UserPrototypeNew, + internal class UserPrototypeWithDefaultSchema : UserPrototype, IUserPrototypeWithDefaultSchema { private CDataContainer context; @@ -648,8 +684,8 @@ namespace Microsoft.SqlTools.ServiceLayer.Security /// /// The context for the dialog public UserPrototypeWithDefaultSchema(CDataContainer context, - UserPrototypeDataNew current, - UserPrototypeDataNew original) + UserPrototypeData current, + UserPrototypeData original) : base(context, current, original) { this.context = context; @@ -692,8 +728,8 @@ namespace Microsoft.SqlTools.ServiceLayer.Security /// /// The context for the dialog public UserPrototypeForSqlUserWithLogin(CDataContainer context, - UserPrototypeDataNew current, - UserPrototypeDataNew original) + UserPrototypeData current, + UserPrototypeData original) : base(context, current, original) { } @@ -719,9 +755,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Security get { //Default Schema was not supported before Denali for windows group. - User user = null; - - user = this.GetUser(); + User user = this.GetUser(); if (this.Exists && user.LoginType == LoginType.WindowsGroup) { return SqlMgmtUtils.IsSql11OrLater(this.context.Server.ConnectionContext.ServerVersion); @@ -763,8 +797,8 @@ namespace Microsoft.SqlTools.ServiceLayer.Security /// /// The context for the dialog public UserPrototypeForWindowsUser(CDataContainer context, - UserPrototypeDataNew current, - UserPrototypeDataNew original) + UserPrototypeData current, + UserPrototypeData original) : base(context, current, original) { this.context = context; @@ -869,8 +903,8 @@ namespace Microsoft.SqlTools.ServiceLayer.Security /// /// The context for the dialog public UserPrototypeForSqlUserWithPassword(CDataContainer context, - UserPrototypeDataNew current, - UserPrototypeDataNew original) + UserPrototypeData current, + UserPrototypeData original) : base(context, current, original) { this.context = context; @@ -933,26 +967,26 @@ namespace Microsoft.SqlTools.ServiceLayer.Security /// internal class UserPrototypeFactory { - private static UserPrototypeFactory singletonInstance; + private static UserPrototypeFactory? singletonInstance; - private UserPrototypeDataNew currentData; - private UserPrototypeDataNew originalData; + private UserPrototypeData currentData; + private UserPrototypeData originalData; private CDataContainer context; - private UserPrototypeNew asymmetricKeyMappedUser; - private UserPrototypeNew certificateMappedUser; - private UserPrototypeNew loginMappedUser; - private UserPrototypeNew noLoginUser; - private UserPrototypeNew sqlUserWithPassword; - private UserPrototypeNew windowsUser; + private UserPrototype? asymmetricKeyMappedUser; + private UserPrototype? certificateMappedUser; + private UserPrototype? loginMappedUser; + private UserPrototype? noLoginUser; + private UserPrototype? sqlUserWithPassword; + private UserPrototype? windowsUser; - private UserPrototypeNew currentPrototype; + private UserPrototype? currentPrototype; - public UserPrototypeNew CurrentPrototype + public UserPrototype CurrentPrototype { get { - currentPrototype ??= new UserPrototypeNew(this.context, + currentPrototype ??= new UserPrototype(this.context, this.currentData, this.originalData); return currentPrototype; @@ -963,7 +997,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Security { this.context = context; - this.originalData = new UserPrototypeDataNew(this.context); + this.originalData = new UserPrototypeData(this.context); this.currentData = this.originalData.Clone(); } @@ -980,19 +1014,19 @@ namespace Microsoft.SqlTools.ServiceLayer.Security return singletonInstance; } - public UserPrototypeNew GetUserPrototype(ExhaustiveUserTypes userType) + public UserPrototype GetUserPrototype(ExhaustiveUserTypes userType) { switch (userType) { case ExhaustiveUserTypes.AsymmetricKeyMappedUser: currentData.userType = UserType.AsymmetricKey; - this.asymmetricKeyMappedUser ??= new UserPrototypeNew(this.context, this.currentData, this.originalData); + this.asymmetricKeyMappedUser ??= new UserPrototype(this.context, this.currentData, this.originalData); this.currentPrototype = asymmetricKeyMappedUser; break; case ExhaustiveUserTypes.CertificateMappedUser: currentData.userType = UserType.Certificate; - this.certificateMappedUser ??= new UserPrototypeNew(this.context, this.currentData, this.originalData); + this.certificateMappedUser ??= new UserPrototype(this.context, this.currentData, this.originalData); this.currentPrototype = certificateMappedUser; break;