Post History

>     <Window x:Class="Login_App.MainWindow"
>             xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
>             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
>             xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
>             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
>             xmlns:local="clr-namespace:Login_App"
>             xmlns:vm ="clr-namespace:Login_App.ViewModel"
>             mc:Ignorable="d"

The template contains a bunch of junk. If you're not using it, delete it. I think that you can eliminate at the very least the namespaces `local`, `mc`, `d`. My next suggestion would also eliminate `vm`.

---

>         <Window.Resources>
>             <ResourceDictionary>
>                 <vm:LoginVM x:Key="vm"/>
>             </ResourceDictionary>
>         </Window.Resources>
>         
>         <StackPanel DataContext="{StaticResource vm}">

This feels to me like an odd way to do things. I would be more inclined to remove this and instead set `DataContext = viewModel` in the codebehind constructor, before `InitializeComponent();`. (That would make it the `DataContext` for the window rather than just the `StackPanel`, but I don't see any problem with that).

---

>             <TextBlock Text="Username"/>
>             <TextBox Text="{Binding Username, Mode=TwoWay, UpdateSourceTrigger=PropertyChanged}"/>
>             <TextBlock Text="Password"/>
>             <TextBox Text="{Binding Password, Mode=TwoWay, UpdateSourceTrigger=PropertyChanged}"/>
>             <Button Content="Login"
>                     Command="{Binding LoginCommand}"
>                     CommandParameter="{Binding User}"/>

This should probably use `PasswordBox` for the password. It's a hassle to use (you have to name it and pass it as the command parameter with `CommandParameter="{Binding ElementName=myPasswordBox}"`), but it's pretty much expected nowadays that you don't show passwords in plain text.

---

>         /// <summary>
>         /// Interaction logic for MainWindow.xaml
>         /// </summary>

Again, ditch the junk (optionally replacing it with an actually useful comment).

---

>             private void ViewModel_Authenticated(object sender, EventArgs e)
>             {
>                 Close();
>             }

Ok, this is a proof-of-concept, but in practical application you need to be careful what assumptions other code makes on the basis of this window closing.

---

>         public class UserModel
>         {
>             public int UserID { get; set; }
>             public string Username { get; set; }
>             public string Email { get; set; }
>             public string Password { get; set; }
>             public int DepartmentID { get; set; }
>             public int GroupID { get; set; }
>             public bool Inactive { get; set; }
>         }

Is it really necessary for all of these to have public setters? That seems like a bad idea.

---

>         public class LoginVM : INotifyPropertyChanged
>         {
>             private UserModel user;
>             public UserModel User
>             {
>                 get { return user; }
>                 set
>                 {
>                     user = value;
>                     OnPropertyChanged("User");

Use `nameof`. If your IDE didn't give you a warning here, check that your project is including the standard analysers. If it did give you a warning, configure the analysers to make it an error. The only "good" reason for a property name not to use `nameof` is backwards compatibility with something which had a typo.

---

>             private string username;
>             public string Username
>             {
>                 get { return username; }
>                 set
>                 {
>                     username = value;
>                     User = new UserModel
>                     {
>                         Username = username,
>                         Password = this.Password
>                     };

and similarly `Password`.

I'm deeply suspicious of this duplication, especially because `UserModel` isn't immutable.

    myModel.User.Username = "foo";
    myModel.Password = "bar";

would do something very different if you changed the order of the two lines, and that's a trap waiting to catch someone out.

Making the `UserModel` class immutable would improve the situation slightly, but I'm not even convinced that the same class should expose both `User` and its properties. Probably the login model should have `Username` and `Password` and `User` should be the return value of the `Login` method.

---

>             public Commands.LoginCommand LoginCommand { get; set; }

Code to the interface: is there any reason that this shouldn't have type `ICommand`?

Why the setter?

---

>             public bool CanExecute(object parameter)
>             {
>                 UserModel user = parameter as UserModel;
>     
>                 if (user == null)
>                     return false;
>     
>                 if (string.IsNullOrEmpty(user.Username))
>                     return false;
>     
>                 if (string.IsNullOrEmpty(user.Password))
>                     return false;
>     
>                 return true;
>     
>             }
>     
>             public void Execute(object parameter)
>             {
>                 ViewModel.Login();
>             }

Having a `CanExecute` which checks the parameter and an `Execute` which ignores the parameter is a code smell which says that this can almost certainly be refactored to be simpler and less tightly coupled.

---

>         public static async Task<bool> Login(UserModel user)
>         {
>             try
>             {
>                 /// Query to get count of users from the parameters provided > by the user.
>                 /// On successful Login, the details from the database,
>                 /// will be saved in the <App.xaml.cs> file.
>     
>                 string mysqlUserDetails = "SELECT COUNT(*), u.id AS UserID, Username, Email, DepartmentID, Inactive" +
>                     " FROM users u WHERE Username = @Username AND Password = MD5(@Password)";
>     
>                 using(var connection = new MySqlConnection(ConfigurationManager.ConnectionStrings["MySQL"].ConnectionString))
>                 {
>                     var userDetails = connection.Query<UserModel>(mysqlUserDetails, new { Username = user.Username, Password = user.Password }).ToList();
>     
>                     // Determine whether to autherize or not
>                     if(userDetails.Count == 1)
>                     {
>                         var details = userDetails.First();
>                         App.UserId = details.UserID;
>                         App.Username = details.Username;
>                         App.Email = details.Email;
>                         App.DepartmentID = details.DepartmentID;
>                         App.Inative = details.Inactive;
> 
>                     }
> 
>     
>                     
>     
>                 }
>                 return false;
>             }
>             catch (Exception ex)
>             {
>                 return false;
>             }
>         }
>     }

Why the many contiguous lines of whitespace in the middle?

The comment says that "the details from the database will be saved in the <App.xaml.cs> file". What does that mean? It sounds like it's trying to modify its own code, but it's not in an interpreted language.

There is no path in this method which returns `true`.

Consider using an ORM, even if it's as lightweight as Dapper.

MD5 has not been an acceptable way to hash passwords for at least the past 20 years. You should be looking at something like bcrypt, scrypt, Argon2, ..., and ideally should include an upgrade path for both algorithm and difficulty parameters. There should probably also be a random delay to reduce the side-channel information on which usernames are valid.