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

code review - Laravel/Vue SPA

VPS Starter Arubacloud
0 głosów
288 wizyt
pytanie zadane 28 stycznia 2022 w JavaScript przez Pico Obywatel (1,330 p.)

Witam, w końcu udało mi się uporać z systemem logowania i rejestracji jeśli chodzi o SPA w VUE z użyciem Tokenów z Sanctuma do autoryzacji. Nie wiem tylko czy ma to jakieś ręce i nogi, więc byłbym na prawdę wdzięczny za jakąś recenzję kodu i uwagi co mogę poprawić. Oto link do repozytorium projektu: Haniewicz/SPA-Blog (github.com)

1 odpowiedź

+1 głos
odpowiedź 30 stycznia 2022 przez CSSoup Mądrala (6,460 p.)

Rzuciłem na szybko okiem na twój kod i rzeczy które moim zdaniem możesz zmienić to m.in:

1. Skoro ogarniasz rejestrację i logowanie po stronie Vue + Sanctum, to wywaliłbym wszystko co dostarczyło Ci Laravel/ui* auth, czyli widoki, controllery itd.

2. Samą logikę odpowiedzialną za logowanie i wylogowywanie wywaliłbym z UserController do innego controllera np, LoginController, albo do dwóch mniejszych "single action" controllerów np. LoginController i LogoutController.

3. Hashowanie hasła

       $data = $request->all();
        $check = User::create([
            'name' => $data['name'],
            'password' => Hash::make($data['password']), // <= O tutaj :D
            'email' => $data['email'],
        ]);

Możesz przenieść do mutatora w modelu User:

<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Model;

class User extends Model
{
    // Tutaj
    public function setPasswordAttribute($value)
    {
        $this->attributes['password'] = Hash::make($value);
    }
}

Dzięki czemu przy każdym tworzeniu użytkownika, wystarczy że podasz "gołe" hasło, a reszta wykona "pod maską":

       $data = $request->all();
        $check = User::create([
            'name' => $data['name'],
            'password' => $data['password'], 
            'email' => $data['email'],
        ]);

// Tutaj ładniejsza wersja
User::create($request->all());

// A tutaj ładniejsza i na pewno BEZPIECZNIEJSZA
User::create($request->validated());

4. Jak wspomniałem wyżej jeżeli używasz FormRequestów i chcesz przekazać od razu do metody create(), wszystkie pola z requestu, nigdy nie rób tego przy pomocy, $request->all(), ponieważ zwraca ona jak sama nazwa wskazuje WSZYSTKIE pola, wysłane  requeście, więc dla lepszego zobrazowania załóżmy że masz w  modelu User taką tablicę $fillable:

protected $fillable = ['name', 'password','email', 'is_admin'];

Twój formularz rejestracji zawiera tylko 3 inputy: name, password, email. Nic nie stoi jednak na przeszkodzie, aby ktoś dodał sobie w htmlu dodatkowego inputa "is_admin" i przekazał tam wartość 1, dzięki temu: Bang - został adminem.

Jednak gdy użyjesz $request->validated(), wtedy ta metoda zwróci jedynie pola uwzględnione w metodzie rules() w twoim FormRequeście

    public function rules()
     {
       // Nie ma tutaj pola "is_admin", więc zostanie pominiete
         return [
             'name' => 'required',
             'password' => 'required',
             'email' => 'required'
         ];
     }

4. Zauważyłem, że tworzysz specjalnie zmienną $success po to aby określić status odpowiedzi, jest to jednak niepotrzebne bo do tego służą kody odpowiedzi HTTP, więc na przykładzie:

// Przykładowe logowanie

public function login(LoginRequest $request) {

    if(!Auth::attempt($request->only(['name', 'password']))) {
      // ... Some magic logic

     return response()->json(['message' => 'Invalid credentials'] ,401); // Kod zamiast zmiennej
     }

    // ... 
     return response()->json(['message' => 'Ok!'], 200); // A tutaj kolejny
}

A wtedy w Vue możesz zrobić taki myk z axiosem:

axios.post(twoj_url_do_logowania, credentials)
.then(res => {
     console.log(res.status) // 200
}).catch(e => {
     console.log(e.response.status) // 401
})


 

komentarz 30 stycznia 2022 przez Pico Obywatel (1,330 p.)
Szczerze to przyznam że bardziej mi zależało na ocenie kodu od strony VUE bo tego dopiero się uczę, a backend na pewno będzie do poprawy tylko teraz jest zrobiony na odwal żebym miał bazę pod eksperymenty z tym VUE
komentarz 31 stycznia 2022 przez CubeStorm Pasjonat (15,020 p.)
return response()->json(['message' => 'Ok!'], 200);

Wartość 200 dla statusu jest defaultowa, więc nie trzeba jej podawać

Podobne pytania

0 głosów
0 odpowiedzi 166 wizyt
pytanie zadane 8 marca 2021 w JavaScript przez JWwvZVSu Nowicjusz (120 p.)
+1 głos
1 odpowiedź 169 wizyt
pytanie zadane 16 stycznia 2023 w JavaScript przez mi-20 Stary wyjadacz (13,190 p.)
0 głosów
1 odpowiedź 200 wizyt
pytanie zadane 30 czerwca 2021 w JavaScript przez Łukasz Sitnik Początkujący (380 p.)

92,452 zapytań

141,262 odpowiedzi

319,077 komentarzy

61,854 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!

...