• Najnowsze pytania
  • Bez odpowiedzi
  • Zadaj pytanie
  • Kategorie
  • Tagi
  • Zdobyte punkty
  • Ekipa ninja
  • IRC
  • FAQ
  • Regulamin
  • Książki warte uwagi

Ocena kodu ASP NET Core

VPS Starter Arubacloud
0 głosów
273 wizyt
pytanie zadane 11 października 2020 w Nasze projekty przez Szyszka Gaduła (3,490 p.)
Siemka, jakoś tydzien, może dwa uczę się na własną ręke ASP NET Core. W końcu zrobiłem jakieś tam logowanie z uwierzytelnianiem uzytkownika. Oto jest kod: https://github.com/Szyszka947/netcore

Prosił bym o ocenę tego kodu, i może jakieś poprawki. Szczególnie zależy mi na ocenie pliku LoginController.cs, gdyż wydaje mi się, że można go napisać lepiej, ale w sumie, to nie wiem jak :|. Dzięki wszystkim!

1 odpowiedź

+1 głos
odpowiedź 11 października 2020 przez adrian17 Ekspert (344,100 p.)

Nie ukrywam że nie znam dobrze ASP.NETa, ale na oko:

<input type="password" asp-for="PasswordHash"/>|>Hasło<br />

Nie. Użytkownik ma tutaj wpisać zwykłe hasło, hashowanie powinno się dziać już po stronie serwera, przy rejestracji przez przekazanie hasła do UserManager.CreateAsync oraz przy logowaniu, używając CheckPasswordSignInAsync albo PasswordSignInAsync.

        [HttpPost]
        public async Task<IActionResult> Post([FromForm] AppUser appUser)

nie udawaj, że to user. Wyciągnij login i hasło jak zwykłe parametry i wyciągnij usera z bazy tak:

var user = UserManager.FindByNameAsync(userName);

 

komentarz 11 października 2020 przez adrian17 Ekspert (344,100 p.)

A na boku, co do samego stylu kodu:

var getUserNameFromDb = Context.Users.Where(p => p.UserName == appUser.UserName).Select(p => p.UserName).FirstOrDefault();
var getPasswordFromDb = Context.Users.Where(p => p.UserName == appUser.UserName).Select(p => p.PasswordHash).FirstOrDefault();
var getUserIDFromDb = Context.Users.Where(p => p.UserName == appUser.UserName).Select(p => p.Id).FirstOrDefault();

Wyżej pisałem że to nie ma sensu, ale jeśli już to robić, to... czemu nie wyciągnąć usera z bazy raz, a potem tylko używać jego pola?

                            if (getUserNameFromDb == (object)null)
                            {
                                if (getEmailFromDb == (object)null)
                                {
                                    if (appUser.Email != (object)null)

Może odwrócisz te warunki? Bo to jest dość straszny łańcuszek...

Ta konwersja (object)null też nie jest potrzebna.

Inna sprawa, że cały ten blok kodu też nie jest potrzebny, bo ASP.NET ma wbudowane mechanizmy do walidacji formularzy. Rzuć okiem np na

https://docs.microsoft.com/en-us/aspnet/mvc/overview/getting-started/introduction/adding-validation

komentarz 11 października 2020 przez JakSky Stary wyjadacz (14,770 p.)
Dodatkowo jest już gotowy system logowania i rejestracji wspierający Oauth2 itp. Asp.net Core Identity.
komentarz 11 października 2020 przez Szyszka Gaduła (3,490 p.)
Dzięki wielkie! Co do PasswordHash, to tak, wiem, że to powinno być zwykłe Password, ale IdentityUser daje mi tylko PasswordHash. Za jakiś czas dodam hashowanie hasła. Dzięki raz jeszcze!
komentarz 11 października 2020 przez JakSky Stary wyjadacz (14,770 p.)
Do hashowania najlepiej chyba użyć klasy rfc2898derivebytes. Pamiętaj tylko, aby wywołać metodę Dispose. Czyli użyć słowa kluczowego using, albo try finally
komentarz 11 października 2020 przez Szyszka Gaduła (3,490 p.)
Myślałem nad BCrypt, nie będzie lepszy? :D
komentarz 11 października 2020 przez adrian17 Ekspert (344,100 p.)
edycja 11 października 2020 przez adrian17

BCrypt

rfc2898derivebytes

wait, ale o tym w ogóle się nie myśli. W ogóle.

ale IdentityUser daje mi tylko PasswordHash

Nie potrzebujesz ani Password, ani PasswordHash.

Za jakiś czas dodam hashowanie hasła

Nie musisz nic ręcznie dodawać.

Jeszcze raz, tak jak napisałem wyżej:

Użytkownik ma tutaj wpisać zwykłe hasło, hashowanie powinno się dziać już po stronie serwera, przy rejestracji przez przekazanie hasła do UserManager.CreateAsync oraz przy logowaniu, używając CheckPasswordSignInAsync albo PasswordSignInAsync.

Na przykład:

var newUser = new AppUser
{
  UserName = "adrian",
  Email = "adrian@adrian.com"
};

var result = await UserManager.CreateAsync(newUser, "Adrian1Lubi2Psy3");

CreateAsync z drugim argumentem automatycznie hashuje hasło i je ustawia userowi. Tyle, nie ma wybierania algorytmów hashowania ani nic takiego.
(disclaimer: napisane na suchu)

Podobne pytania

0 głosów
0 odpowiedzi 139 wizyt
pytanie zadane 24 października 2020 w C# przez Szyszka Gaduła (3,490 p.)
0 głosów
1 odpowiedź 281 wizyt
pytanie zadane 30 lipca 2019 w Nasze projekty przez mi-20 Stary wyjadacz (13,190 p.)
0 głosów
1 odpowiedź 211 wizyt
pytanie zadane 16 maja 2019 w Nasze projekty przez mi-20 Stary wyjadacz (13,190 p.)

92,451 zapytań

141,261 odpowiedzi

319,073 komentarzy

61,853 pasjonatów

Motyw:

Akcja Pajacyk

Pajacyk od wielu lat dożywia dzieci. Pomóż klikając w zielony brzuszek na stronie. Dziękujemy! ♡

Oto polecana książka warta uwagi.
Pełną listę książek znajdziesz tutaj.

Akademia Sekuraka

Akademia Sekuraka 2024 zapewnia dostęp do minimum 15 szkoleń online z bezpieczeństwa IT oraz dostęp także do materiałów z edycji Sekurak Academy z roku 2023!

Przy zakupie możecie skorzystać z kodu: pasja-akademia - użyjcie go w koszyku, a uzyskacie rabat -30% na bilety w wersji "Standard"! Więcej informacji na temat akademii 2024 znajdziecie tutaj. Dziękujemy ekipie Sekuraka za taką fajną zniżkę dla wszystkich Pasjonatów!

Akademia Sekuraka

Niedawno wystartował dodruk tej świetnej, rozchwytywanej książki (około 940 stron). Mamy dla Was kod: pasja (wpiszcie go w koszyku), dzięki któremu otrzymujemy 10% zniżki - dziękujemy zaprzyjaźnionej ekipie Sekuraka za taki bonus dla Pasjonatów! Książka to pierwszy tom z serii o ITsec, który łagodnie wprowadzi w świat bezpieczeństwa IT każdą osobę - warto, polecamy!

...