From 0b7db1681290c6177538900064fa05bb5ef34916 Mon Sep 17 00:00:00 2001 From: Syndamia Date: Fri, 18 Dec 2020 14:47:45 +0200 Subject: ValidJWT bypasses username check when user is admin --- src/DevHive.Data/Models/Role.cs | 1 + src/DevHive.Services/Services/UserService.cs | 25 +++++++++++-------------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/DevHive.Data/Models/Role.cs b/src/DevHive.Data/Models/Role.cs index 0bf2ab9..0e7dd9f 100644 --- a/src/DevHive.Data/Models/Role.cs +++ b/src/DevHive.Data/Models/Role.cs @@ -9,6 +9,7 @@ namespace DevHive.Data.Models public class Role : IdentityRole { public const string DefaultRole = "User"; + public const string AdminRole = "Admin"; public virtual List Users { get; set; } } diff --git a/src/DevHive.Services/Services/UserService.cs b/src/DevHive.Services/Services/UserService.cs index b2d5aee..ea47bdb 100644 --- a/src/DevHive.Services/Services/UserService.cs +++ b/src/DevHive.Services/Services/UserService.cs @@ -105,37 +105,34 @@ namespace DevHive.Services.Services throw new InvalidOperationException("Unable to delete user!"); } - /// - /// Checks wether the given token's UserName and Roles are the same as these of the user with the given id. - /// public async Task ValidJWT(Guid id, string rawTokenData) { // There is authorization name in the beginning, i.e. "Bearer eyJh..." var jwt = new JwtSecurityTokenHandler().ReadJwtToken(rawTokenData.Remove(0, 7)); + + string jwtUserName = this.GetClaimTypeValues("unique_name", jwt.Claims)[0]; + List jwtRoleNames = this.GetClaimTypeValues("role", jwt.Claims); - User user = await this._userRepository.GetByIdAsync(id) + User user = await this._userRepository.GetByUsername(jwtUserName) ?? throw new ArgumentException("User does not exist!"); - - /* Check username */ - string jwtUserName = this.GetClaimTypeValues("unique_name", jwt.Claims)[0]; - if (jwtUserName != user.UserName) - return false; + /* Username check, only when user isn't admin */ - /* Check roles */ - List jwtRoleNames = this.GetClaimTypeValues("role", jwt.Claims); + if (!jwtRoleNames.Contains(Role.AdminRole)) + if (!this._userRepository.DoesUserHaveThisUsername(id, jwtUserName)) + return false; + /* Check roles */ + // Check if jwt contains all user roles (if it doesn't, jwt is either old or tampered with) foreach(var role in user.Roles) { if (!jwtRoleNames.Contains(role.Name)) return false; - - jwtRoleNames.Remove(role.Name); } // Check if jwt contains only roles of user - if (jwtRoleNames.Count > 0) + if (jwtRoleNames.Count != user.Roles.Count) return false; return true; -- cgit v1.2.3