Merge pull request #1 from AXeL-dev/enforce-user-security

AXeL-dev · web-flow · commit a76542f60903 · 2020-03-24T20:10:01.000+01:00
Enforce user security
diff --git a/README.md b/README.md
@@ -156,20 +156,72 @@ npm install --save ng-fire-admin
 
 **7**. In order to protect your database & storage data, you must set the following rules in your firebase console:
 
+<details>
+  <summary>How to setup your firebase project?</summary>
+  
+  - Start by adding a new project in your firebase console.
+
+  - Enable Authentication by email & password.
+
+  - Add a database to your project.
+
+  - Add a storage.
+</details>
+
 **Firestore Database rules:**
 
 ```javascript
 rules_version = '2';
 service cloud.firestore {
   match /databases/{database}/documents {
-    match /{collection}/{document=**} {
-      allow read: if collection != 'users' || request.auth != null;
-      allow write: if request.auth != null;
+    match /{collection}/{document}/{path=**} {
+      allow read: if isPublic(collection, document) || isAdmin();
+      allow write: if registrationEnabled(collection) || isAdmin() || (isEditor() && isPublic(collection, document));
+    }
+    function isPublic(collection, document) {
+      return collection != 'config' && (collection != 'users' || isOwner(document));
+    }
+    function isSignedIn() {
+      return request.auth != null;
+    }
+    function hasRole(role) {
+      return isSignedIn() && get(/databases/$(database)/documents/users/$(request.auth.uid)).data.role == role;
+    }
+    function isAdmin() {
+      return hasRole('admin');
+    }
+    function isEditor() {
+      return hasRole('editor');
+    }
+    function isOwner(ownerId) {
+      return isSignedIn() && request.auth.uid == ownerId;
+    }
+    function registrationEnabled(collection) {
+      return collection == 'users' && (
+        !exists(/databases/$(database)/documents/config/registration) ||
+        get(/databases/$(database)/documents/config/registration).data.enabled
+      );
     }
   }
 }
 ```
 
+<details>
+  <summary>More basic database rules? (not recommended)</summary>
+  
+  ```javascript
+    rules_version = '2';
+    service cloud.firestore {
+      match /databases/{database}/documents {
+        match /{collection}/{document=**} {
+          allow read: if collection != 'users' || request.auth != null;
+          allow write: if request.auth != null;
+        }
+      }
+    }
+  ```
+</details>
+
 **Storage rules:**
 
 ```javascript
diff --git a/projects/fire-admin/src/lib/components/dashboard/dashboard.component.html b/projects/fire-admin/src/lib/components/dashboard/dashboard.component.html
@@ -44,7 +44,7 @@ <h6 class="stats-small__value count my-3">{{ statistics?.comments || 0 }}</h6>
         </div>
       </div>
     </div>
-    <div class="col-lg col-md-4 col-sm-6 mb-4">
+    <div class="col-lg col-md-4 col-sm-6 mb-4" *ngIf="currentUser?.isAdmin()">
       <div class="stats-small stats-small--1 card card-small">
         <div class="card-body p-0 d-flex">
           <div class="d-flex flex-column m-auto">
@@ -56,7 +56,7 @@ <h6 class="stats-small__value count my-3">{{ statistics?.users || 0 }}</h6>
         </div>
       </div>
     </div>
-    <div class="col-lg col-md-4 col-sm-12 mb-4">
+    <div class="col-lg col-md-4 mb-4" [ngClass]="currentUser?.isAdmin() ? 'col-sm-12' : 'col-sm-6'">
       <div class="stats-small stats-small--1 card card-small">
         <div class="card-body p-0 d-flex">
           <div class="d-flex flex-column m-auto">
diff --git a/projects/fire-admin/src/lib/components/dashboard/dashboard.component.ts b/projects/fire-admin/src/lib/components/dashboard/dashboard.component.ts
@@ -13,6 +13,7 @@ import { map, takeUntil } from 'rxjs/operators';
 import { NavigationService } from '../../services/navigation.service';
 import { initPieChart } from '../../helpers/charts.helper';
 import { I18nService } from '../../services/i18n.service';
+import { CurrentUserService } from '../../services/current-user.service';
 
 type PostByStatus = {
   label: string,
@@ -47,6 +48,7 @@ export class DashboardComponent implements OnInit, OnDestroy {
     private categories: CategoriesService,
     private settings: SettingsService,
     public navigation: NavigationService,
+    public currentUser: CurrentUserService,
     private i18n: I18nService
   ) { }
 
@@ -88,7 +90,9 @@ export class DashboardComponent implements OnInit, OnDestroy {
     this.statistics.posts = await this.posts.countAll();
     this.statistics.pages = await this.pages.countAll();
     this.statistics.comments = 0; // ToDo
-    this.statistics.users = await this.users.countAll();
+    if (this.currentUser.isAdmin()) {
+      this.statistics.users = await this.users.countAll();
+    }
     this.statistics.translations = await this.translations.countAll();
   }
 
diff --git a/projects/fire-admin/src/lib/components/pages/list/pages-list.component.html b/projects/fire-admin/src/lib/components/pages/list/pages-list.component.html
@@ -20,7 +20,7 @@ <h3 class="page-title">{{ 'List' | translate }}</h3>
                 <th>{{ 'PageLanguage' | translate }}</th>
                 <th>{{ 'PageCreatedAt' | translate }}</th>
                 <th>{{ 'PageUpdatedAt' | translate }}</th>
-                <th>{{ 'PageAuthor' | translate }}</th>
+                <th *ngIf="currentUser?.isAdmin()">{{ 'PageAuthor' | translate }}</th>
                 <th class="text-right hide-sort-icons">{{ 'Actions' | translate }}</th>
               </tr>
             </thead>
@@ -38,7 +38,7 @@ <h3 class="page-title">{{ 'List' | translate }}</h3>
                 <td>{{ allLanguages[page.lang].label | translate }}</td>
                 <td>{{ page.createdAt | datetime }}</td>
                 <td>{{ page.updatedAt | datetime }}</td>
-                <td>
+                <td *ngIf="currentUser?.isAdmin()">
                   <a *ngIf="page.createdBy" [routerLink]="navigation.getRouterLink('users', 'profile', page.createdBy)">
                     {{ page.author | async }}
                   </a>
diff --git a/projects/fire-admin/src/lib/components/pages/list/pages-list.component.ts b/projects/fire-admin/src/lib/components/pages/list/pages-list.component.ts
@@ -2,7 +2,7 @@ import { Component, OnInit, ViewChild, OnDestroy } from '@angular/core';
 import { DataTableDirective } from 'angular-datatables';
 import { Subject, Subscription, Observable } from 'rxjs';
 import { map, takeUntil } from 'rxjs/operators';
-import { refreshDataTable, clearDataTable } from '../../../helpers/datatables.helper';
+import { refreshDataTable } from '../../../helpers/datatables.helper';
 import { AlertService } from '../../../services/alert.service';
 import { NavigationService } from '../../../services/navigation.service';
 import { I18nService } from '../../../services/i18n.service';
@@ -11,6 +11,7 @@ import { SettingsService } from '../../../services/settings.service';
 import { Language } from '../../../models/language.model';
 import { PagesService } from '../../../services/collections/pages.service';
 import { Page } from '../../../models/collections/page.model';
+import { CurrentUserService } from '../../../services/current-user.service';
 
 @Component({
   selector: 'fa-pages-list',
@@ -38,6 +39,7 @@ export class PagesListComponent implements OnInit, OnDestroy {
     private i18n: I18nService,
     private route: ActivatedRoute,
     public navigation: NavigationService,
+    public currentUser: CurrentUserService,
     private settings: SettingsService
   ) { }
 
@@ -90,8 +92,6 @@ export class PagesListComponent implements OnInit, OnDestroy {
       translationId: page.translationId,
       translations: page.translations
     }).then(() => {
-      clearDataTable(this.dataTableElement);
-      this.isLoading = true;
       this.alert.success(this.i18n.get('PageDeleted', { title: page.title }), false, 5000);
     }).catch((error: Error) => {
       this.alert.error(error.message);
diff --git a/projects/fire-admin/src/lib/components/posts/categories/posts-categories.component.ts b/projects/fire-admin/src/lib/components/posts/categories/posts-categories.component.ts
@@ -56,7 +56,7 @@ export class PostsCategoriesComponent implements OnInit, OnDestroy {
       this.allCategories.subscribe((categories: Category[]) => {
         // console.log(categories);
         // Refresh datatable on data change
-        refreshDataTable(this.dataTableElement, this.dataTableTrigger);
+        refreshDataTable(this.dataTableElement, this.dataTableTrigger, true);
       })
     );
   }
diff --git a/projects/fire-admin/src/lib/components/posts/list/posts-list.component.html b/projects/fire-admin/src/lib/components/posts/list/posts-list.component.html
@@ -23,7 +23,7 @@ <h3 class="page-title">{{ 'List' | translate }}</h3>
                 <th>{{ 'Categories' | translate }}</th>
                 <th>{{ 'PostCreatedAt' | translate }}</th>
                 <th>{{ 'PostUpdatedAt' | translate }}</th>
-                <th>{{ 'PostAuthor' | translate }}</th>
+                <th *ngIf="currentUser?.isAdmin()">{{ 'PostAuthor' | translate }}</th>
                 <th class="text-right hide-sort-icons">{{ 'Actions' | translate }}</th>
               </tr>
             </thead>
@@ -51,7 +51,7 @@ <h3 class="page-title">{{ 'List' | translate }}</h3>
                 </td>
                 <td>{{ post.createdAt | datetime }}</td>
                 <td>{{ post.updatedAt | datetime }}</td>
-                <td>
+                <td *ngIf="currentUser?.isAdmin()">
                   <a *ngIf="post.createdBy" [routerLink]="navigation.getRouterLink('users', 'profile', post.createdBy)">
                     {{ post.author | async }}
                   </a>
diff --git a/projects/fire-admin/src/lib/components/posts/list/posts-list.component.ts b/projects/fire-admin/src/lib/components/posts/list/posts-list.component.ts
@@ -4,7 +4,7 @@ import { Subject, Subscription, Observable } from 'rxjs';
 import { Post, PostStatus } from '../../../models/collections/post.model';
 import { PostsService } from '../../../services/collections/posts.service';
 import { map, takeUntil } from 'rxjs/operators';
-import { refreshDataTable, clearDataTable } from '../../../helpers/datatables.helper';
+import { refreshDataTable } from '../../../helpers/datatables.helper';
 import { AlertService } from '../../../services/alert.service';
 import { NavigationService } from '../../../services/navigation.service';
 import { I18nService } from '../../../services/i18n.service';
@@ -13,6 +13,7 @@ import { CategoriesService } from '../../../services/collections/categories.serv
 import { ActivatedRoute } from '@angular/router';
 import { SettingsService } from '../../../services/settings.service';
 import { Language } from '../../../models/language.model';
+import { CurrentUserService } from '../../../services/current-user.service';
 
 @Component({
   selector: 'fa-posts-list',
@@ -43,6 +44,7 @@ export class PostsListComponent implements OnInit, OnDestroy {
     private i18n: I18nService,
     private route: ActivatedRoute,
     public navigation: NavigationService,
+    public currentUser: CurrentUserService,
     private settings: SettingsService
   ) { }
 
@@ -136,8 +138,6 @@ export class PostsListComponent implements OnInit, OnDestroy {
       translationId: post.translationId,
       translations: post.translations
     }).then(() => {
-      clearDataTable(this.dataTableElement);
-      this.isLoading = true;
       this.alert.success(this.i18n.get('PostDeleted', { title: post.title }), false, 5000);
     }).catch((error: Error) => {
       this.alert.error(error.message);
diff --git a/projects/fire-admin/src/lib/components/translations/translations.component.ts b/projects/fire-admin/src/lib/components/translations/translations.component.ts
@@ -53,7 +53,7 @@ export class TranslationsComponent implements OnInit, OnDestroy, AfterViewInit {
       this.allTranslations.subscribe((translations: TranslationData[]) => {
         // console.log(translations);
         // Refresh datatable on data change
-        refreshDataTable(this.dataTableElement, this.dataTableTrigger);
+        refreshDataTable(this.dataTableElement, this.dataTableTrigger, true);
       })
     );
   }
diff --git a/projects/fire-admin/src/lib/components/users/list/users-list.component.html b/projects/fire-admin/src/lib/components/users/list/users-list.component.html
@@ -44,9 +44,7 @@ <h3 class="page-title">{{ 'List' | translate }}</h3>
                 <td>{{ user.updatedAt | datetime }}</td>
                 <td>
                   <a *ngIf="user.createdBy" [routerLink]="navigation.getRouterLink('users', 'profile', user.createdBy)">
-                    <span *ngIf="user.creator | async as creator">
-                      {{ creator.firstName + ' ' + creator.lastName }}
-                    </span>
+                    <span *ngIf="user.creator | async as creator">{{ creator }}</span>
                   </a>
                 </td>
                 <td class="text-right">
diff --git a/projects/fire-admin/src/lib/components/users/list/users-list.component.ts b/projects/fire-admin/src/lib/components/users/list/users-list.component.ts
@@ -63,7 +63,7 @@ export class UsersListComponent implements OnInit, OnDestroy {
                 url: this.users.getAvatarUrl(user.avatar as string)
               };
               if (user.createdBy) {
-                user.creator = this.users.get(user.createdBy);
+                user.creator = this.users.getFullName(user.createdBy);
               }
             });
             return users.sort((a: User, b: User) => b.createdAt - a.createdAt);
diff --git a/projects/fire-admin/src/lib/helpers/datatables.helper.ts b/projects/fire-admin/src/lib/helpers/datatables.helper.ts
@@ -1,17 +1,17 @@
 import { DataTableDirective } from 'angular-datatables';
 import { Subject } from 'rxjs/internal/Subject';
 
-export function refreshDataTable(dataTableElement: DataTableDirective, dataTableTrigger: Subject<void>) {
-  clearDataTable(dataTableElement).then(() => {
+export function refreshDataTable(dataTableElement: DataTableDirective, dataTableTrigger: Subject<void>, forceClear?: boolean) {
+  clearDataTable(dataTableElement, forceClear).then(() => {
     dataTableTrigger.next();
   });
 }
 
-export function clearDataTable(dataTableElement: DataTableDirective) {
+export function clearDataTable(dataTableElement: DataTableDirective, force?: boolean) {
   return new Promise((resolve, reject) => {
     if (dataTableElement.dtInstance) {
       dataTableElement.dtInstance.then((dtInstance: DataTables.Api) => {
-        if (dtInstance.data().any()) {
+        if (force || dtInstance.data().any()) {
           dtInstance.clear().destroy();
         }
         resolve();
diff --git a/projects/fire-admin/src/lib/models/collections/user.model.ts b/projects/fire-admin/src/lib/models/collections/user.model.ts
@@ -9,8 +9,7 @@ export enum UserRole {
 }
 
 export interface User {
-  id?: string, // document id
-  uid?: string, // firebase user id
+  id?: string, // document id == firebase user id
   firstName: string,
   lastName: string,
   email: string,
@@ -21,7 +20,7 @@ export interface User {
   avatar?: File|string|Observable<string>|{ path: string|any, url: string|Observable<string> },
   createdAt?: number,
   updatedAt?: number,
-  createdBy?: string,
+  createdBy?: string, // creator id
   creator?: string|Observable<string>, // used to fetch creator name without overriding createdBy field
   updatedBy?: string
 }
diff --git a/projects/fire-admin/src/lib/services/collections/categories.service.ts b/projects/fire-admin/src/lib/services/collections/categories.service.ts
@@ -2,6 +2,7 @@ import { Injectable } from '@angular/core';
 import { DatabaseService } from '../database.service';
 import { Category } from '../../models/collections/category.model';
 import { now } from '../../helpers/functions.helper';
+import { map } from 'rxjs/operators';
 
 @Injectable()
 export class CategoriesService {
@@ -22,7 +23,10 @@ export class CategoriesService {
   }
 
   get(id: string) {
-    return this.db.getDocument('categories', id);
+    return this.db.getDocument('categories', id).pipe(map((category: Category) => {
+      category.id = id;
+      return category;
+    }));
   }
 
   getAll() {
diff --git a/projects/fire-admin/src/lib/services/collections/translations.service.ts b/projects/fire-admin/src/lib/services/collections/translations.service.ts
@@ -70,7 +70,7 @@ export class TranslationsService {
     return new Promise((resolve, reject) => {
       this.db.getDocumentRef('translations', lang).get().toPromise().then((doc) => {
         const translations = doc.data();
-        if (translations[key]) {
+        if (translations && translations[key]) {
           resolve(true);
         } else {
           resolve(false);
diff --git a/projects/fire-admin/src/lib/services/collections/users.service.ts b/projects/fire-admin/src/lib/services/collections/users.service.ts
@@ -47,8 +47,7 @@ export class UsersService {
     };
     return new Promise((resolve, reject) => {
       this.firebaseUser.create(data.email, data.password).then((uid: string) => {
-        user.uid = uid;
-        this.uploadImageAfter(this.db.addDocument('users', user), user, data).then(() => {
+        this.uploadImageAfter(this.db.addDocument('users', user, uid), user, data).then(() => {
           resolve();
         }).catch((error: Error) => {
           reject(error);
@@ -110,7 +109,10 @@ export class UsersService {
   }
 
   get(id: string) {
-    return this.db.getDocument('users', id);
+    return this.db.getDocument('users', id).pipe(map((user: User) => {
+      user.id = id;
+      return user;
+    }));
   }
 
   getFullName(id: string) {
diff --git a/projects/fire-admin/src/lib/services/current-user.service.ts b/projects/fire-admin/src/lib/services/current-user.service.ts
@@ -1,5 +1,5 @@
 import { Injectable } from '@angular/core';
-import { map, takeUntil, take } from 'rxjs/operators';
+import { take, takeUntil } from 'rxjs/operators';
 import { User, UserRole } from '../models/collections/user.model';
 import { UsersService } from './collections/users.service';
 import { Subject } from 'rxjs';
@@ -21,8 +21,7 @@ export class CurrentUserService {
   set(user: firebase.User) {
     this.unsubscribe();
     if (user) {
-      this.users.getWhere('uid', '==', user.uid).pipe(
-        map((users: User[]) => users[0] || null),
+      this.users.get(user.uid).pipe(
         takeUntil(this.userChange)
       ).subscribe((user: User) => {
         if (user) {
diff --git a/projects/fire-admin/src/lib/services/firebase-user.service.ts b/projects/fire-admin/src/lib/services/firebase-user.service.ts
@@ -37,8 +37,7 @@ export class FirebaseUserService {
     return new Promise((resolve, reject) => {
       this.app.auth().createUserWithEmailAndPassword(user.email, user.password).then((userCredential: auth.UserCredential) => {
         // console.log('User ' + userCredential.user.uid + ' created successfully!');
-        user.uid = userCredential.user.uid;
-        this.app.firestore().collection('users').add(user).then(() => {
+        this.app.firestore().collection('users').doc(userCredential.user.uid).set(user).then(() => {
           this.app.firestore().collection('config').doc('registration').set({ enabled: false }, { merge: true }).then(() => {
             this.app.auth().signOut();
             resolve(userCredential.user.uid);

Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,7 @@ export class PostsCategoriesComponent implements OnInit, OnDestroy {`
`56`	`56`	`this.allCategories.subscribe((categories: Category[]) => {`
`57`	`57`	`// console.log(categories);`
`58`	`58`	`// Refresh datatable on data change`
`59`		`- refreshDataTable(this.dataTableElement, this.dataTableTrigger);`
	`59`	`+ refreshDataTable(this.dataTableElement, this.dataTableTrigger, true);`
`60`	`60`	`})`
`61`	`61`	`);`
`62`	`62`	`}`
Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ export class TranslationsComponent implements OnInit, OnDestroy, AfterViewInit {`
`53`	`53`	`this.allTranslations.subscribe((translations: TranslationData[]) => {`
`54`	`54`	`// console.log(translations);`
`55`	`55`	`// Refresh datatable on data change`
`56`		`- refreshDataTable(this.dataTableElement, this.dataTableTrigger);`
	`56`	`+ refreshDataTable(this.dataTableElement, this.dataTableTrigger, true);`
`57`	`57`	`})`
`58`	`58`	`);`
`59`	`59`	`}`