Angular Frontend-Anmeldelogik

Question

Dies ist ein Follow-up auf diese Frage Node.js Backend Login Logik . Ich habe die folgende Login-Angular-Frontend-Logik für mein Node.js-Backend geschrieben (siehe die vorherige Frage oben). Ist es gut in Bezug auf Sicherheit, Effizienz, Gebäude, Async / Sync, Protokollierung? SICHERHEIT ist mein Hauptanliegen. In einem schöneren Format wäre die Frage:

• SICHERHEIT : Ist meine Website in irgendeiner Weise, in irgendeiner Form oder in irgendeiner Form sicher? Ich frage mich, ob ich andere Sicherheitsmaßnahmen als die implementieren könnte, die in die von bereitgestellten Methoden integriert sind Angular. Ist die Übermittlung des Passworts im Klartext nicht ein Sicherheitsproblem? Was ist mit XSS und ähnlichen Problemen? Kann mein Login nicht einfach umgangen werden? Das wäre ein kritischer Fehler.
• EFFIZIENZ : Ist die Überprüfung von Benutzernamen und Passwort effizient? Gibt es einen besseren Weg, dies zu tun?
• GEBÄUDE : Ist es akzeptabel, wie ich meine Website geladen habe?
• ASYNC / SYNC : Ich weiß, dass ich gleichzeitig vorformuliereasync und anrufesync . Gibt es ein Problem damit?
• LOGGING : Ich protokolliere alle Verbindungen zum Server und alle Anmeldeversuche. Ist dies eine gute Vorgehensweise oder übertreibe ich, was die Protokollierung bewirken soll?
• MISC : Gibt es Fehler im Spiel zwischen Backend und Frontend? Wenn ich einige andere wichtige Punkte über den Code vergessen hätte, würde ich mich freuen, wenn Sie sie auch erwähnen würden (Quelle: Login Server with Node.js )

Mein Code :

authentication.service.ts:

import { Injectable } from '@angular/core';
import { HttpClient } from '@angular/common/http';
import { BehaviorSubject, Observable } from 'rxjs';
import { map } from 'rxjs/operators';

import { environment } from '../../environments/environment';
import { User } from '../models/user.model';
import { Router } from '@angular/router';
import { GlobalDataService } from './global-data.service';

@Injectable({ providedIn: 'root' })
export class AuthenticationService {
    constructor(private http: HttpClient,
                private router: Router, public DataService: GlobalDataService) {
        this.currentUserSubject = new BehaviorSubject<User>(JSON.parse(localStorage.getItem('currentUser')));
        this.currentUser = this.currentUserSubject.asObservable();
        this.LoggedIn = true;
    }
    public LoggedIn = true;
    public get currentUserValue(): User {
        return this.currentUserSubject.value;
    }
    private currentUserSubject: BehaviorSubject<User>;
    public currentUser: Observable<User>;
  getRedirectUrl() {
    throw new Error('Method not implemented.');
  }
  isUserLoggedIn() {
    throw new Error('Method not implemented.');
  }

  login(email: string, password: string) {
    return this.http.post<any>(`${environment.apiUrl}/api/login`, { email, password }, {withCredentials: true})
        .pipe(map(user => {
            // login successful if there's a jwt token in the response
            if (user && user.token) {

                // store user details and jwt token in local storage to keep user logged in between page refreshes
                // https://dev.to/rdegges/please-stop-using-local-storage-1i04
                localStorage.setItem('currentUserToken', JSON.stringify(user));
                this.currentUserSubject.next(user);
            }
            // set firstname & email of loggedin user
            this.DataService.loggedinfirstname = user['firstname'];
            this.DataService.loggedinemail = user['eMail'];
            this.redirtoDashboard();
            this.Toolbar();
            this.DataService.prefillSenderData();
            return user;
        }));
  }

  redirtoDashboard() {
      this.router.navigate(['order']);
  }

  Toolbar() {
      this.LoggedIn = !this.LoggedIn;
  }
}

login.component.ts:

import { Component, OnInit } from '@angular/core';
import { ActivatedRoute, Router } from '@angular/router';
import { FormBuilder, FormGroup, Validators } from '@angular/forms';
import { first } from 'rxjs/operators';

import { AuthenticationService } from '../services/authentication.service';

@Component({
  selector: 'app-login',
  templateUrl: './login.component.html',
  styleUrls: ['./login.component.css']
})
export class LoginComponent implements OnInit {

  returnUrl: string;
  loginForm: FormGroup;
  submitted = false;
  error = '';
  loading = false;
  public errorMsg = 'Please login to continue.';
  public redirected: boolean;
  public utm_source: string;

  constructor(private router: Router, private formBuilder: FormBuilder,
              private authenticationService: AuthenticationService, private activatedRoute: ActivatedRoute) {
      if (this.authenticationService.currentUserValue) {
        this.router.navigate(['order']);
    }
      this.activatedRoute.queryParams.subscribe(params => {
      const param = params['utm_source'];

      if (param === 'order' || param === 'work-document' || param === 'profile') {
        this.redirected = true;
        this.utm_source = param;
      } else {
        this.redirected = false;
      }
  });
  }

  ngOnInit(): void {
    this.loginForm = this.formBuilder.group({
      email: ['', [Validators.required, Validators.email]],
      password: ['', [Validators.required, Validators.minLength(6)]]
  });
  }

// convenience getter for easy access to form fields
get f() { return this.loginForm.controls; }

  onSubmit(loginsubmit) {
    this.submitted = true;
    // stop here if form is invalid
    if (this.loginForm.invalid) {
        return console.log('LoginForm Invalid');
    }
    this.loading = true;
    this.authenticationService.login(this.f.email.value, this.f.password.value)
        .pipe(first())
        .subscribe(
            data => {
                if (this.redirected) {
                  this.router.navigate([this.utm_source]);
                } else {
                  this.router.navigate(['order']);
                }

            },
            error => {
                console.log('Login->authservice->err: ', error);
                this.error = error;
                this.loading = false;
            });
}

}

login.component.html:

<div class="container">
  <div class="row">
    <div class="col-sm-9 col-md-7 col-lg-5 mx-auto">
      <div class="card card-signin my-5">
        <div class="card-body">
          <h5 class="card-title text-center">Login</h5>
          <br>
            <form [formGroup]="loginForm" class="form-signin" (ngSubmit)="onSubmit(this.loginForm.value)">
              <div class="form-label-group">
                <input #userName formControlName="email" type="text" id="inputUser" class="form-control" placeholder="E-Mail" required autofocus [ngClass]="{ 'is-invalid': submitted && f.email.errors }">
                  <div *ngIf="submitted && f['email'].errors" class="invalid-feedback">
                    <div *ngIf="f['email'].errors.required">E-Mail is required</div>
                  </div>
                </div>
                <br>
                  <div class="form-label-group">
                    <input #password type="password" formControlName="password" id="inputPassword" class="form-control" placeholder="Password" required [ngClass]="{ 'is-invalid': submitted && f.password.errors }">
                      <div *ngIf="submitted && f['password'].errors" class="invalid-feedback">
                        <div *ngIf="f['password'].errors.required">Password is required</div>
                      </div>
                    </div>
                    <br>
                      <div *ngIf="redirected">
                        <mat-error>
                          <p class="alert alert-danger">
                            {{errorMsg}}
                          </p>
                        </mat-error>
                      </div>
                      <button [disabled]="!loginForm.valid" class="btn btn-dark btn-block" id="loginSubmit" type="submit">Login</button>
                      <div class="forgot-password-link">
                        <a routerLink="/forgot-password">Forgot password</a>
                      </div>
                    </form>
                  </div>
                </div>
              </div>
            </div>
          </div>

Answer 1

Entschuldigung, ich habe mir Ihren Backend-Code nicht angesehen, daher ist dies nur eine halbe Bewertung.

Für Ihre Fragen:

• Sicherheit
  Wie Sam bereits analysiert hat, gibt es im Frontend-Teil kein offensichtliches Problem (solange Sie mit HTTPS arbeiten). Ich gehe davon aus, dass das Passwort im Backend (mit einem Salt) gehasht wird und nur der Hash in der Datenbank gespeichert ist, sodass niemand die echten Passwörter aus der Datenbank extrahieren kann.

• Effizienz
  Ja, Validatoren sind der richtige Weg in Angular. Es wäre technisch etwas leistungsfähiger, HTML-Validierung zu verwenden, aber diese Fraktale von Millisekunden sind es definitiv nicht wert, die Flexibilität von Validatoren zu verlieren.

• Gebäude

• Async / Sync
  Das Problem beim Aufrufen einer asynchronen Methode ohne Behandlung des Ergebnisses besteht darin, dass Sie nicht feststellen, ob etwas nicht wie erwartet funktioniert hat. Das ist in Ordnung, wenn Sie wissen, dass der von Ihnen verwendete Code von einem gottähnlichen Entwickler entwickelt und gepflegt wurde, der über den Fehlern steht. Wenn der Entwickler ein Mensch ist, sollten Sie immer damit rechnen, dass ein Problem vorliegt. Und wenn Sie eine Methode aufrufen und wissen, dass es kein Problem mit Ihrem Code gibt, machen Sie es in Ihrem Code explizit, damit die folgenden Entwickler (z. B. Sie in 3 Monaten) dies ebenfalls wissen. :-)

• Protokollierung
  Die Frage ist, was Sie mit der Protokollierung beabsichtigen. Wenn Sie alles überwachen möchten, um Informationen über Ihre Benutzer, Ihre Infrastruktur usw. usw. zu erhalten, ist dies in Ordnung. Okay, ich würde dann eines der vorhandenen Frameworks verwenden, um das für mich zu tun und das Rad nicht neu zu erfinden.
  Wenn Sie nur an den schlechten Dingen interessiert sind, würde ich nur diese protokollieren (wie fehlgeschlagene Anmeldeversuche).
  Wie immer gibt es kein "JA" oder "NEIN". Es hängt von Ihrer Absicht ab.
  Als Klarstellung console.logwird für mich nicht "protokolliert", da es nur für den Benutzer und für ihn nur sichtbar ist, wenn er die Konsole geöffnet hat.

• VERSCHIEDENES
  Siehe Folgendes

Refactoring

Ich möchte den Code zunächst ein wenig überarbeiten, um die Lesbarkeit zu gewährleisten. Nach meiner Erfahrung ist es einfacher, Fehler zu erkennen. Sie können es ignorieren und zum interessanten Teil springen, wenn Sie möchten.

Im Allgemeinen verwende ich wirklich gerne immer privateund publicund so restriktiv wie möglich sein. Es zeigt dem Leser, dass ich über den Umfang einer Methode / Variablen nachgedacht habe. Und es verringert die Wahrscheinlichkeit eines Missbrauchs. Wenn ich nicht sicher bin, dann beginne ich mit private.
Wenn nichts verwendet wird, ist es standardmäßig öffentlich. Und als Leser weiß ich dann nicht, ob der Entwickler das absichtlich gewählt oder einfach vergessen hat.

AuthenticationService

LoggedInwird zur Definitionszeit ( public LoggedIn = true) und erneut im Konstruktor auf true gesetzt . Ich persönlich bevorzuge die Zuweisung des Anfangswertes zum Zeitpunkt der Definition. Auch dieser Wert wird als "true bedeutet nicht angemeldet" verwendet. Das ist irritierend, deshalb würde ich den Namen ändern isLogedInund ihn mit false initialisieren.

DataService ist öffentlich, scheint aber nicht außerhalb der Klasse verwendet zu werden.

Ich mag keine konstanten Zeichenfolgen in meinem Code, deshalb extrahiere ich sie in Konstanten. Mögen
private loginUrl: string = `${environment.apiUrl}/api/login`;

Ich verwende gerne "sprechende" RxJs-Operatoren. Die mapMethode in loginändert den Stream nicht, daher würde ich tapstattdessen verwenden. Das macht deutlich, dass es nur einen Nebeneffekt gibt. "Auch dann könnten wir die returnZeile überspringen .

Ich mag Speedreading-Code. Wenn ich also einige Codezeilen in eine sprechende Methode extrahieren kann, mache ich das, weil ich dann nur den Methodennamen lesen muss, nicht den gesamten Code dahinter und entscheiden kann, ob ich tiefer tauchen oder einfach weitermachen möchte.
Dafür würde ich den Code in mapder loginMethode etwas ändern .

Es scheint auch, dass die "Symbolleisten" -Methode nur einmal verwendet wird, um den LogedInStatus zu ändern . Wir könnten also einfach den Wert einstellen, ohne umzuschalten.
Es gibt hier auch ein Problem (siehe weiter unten in diesem Kapitel), daher werde ich den Aufruf dieser Funktionalität in die if-Anweisung verschieben.

Normalerweise ist die Umleitung das Letzte, was passieren sollte, daher verschiebe ich sie an das Ende des Hahns

export class AuthenticationService {
    public isLoggedIn = false;
    public currentUser: Observable<User>

    public get currentUserValue(): User {
        return this.currentUserSubject.value;
    }    
    
    private currentUserSubject: BehaviorSubject<User> = new BehaviorSubject<User>(JSON.parse(localStorage.getItem('currentUser')));
    private loginUrl: string = `${environment.apiUrl}/api/login`;

    constructor(private DataService: GlobalDataService,
                private http: HttpClient,
                private router: Router) {
        this.currentUser = this.currentUserSubject.asObservable();
    }
       
  private getRedirectUrl() {
    throw new Error('Method not implemented.');
  }
  
  private isUserLoggedIn() {
    throw new Error('Method not implemented.');
  }

  public login(email: string, password: string):Observable<User> {
    return this.http.post<User>(loginUrl, { email, password }, {withCredentials: true})
        .pipe(
            tap(user => {
            // login successful if there's a jwt token in the response
            if (this.isLoginSuccessful(user)) {
                this.setLogedInUser(user);
                this.isLoggedIn = true;
            }
            this.setDataServiceForUser(user);            
            this.toggleLoginStatus();
            this.redirectToDashboard();
        })
    );
  }

    private isLoginSuccessful(user:User):boolean{
        return user && user.token;
    }

  private redirectToDashboard():void {
      this.router.navigate(['order']);
  }

    private setLogedInUser(user: User):void{
        // store user details and jwt token in local storage to keep user logged in between page refreshes
        // https://dev.to/rdegges/please-stop-using-local-storage-1i04
        localStorage.setItem('currentUserToken', JSON.stringify(user));
        this.currentUserSubject.next(user);
    }

    private setDataServiceForUser(user:User):void{
        // set firstname & email of loggedin user
        this.DataService.loggedinfirstname = user['firstname'];
        this.DataService.loggedinemail = user['eMail'];
        this.DataService.prefillSenderData();
    }
}

LoginComponent

In onSubmitbrauchst du keine first(). Dahinter this.authenticationService.loginsteht eine http-Anfrage. Und diese enden automatisch nach dem ersten Ergebnis. Aus dem gleichen Grund müssen Sie diese Abonnements nicht abbestellen.

Probleme

AuthenticationService

Bei der loginMethode scheint es, dass selbst im Fall einer nicht erfolgreichen Anmeldung (kein Benutzer oder keine Token-Informationen) immer noch versucht wird, Daten in das zu senden DataServiceund Dinge zu tun. Hier würde ich sehr klar die Dinge trennen, die IMMER nach einem Anmeldeversuch passieren sollten, und diejenigen, die nur nach einem erfolgreichen Login passieren können.
Insbesondere wird es sich derzeit ändern, LogedInauch wenn die Anmeldung nicht erfolgreich war.

LoginComponent

Empfohlene Vorgehensweise

Hier einige Best Practices (zumindest in meinen Augen :-))

Im Code werden die Benutzerinformationen für zwei Dinge verwendet. Für detaillierte Informationen über den Benutzer und zweitens als implizites "der Benutzer ist angemeldet". Im AuthentificationService ist diese Verbindung gültig. Aber nach außen würde ich Benutzerinformationen und zusätzlich eine "isLogedIn" -Information bereitstellen. Auf diese Weise muss ein Entwickler nicht "wissen", dass Benutzerinformationen bedeuten, dass ein Benutzer angemeldet ist.

Es ist eine gute Angewohnheit, sich abzumelden, wenn Sie die Komponente verlassen. Dafür mache ich normalerweise so etwas

private subscriptions: Subscription() = new Subscription();
...
this.subscriptions.add(
    sourceA.subscribe(...)
)
this.subscriptions.add(
    sourceB.subscribe(...)
)

ngOnDestroy(){
    this.subscriptions.unsubcribe();
}

Auf diese Weise werden alle Abonnements automatisch abgemeldet, sobald die Komponente zerstört wird. Beachten Sie, dass eine Komponente nur zerstört wird, wenn sie vollständig aus dem DOM entfernt wurde. Wenn es versteckt ist, lebt es noch.

Ich hoffe, ein oder zwei Dinge waren hilfreich für Sie.

herzliche Grüße

Answer 2

Vorwort

Ich habe AngularJS vor ein paar Jahren verwendet, bin aber nicht in Angular2 + eingestiegen, daher ist mein Wissen darüber schlank bis gar nicht. Ich bin jedoch ziemlich gut mit Javascript und verschiedenen Frameworks vertraut.

Fragen Antworten

Sicherheit Ist die Übertragung des Passworts im Klartext nicht ein Sicherheitsproblem?

Ich habe Beiträge zu dieser Frage auf mehreren SE-Websites gefunden. Zum Beispiel fand ich Ist es in Ordnung, ein Klartextkennwort über HTTPS zu senden? [Duplikat] . Um die akzeptierte Antwort von Buffalo5ix zu zitieren :

Es ist üblich, "Klartext" -Kennwörter über HTTPS zu senden. Die Passwörter sind letztendlich kein Klartext, da die Client-Server-Kommunikation gemäß TLS verschlüsselt ist.

Diese Frage ist als Duplikat von zwei anderen Posts markiert, einschließlich dieses: Ich sende nur Benutzername und Passwort über https. Ist das ok? . Es hat zwei Antworten und die zweite Antwort von Steve bietet eine Option:

Eine weitere Möglichkeit besteht darin, Client-Zertifikate zu verwenden. Der Server kann sich nur dann selbst garantieren, dass kein MitM vorhanden ist, wenn er ein Client-Zertifikat benötigt. Andernfalls muss er dem Kunden vertrauen, um das Fehlen eines MitM ordnungsgemäß zu überprüfen. Dies ist mehr als viele Dienste, denen man vertrauen sollte.

Ich habe noch nie von jemandem gehört, der das tut, aber vielleicht ist es getan und wir wissen einfach nichts davon.

Es gibt sogar eine Stapelüberlauffrage zu dieser Frage , wobei die akzeptierte Antwort der akzeptierten Antwort der oben erwähnten ersten Frage (von Security SE) sehr ähnlich ist.

EFFIZIENZ Wie überprüfe ich Benutzernamen und Passwort effizient? Gibt es einen besseren Weg, dies zu tun?

Mir ist kein besserer Weg bekannt, dies zu tun, aber ich bemerke diese Zeilen darin, AuthenticationService.login()dass die Punktnotation verwendet werden sollte:

 this.DataService.loggedinfirstname = user['firstname'];
 this.DataService.loggedinemail = user['eMail'];

Methodenname

Ich fragte nach der Aktualisierung der Eigenschaft LoggedInund stellte fest, dass sie im Konstruktor festgelegt und dann in der Methode geändert wurde Toolbar. Ein Name wie Toolbarscheint mit dem Abrufen einer Symbolleiste verbunden zu sein. Die anderen Methoden in dieser Klasse haben ein Verb - zB login , redirtoDashboard . Ein geeigneterer Methodenname für diese Methode könnte ToggleLoggedInoder etwas in dieser Richtung sein.

Vereinfachung der OP-Bedingungen

Diese Zeile in LoginComponent::constructor():

       if (param === 'order' || param === 'work-document' || param === 'profile') {

könnte vereinfacht werden, indem Array.prototype.includes()ein strenger Vergleich durchgeführt wird ^{1 2} :

  if ([ 'order', 'work-document', 'profile'].includes(param)) {