From 02798a05f31efe04820bf16c7179885c5b4ff95c Mon Sep 17 00:00:00 2001
From: Bubka <858858+Bubka@users.noreply.github.com>
Date: Sat, 14 Nov 2020 18:55:10 +0100
Subject: [PATCH] Move token generation from dedicated class to TwoFAccount
 model class

---
 app/Classes/OTP.php                           | 60 -------------------
 .../Controllers/TwoFAccountController.php     | 28 ++++++---
 app/TwoFAccount.php                           | 50 +++++++++++++++-
 resources/js/components/TokenDisplayer.vue    | 10 +---
 resources/js/views/twofaccounts/Create.vue    | 24 +++-----
 5 files changed, 81 insertions(+), 91 deletions(-)
 delete mode 100644 app/Classes/OTP.php

diff --git a/app/Classes/OTP.php b/app/Classes/OTP.php
deleted file mode 100644
index 0c6010cf..00000000
--- a/app/Classes/OTP.php
+++ /dev/null
@@ -1,60 +0,0 @@
-<?php
-
-namespace App\Classes;
-
-use OTPHP\TOTP;
-use OTPHP\Factory;
-use Assert\AssertionFailedException;
-
-class OTP
-{
-
-    /**
-     * Generate a TOTP
-     *
-     * @param  \App\TwoFAccount  $twofaccount
-     * @param  Boolean $isPreview   Prevent updating storage in case of HOTP preview
-     * @return an array that represent the totp code
-     */
-    public static function generate($twofaccount, $isPreview = false)
-    {
-
-        if( $twofaccount->otpType === 'totp' ) {
-
-            $currentPosition = time();
-            $PeriodCount = floor($currentPosition / $twofaccount->totpPeriod); //nombre de période de x s depuis T0 (x=30 par défaut)
-            $currentPeriodStartAt = $PeriodCount * $twofaccount->totpPeriod;
-            $positionInCurrentPeriod = $currentPosition - $currentPeriodStartAt;
-
-            // For memo :
-            // $nextOtpAt = ($PeriodCount+1)*$period
-            // $remainingTime = $nextOtpAt - time()
-
-            return $totp = [
-                'token' => $twofaccount->token(),
-                'position' => $positionInCurrentPeriod
-            ];
-        }
-        else {
-            // It's a HOTP
-            $hotp = [
-                'token' => $twofaccount->token(),
-                'hotpCounter' => $twofaccount->hotpCounter
-            ];
-
-            // now we update the counter for the next OTP generation
-            $twofaccount->increaseHotpCounter();
-
-            $hotp['nextHotpCounter'] = $twofaccount->hotpCounter;
-            $hotp['nextUri'] = $twofaccount->uri;
-
-            if( !$isPreview ) {
-                $twofaccount->save();
-            }
-
-            return $hotp;
-        }
-
-    }
-
-}
diff --git a/app/Http/Controllers/TwoFAccountController.php b/app/Http/Controllers/TwoFAccountController.php
index 011a9c90..502801b9 100644
--- a/app/Http/Controllers/TwoFAccountController.php
+++ b/app/Http/Controllers/TwoFAccountController.php
@@ -114,31 +114,45 @@ public function reorder(Request $request)
      */
     public function generateOTP(Request $request)
     {
-        $isPreview = false;
 
         if( $request->id ) {
-            // The request data is the Id of the account
+
+            // The request data is the Id of an existing account
             $twofaccount = TwoFAccount::FindOrFail($request->id);
         }
         else if( $request->otp['uri'] ) {
+
             // The request data contain an uri
             $twofaccount = new TwoFAccount;
             $twofaccount->populateFromUri($request->otp['uri']);
-
-            $isPreview = true;  // HOTP generated for preview (in the Create form) will not have its counter updated
         }
         else {
+
             // The request data should contain all otp parameter
             $twofaccount = new TwoFAccount;
             $twofaccount->populate($request->otp);
-
-            $isPreview = true;  // HOTP generated for preview (in the Create form) will not have its counter updated
         }
 
-        return response()->json(OTP::generate($twofaccount, $isPreview ? true : false), 200);
+        if( $twofaccount->otpType === 'hotp' ) {
+
+            // returned counter & uri will be updated
+            $twofaccount->increaseHotpCounter();
+
+            // and the db too
+            if( $request->id ) {
+                $twofaccount->save();
+            }
+        }
+
+        if( $request->id ) {
+            return response()->json($twofaccount, 200);
+        }
+
+        return response()->json($twofaccount->makeVisible(['uri', 'secret', 'algorithm']), 200);
     }
 
 
+
     /**
      * Update the specified resource in storage.
      *
diff --git a/app/TwoFAccount.php b/app/TwoFAccount.php
index 1df82e8f..c617b45a 100644
--- a/app/TwoFAccount.php
+++ b/app/TwoFAccount.php
@@ -40,7 +40,7 @@ class TwoFAccount extends Model implements Sortable
      *
      * @var array
      */
-    protected $appends = ['isConsistent', 'otpType', 'secret', 'algorithm', 'digits', 'totpPeriod', 'hotpCounter', 'imageLink'];
+    protected $appends = ['token', 'isConsistent', 'otpType', 'secret', 'algorithm', 'digits', 'totpPeriod', 'totpPosition', 'hotpCounter', 'imageLink'];
 
 
     /**
@@ -348,6 +348,29 @@ public function populate(Array $attrib = [])
     }
 
 
+    /**
+     * Calculate where is now() in the totp current period
+     * @return mixed The position
+     */
+    private function getTotpPosition()
+    {
+        // For memo :
+        // $nextOtpAt = ($PeriodCount+1)*$period
+        // $remainingTime = $nextOtpAt - time()
+        if( $this->otpType === 'totp' ) {
+
+            $currentPosition = time();
+            $PeriodCount = floor($currentPosition / $this->totpPeriod); //nombre de période de x s depuis T0 (x=30 par défaut)
+            $currentPeriodStartAt = $PeriodCount * $this->totpPeriod;
+            $positionInCurrentPeriod = $currentPosition - $currentPeriodStartAt;
+
+            return $positionInCurrentPeriod;
+        }
+
+        return null;
+    }
+
+
     /**
      * Update the uri attribute using the OTP object
      * @return void
@@ -362,12 +385,13 @@ private function refreshUri() : void
      * Generate a token which is valid at the current time (now)
      * @return string The generated token
      */
-    public function token() : string
+    public function generateToken() : string
     {
         return $this->otpType === 'totp' ? $this->otp->now() : $this->otp->at($this->otp->getCounter());
     }
 
 
+
     /**
      * Increment the hotp counter by 1
      * @return string The generated token
@@ -381,6 +405,28 @@ public function increaseHotpCounter() : void
     }
 
 
+    /**
+     * get token attribute
+     * 
+     * @return string The token
+     */
+    public function getTokenAttribute() : string
+    {
+        return $this->generateToken();
+    }
+
+
+    /**
+     * get totpPosition attribute
+     * 
+     * @return int The position
+     */
+    public function getTotpPositionAttribute()
+    {
+        return $this->getTotpPosition();
+    }
+
+
     /**
      * get OTP Type attribute
      *
diff --git a/resources/js/components/TokenDisplayer.vue b/resources/js/components/TokenDisplayer.vue
index 6cec9e76..83640f45 100644
--- a/resources/js/components/TokenDisplayer.vue
+++ b/resources/js/components/TokenDisplayer.vue
@@ -22,8 +22,6 @@
         data() {
             return {
                 id: null,
-                next_uri: '',
-                nextHotpCounter: null,
                 token : '',
                 timerID: null,
                 position: null,
@@ -127,7 +125,7 @@
                     let spacePosition = Math.ceil(response.data.token.length / 2);
                     
                     this.token = response.data.token.substr(0, spacePosition) + " " + response.data.token.substr(spacePosition);
-                    this.position = response.data.position;
+                    this.position = response.data.totpPosition;
 
                     let dots = this.$el.querySelector('.dots');
 
@@ -172,11 +170,9 @@
                     let spacePosition = Math.ceil(response.data.token.length / 2);
                     
                     this.token = response.data.token.substr(0, spacePosition) + " " + response.data.token.substr(spacePosition)
-                    this.internal_hotpCounter = response.data.hotpCounter
-                    this.nextHotpCounter = response.data.nextHotpCounter
-                    this.next_uri = response.data.nextUri
 
-                    this.$emit('update-hotp-counter', { nextHotpCounter: this.nextHotpCounter })
+                    // returned counter & uri are incremented
+                    this.$emit('increment-hotp', { nextHotpCounter: response.data.hotpCounter, nextUri: response.data.uri })
 
                 })
                 .catch(error => {
diff --git a/resources/js/views/twofaccounts/Create.vue b/resources/js/views/twofaccounts/Create.vue
index 04ef9211..bccdeac9 100644
--- a/resources/js/views/twofaccounts/Create.vue
+++ b/resources/js/views/twofaccounts/Create.vue
@@ -9,7 +9,7 @@
                         <font-awesome-icon :icon="['fas', 'image']" size="2x" />
                     </label>
                     <button class="delete delete-icon-button is-medium" v-if="tempIcon" @click.prevent="deleteIcon"></button>
-                    <token-displayer ref="QuickFormTokenDisplayer" v-bind="form.data()">
+                    <token-displayer ref="QuickFormTokenDisplayer" v-bind="form.data()" @increment-hotp="incrementHotp">
                     </token-displayer>
                 </div>
             </div>
@@ -128,7 +128,7 @@
         </form>
         <!-- modal -->
         <modal v-model="ShowTwofaccountInModal">
-            <token-displayer ref="AdvancedFormTokenDisplayer" v-bind="form.data()" @update-hotp-counter="updateHotpCounter">
+            <token-displayer ref="AdvancedFormTokenDisplayer" v-bind="form.data()" @increment-hotp="incrementHotp">
             </token-displayer>
         </modal>
     </form-wrapper>
@@ -205,17 +205,6 @@
                 // set current temp icon as account icon
                 this.form.icon = this.tempIcon
 
-                // The quick form or the preview feature has incremented the HOTP counter so the next_uri property
-                // must be used as the uri to store.
-                // This could desynchronized the HOTP verification server and our local counter if the user never verified the HOTP but this
-                // is acceptable (and HOTP counter can be edited by the way)
-                if( this.isQuickForm && this.$refs.QuickFormTokenDisplayer.next_uri ) {
-                    this.form.uri = this.$refs.QuickFormTokenDisplayer.next_uri
-                }
-                else if( this.$refs.AdvancedFormTokenDisplayer && this.$refs.AdvancedFormTokenDisplayer.next_uri ) {
-                    this.form.uri = this.$refs.AdvancedFormTokenDisplayer.next_uri
-                }
-
                 await this.form.post('/api/twofaccounts')
 
                 if( this.form.errors.any() === false ) {
@@ -253,7 +242,7 @@
                 this.form.fill(data)
                 this.form.otpType = this.form.otpType.toUpperCase()
                 this.form.secretIsBase32Encoded = 1
-                this.form.uri = '' // we don't want an uri now because the user can change any otp parameter in the form
+                this.form.uri = '' // we don't want the uri because the user can change any otp parameter in the form
 
             },
 
@@ -278,8 +267,13 @@
                 }
             },
 
-            updateHotpCounter(payload) {
+            incrementHotp(payload) {
+                // The quick form or the preview feature has incremented the HOTP counter so we get the new value from
+                // the component.
+                // This could desynchronized the HOTP verification server and our local counter if the user never verified the HOTP but this
+                // is acceptable (and HOTP counter can be edited by the way)
                 this.form.hotpCounter = payload.nextHotpCounter
+                this.form.uri = payload.nextUri
             },
             
         },