Według mnie powinieneś wstrzykiwać DbContext do serwisów poprzez konstruktor zamiast kodu tego typu w każdej metodzie:
using (var context = new ShopDbContext())
{
...
}
AccountService:
if (checkPassword == PasswordVerificationResult.Success)
{
var claims = new List<Claim>
{
new Claim(ClaimTypes.NameIdentifier, (context.User.ToList().Count+1).ToString()),
new Claim(ClaimTypes.Email, user.Email),
new Claim(ClaimTypes.Role, user.Role),
};
...
Linia 5 w powyższym wycinku. Dla mnie to nie ma sensu. Według mnie chcesz tam umieścić ID. Dodatkowo w takim wypadku nie musisz już odpytywać dbContextu bo pobrałeś Usera kilka linii wyżej.
https://stackoverflow.com/questions/5814017/what-is-the-purpose-of-nameidentifier-claim
Nie możesz ufać danym przesyłanym do serwera. Obecnie w Twojej aplikacji możliwe jest podszywanie pod innych użytkowników i kupowanie na "ich konto".
<a class="btn btn-success" asp-controller="Order" asp-action="Order" asp-route-userID="@User.FindFirst(ClaimTypes.NameIdentifier).Value">Kup</a>
Zamiast przesyłania asp-route-userID
w widoku pobierz wartość w kontrolerze: var userId = User.FindFirst(ClaimTypes.NameIdentifier);
Mógłbyś też pozbyć się przekazywania UserId w metodach serwisów poprzez wstrzyknięcie serwisu, który miałby wstrzyknięty IHttpContextAccessor
. Przykładowy kod poniżej:
public sealed class UserContext
{
public int UserId { get; private set; }
public UserContext(IHttpContextAccessor httpContextAccessor)
{
UserId = GetUserId(httpContextAccessor.HttpContext);
}
public static int GetUserId(HttpContext? httpContext)
{
int result = -1;
if (httpContext?.User?.Identity is ClaimsIdentity identity)
{
string? nameIdentifierValue = identity.FindFirst(ClaimTypes.NameIdentifier)?.Value;
int.TryParse(nameIdentifierValue, out result);
}
return result;
}
}
Zamiast:
public async Task<IActionResult> Delete (int productID, int userID)
{
await Task.Run(() =>
{
var orderService = new OrderService();
return orderService.DeleteProduct(productID, userID);
});
return RedirectToAction("Index", new { id = userID });
}
Lepiej tak:
// GET /OrderController/Create
public async Task<IActionResult> Delete(int productID, int userID)
{
var orderService = new OrderService();
await orderService.DeleteProduct(productID, userID);
return RedirectToAction("Index", new { id = userID });
}