Handle correctly counter update in case of HOTP preview

This commit is contained in:
Bubka 2020-02-06 12:24:18 +01:00
parent 9e0e05ad7c
commit be4e678080
4 changed files with 47 additions and 19 deletions

View File

@ -13,9 +13,10 @@ class OTP
* Generate a TOTP * Generate a TOTP
* *
* @param \App\TwoFAccount $twofaccount * @param \App\TwoFAccount $twofaccount
* @param Boolean $isPreview Prevent updating storage in case of HOTP preview
* @return an array that represent the totp code * @return an array that represent the totp code
*/ */
public static function generate($uri) public static function generate($uri, $isPreview = false)
{ {
$otp = OTP::get($uri); $otp = OTP::get($uri);
@ -40,15 +41,18 @@ public static function generate($uri)
// It's a HOTP // It's a HOTP
$hotp = [ $hotp = [
'otp' => $otp->at($otp->getCounter()), 'otp' => $otp->at($otp->getCounter()),
'counter' => $otp->getCounter(), 'counter' => $otp->getCounter()
]; ];
// now we update the counter for next code // now we update the counter for the next OTP generation
$otp->setParameter( 'counter', $otp->getcounter() + 1 ); $otp->setParameter( 'counter', $otp->getcounter() + 1 );
$hotp['nextUri'] = urldecode($otp->getProvisioningUri());
$twofaccount = \App\TwoFAccount::where('uri', $uri)->first(); if( !$isPreview ) {
$twofaccount->uri = $otp->getProvisioningUri(); $twofaccount = \App\TwoFAccount::where('uri', $uri)->first();
$twofaccount->save(); $twofaccount->uri = $hotp['nextUri'];
$twofaccount->save();
}
return $hotp; return $hotp;
} }

View File

@ -70,15 +70,18 @@ public function show(TwoFAccount $twofaccount)
*/ */
public function generateOTP(Request $request) public function generateOTP(Request $request)
{ {
$isPreview = false;
if( is_int($request->data) ) { if( is_int($request->data) ) {
$twofaccount = TwoFAccount::FindOrFail($request->data); $twofaccount = TwoFAccount::FindOrFail($request->data);
$uri = $twofaccount->uri; $uri = $twofaccount->uri;
} }
else { else {
$uri = $request->data; $uri = $request->data;
$isPreview = true;
} }
return response()->json(OTP::generate($uri), 200); return response()->json(OTP::generate($uri, $isPreview), 200);
} }

View File

@ -23,6 +23,7 @@
internal_service: '', internal_service: '',
internal_account: '', internal_account: '',
internal_uri: '', internal_uri: '',
next_uri: '',
internal_icon: '', internal_icon: '',
type: '', type: '',
otp : '', otp : '',
@ -50,8 +51,8 @@
// 2 possible cases : // 2 possible cases :
// - ID is provided so we fetch the account data from db but without the uri. // - ID is provided so we fetch the account data from db but without the uri.
// This prevent the uri (a sensitive data) to transit via http request unnecessarily. In this // This prevent the uri (a sensitive data) to transit via http request unnecessarily. In this
// case this.type is send by the backend. // case this.type is sent by the backend.
// - an URI has been set in $parent because we need to preview some OTP before storing the account. // - the URI prop has been set via the create form, we need to preview some OTP before storing the account.
// So this.type is set on client side from the provided URI // So this.type is set on client side from the provided URI
this.id = id this.id = id
@ -72,9 +73,6 @@
this.internal_account = this.account this.internal_account = this.account
this.internal_icon = this.icon this.internal_icon = this.icon
this.internal_uri = this.uri this.internal_uri = this.uri
}
if( !this.type ) {
this.type = this.internal_uri.slice(0, 15 ) === "otpauth://totp/" ? 'totp' : 'hotp'; this.type = this.internal_uri.slice(0, 15 ) === "otpauth://totp/" ? 'totp' : 'hotp';
} }
@ -129,8 +127,10 @@
this.axios.post('api/twofaccounts/otp', {data: this.id ? this.id : this.internal_uri }).then(response => { this.axios.post('api/twofaccounts/otp', {data: this.id ? this.id : this.internal_uri }).then(response => {
let spacePosition = Math.ceil(response.data.otp.length / 2); let spacePosition = Math.ceil(response.data.otp.length / 2);
this.otp = response.data.otp.substr(0, spacePosition) + " " + response.data.otp.substr(spacePosition); this.otp = response.data.otp.substr(0, spacePosition) + " " + response.data.otp.substr(spacePosition)
this.counter = response.data.counter; this.counter = response.data.counter
this.next_uri = response.data.nextUri
}) })
}, },
@ -139,12 +139,20 @@
this.id = this.timerID = this.position = this.counter = null this.id = this.timerID = this.position = this.counter = null
this.internal_service = this.internal_account = this.internal_icon = this.internal_uri = '' this.internal_service = this.internal_account = this.internal_icon = this.internal_uri = ''
this.otp = '... ...' this.otp = '... ...'
this.$el.querySelector('[data-is-active]').removeAttribute('data-is-active');
this.$el.querySelector('.dots li:first-child').setAttribute('data-is-active', true); try {
this.$el.querySelector('[data-is-active]').removeAttribute('data-is-active');
this.$el.querySelector('.dots li:first-child').setAttribute('data-is-active', true);
}
catch(e) {
// we do not throw anything
}
}, },
stopLoop: function() { stopLoop: function() {
clearInterval(this.timerID) if( this.type === 'totp' ) {
clearInterval(this.timerID)
}
}, },
clipboardSuccessHandler ({ value, event }) { clipboardSuccessHandler ({ value, event }) {

View File

@ -1,4 +1,5 @@
<template> <template>
<!-- Quick form -->
<form @submit.prevent="createAccount" @keydown="form.onKeydown($event)" v-if="isQuickForm"> <form @submit.prevent="createAccount" @keydown="form.onKeydown($event)" v-if="isQuickForm">
<div class="container preview has-text-centered"> <div class="container preview has-text-centered">
<div class="columns is-mobile"> <div class="columns is-mobile">
@ -29,6 +30,7 @@
</div> </div>
</div> </div>
</form> </form>
<!-- Full form -->
<form-wrapper :title="$t('twofaccounts.forms.new_account')" v-else> <form-wrapper :title="$t('twofaccounts.forms.new_account')" v-else>
<form @submit.prevent="createAccount" @keydown="form.onKeydown($event)"> <form @submit.prevent="createAccount" @keydown="form.onKeydown($event)">
<div class="field"> <div class="field">
@ -164,9 +166,9 @@
} }
// stop OTP generation on modal close // stop TOTP generation on modal close
this.$on('modalClose', function() { this.$on('modalClose', function() {
this.$refs.TwofaccountPreview.clearOTP() this.$refs.TwofaccountPreview.stopLoop()
}); });
}, },
@ -181,6 +183,17 @@
// set current temp icon as account icon // set current temp icon as account icon
this.form.icon = this.tempIcon this.form.icon = this.tempIcon
// The quick form (possibly the preview feature too) 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.TwofaccountShow.next_uri ) {
this.form.uri = this.$refs.TwofaccountShow.next_uri
}
else if( this.$refs.TwofaccountPreview && this.$refs.TwofaccountPreview.next_uri ) {
this.form.uri = this.$refs.TwofaccountPreview.next_uri
}
await this.form.post('/api/twofaccounts') await this.form.post('/api/twofaccounts')
if( this.form.errors.any() === false ) { if( this.form.errors.any() === false ) {