Conversation
prashant1shukla
commented
May 18, 2024
| [HttpPost] | ||
| public IActionResult Login(LoginViewModel login) | ||
| { | ||
| var user = _authenticationService.AuthenticateUser(login); |
| { | ||
| var user = _authenticationService.AuthenticateUser(login); | ||
| var token = _tokenService.GenerateToken(user); | ||
| return Ok(new { token }); |
There was a problem hiding this comment.
you should have separate DTOs/View Model for this response.
| var user = _authenticationService.AuthenticateUser(login); | ||
| var token = _tokenService.GenerateToken(user); |
There was a problem hiding this comment.
you can write a function in service itself which first authenticates the user and then directly returns the token
| if (!_userService.RegisterUser(user)) | ||
| { | ||
| return Conflict("Username already exists"); | ||
| } |
There was a problem hiding this comment.
this should be in the service itself. You can throw custom exception and catch it in the middleware directly and then can convert the message.
| var username = User.Identity.Name; | ||
| // Comparing the usernames using GetUserByUsername function | ||
| var user = _userService.GetUserByUsername(username); | ||
|
|
||
| //For invalid token | ||
| if (user == null) | ||
| { | ||
| return NotFound("User not found"); | ||
| } | ||
|
|
||
| // Returning the user data if a valid token is passed | ||
| return Ok(user); |
There was a problem hiding this comment.
this should also be in the service layer with a custom exception to handle user not found
| public string GenerateToken(UserModel user) | ||
| { | ||
| // providing the security key and credentials for token generation | ||
| var securityKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(_config["Jwt:Key"])); | ||
| var credentials = new SigningCredentials(securityKey, SecurityAlgorithms.HmacSha256); | ||
|
|
||
| // Taking data from appsettings.json | ||
| var token = new JwtSecurityToken( | ||
| issuer: _config["Jwt:Issuer"], | ||
| audience: _config["Jwt:Audience"], | ||
| claims: new[] | ||
| { | ||
| new Claim(ClaimTypes.Name, user.Username) | ||
| // Add additional claims if needed (e.g., user roles) | ||
| }, | ||
| expires: DateTime.UtcNow.AddMinutes(30), // Token expires in 30 minutes | ||
| signingCredentials: credentials | ||
| ); | ||
|
|
||
| return new JwtSecurityTokenHandler().WriteToken(token); |
There was a problem hiding this comment.
instead of having this as a separate service function you can create a jwtUtil class and call use this function in authentication service directly.
|
|
||
| //Post request to register a user | ||
| [HttpPost] | ||
| public IActionResult Register(UserModel user) |
There was a problem hiding this comment.
Instead of directly using UserModel use DTOs/ViewModel
| if (UserDataStore.Users.Any(u => u.Username == user.Username)) | ||
| { | ||
| return false; // Username already exists | ||
| } | ||
|
|
||
| //If the username already does not exist, make new registration | ||
| UserDataStore.Users.Add(user); |
| } | ||
|
|
||
| // Function to get the User data, it is used in User Controller | ||
| public UserModel GetUserByUsername(string username) |
There was a problem hiding this comment.
use DTO/View Model as the return type.
| // Check if password contains any special characters | ||
| if (password.Any(char.IsPunctuation) || password.Any(char.IsSymbol)) | ||
| { | ||
| return new ValidationResult("Password cannot contain special characters"); |
There was a problem hiding this comment.
Using this for Name validation and returning a message related to password