Skip to content

Commit

Permalink
refactor: remove redundant Advanced Settings from PersonalTotpSetting…
Browse files Browse the repository at this point in the history 
…s.vue

Signed-off-by: ernolf <raphael.gradenwitz@googlemail.com>
  • Loading branch information
@ernolf
ernolf committed Aug 9, 2024
1 parent 4d23809 commit ac97afb
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 255 deletions.
276 changes: 22 additions & 254 deletions src/components/PersonalTotpSettings.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,94 +27,15 @@
<span> {{ t('twofactor_totp', 'Enable TOTP') }} </span>
</template>
<div v-else>
<!-- Checkbox and Advanced Settings Button in the same row -->
<div class="row">
<div class="checkbox-container">
<input id="totp-enabled"
v-model="enabled"
type="checkbox"
class="checkbox"
:disabled="loading"
@change="toggleEnabled">
<label for="totp-enabled">{{
t('twofactor_totp', 'Enable TOTP')
}}</label>
</div>

<!-- Advanced Settings Button -->
<div v-if="enabled" class="advanced-settings-container">
<button class="advanced-settings-btn" @click="toggleAdvancedSettings">
{{ showAdvanced ? t('twofactor_totp', 'Hide Advanced Settings') : t('twofactor_totp', 'Advanced Settings') }}
</button>
</div>
</div>

<!-- Advanced Settings Section -->
<div v-if="showAdvanced && enabled" class="advanced-settings">
<p class="warning-message">
{{ t('twofactor_totp', 'Warning: Not all TOTP apps support changing these settings or may not support their full range.') }}
</p>
<p class="instruction-message">
{{ t('twofactor_totp', 'Changes made here must be reflected exactly in your TOTP app. Ensure that both settings match. Keep any setting in this form that cannot be edited or updated in your TOTP app unchanged here.') }}
</p>

<!-- Algorithm Select -->
<div class="form-group">
<label for="algorithm">{{
t('twofactor_totp', 'Algorithm')
}}</label>
<select id="algorithm"
v-model.number="algorithm"
:disabled="loading || !enabled"
@mouseleave="onMouseLeave">
<option :value="1">
SHA1
</option>
<option :value="2">
SHA256
</option>
<option :value="3">
SHA512
</option>
</select>
</div>

<!-- Digits Select -->
<div class="form-group">
<label for="digits">{{
t('twofactor_totp', 'Digits (OTP token length)')
}}</label>
<select id="digits"
v-model.number="digits"
:disabled="loading || !enabled"
@mouseleave="onMouseLeave">
<option v-for="length in digitsOptions" :key="length" :value="length">
{{ length }}
</option>
</select>
</div>

<!-- Period Select -->
<div class="form-group">
<label for="period">{{
t('twofactor_totp', 'Period (OTP validity in seconds)')
}}</label>
<select id="period"
v-model.number="period"
:disabled="loading || !enabled"
@mouseleave="onMouseLeave">
<option v-for="seconds in periodOptions" :key="seconds" :value="seconds">
{{ seconds }}
</option>
</select>
</div>

<!-- Save Button -->
<button :disabled="!settingsChanged || loading"
@click="updateSettings">
{{ t('twofactor_totp', 'Save changes') }}
</button>
</div>
<input id="totp-enabled"
v-model="enabled"
type="checkbox"
class="checkbox"
:disabled="loading"
@change="toggleEnabled">
<label for="totp-enabled">{{
t('twofactor_totp', 'Enable TOTP')
}}</label>
</div>

<SetupConfirmation v-if="secret"
Expand All @@ -123,7 +44,7 @@
:loading="loadingConfirmation"
:confirmation.sync="confirmation"
@confirm="enableTOTP"
@updateQR="updateQR" />
@updateQr="updateQr" />
</div>
</template>

Expand All @@ -148,41 +69,13 @@ export default {
secret: undefined,
qrUrl: '',
confirmation: '',
algorithm: null, // initially null to ensure it gets set when advanced settings are loaded
digits: null, // initially null to ensure it gets set when advanced settings are loaded
period: null, // initially null to ensure it gets set when advanced settings are loaded
digitsOptions: [4, 5, 6, 7, 8, 9, 10], // options for digits
periodOptions: [15, 20, 25, 30, 35, 40, 45, 50, 55, 60], // options for period
settingsChanged: false, // track if settings have changed
showAdvanced: false, // whether to show advanced settings
initialSettings: {},
}
},
computed: {
state() {
return this.$store.state.totpState
},
},
watch: {
algorithm() {
this.checkIfSettingsChanged()
},
digits() {
this.checkIfSettingsChanged()
},
period() {
this.checkIfSettingsChanged()
},
enabled(newValue) {
if (!newValue) {
this.hideAdvancedSettings()
}
},
},
mounted() {
this.storeInitialSettings()
},
methods: {
toggleEnabled() {
if (this.loading) {
Expand All @@ -192,9 +85,9 @@ export default {
}
if (this.enabled) {
this.createTOTP()
return this.createTOTP()
} else {
this.disableTOTP()
return this.disableTOTP()
}
},
Expand All @@ -211,18 +104,20 @@ export default {
this.qrUrl = qrUrl
// If the state could be changed, keep showing the loading
// spinner until the user has finished the registration
this.loading = this.$store.state.totpState === STATE.STATE_CREATED
this.loading
= this.$store.state.totpState === STATE.STATE_CREATED
})
.catch((e) => {
OC.Notification.showTemporary(
this.t('twofactor_totp', 'Could not enable TOTP'),
t('twofactor_totp', 'Could not enable TOTP'),
)
Logger.error('Could not enable TOTP', e)
// Restore on error
this.loading = false
this.enabled = false
})
.catch((e) => Logger.error(e))
},
enableTOTP() {
Expand All @@ -243,7 +138,10 @@ export default {
this.secret = undefined
} else {
OC.Notification.showTemporary(
this.t('twofactor_totp', 'Could not verify your key. Please try again'),
t(
'twofactor_totp',
'Could not verify your key. Please try again',
),
)
}
Expand All @@ -261,90 +159,12 @@ export default {
return confirmPassword()
.then(() => this.$store.dispatch('disable'))
.then(() => {
this.enabled = false
this.hideAdvancedSettings()
})
.then(() => (this.enabled = false))
.catch(Logger.error.bind(this))
.then(() => (this.loading = false))
},
fetchSettings() {
this.$store.dispatch('getSettings')
.then(() => {
this.algorithm = this.$store.state.algorithm
this.digits = this.$store.state.digits
this.period = this.$store.state.period
this.storeInitialSettings()
})
.catch((e) => {
Logger.error('Could not fetch settings', e)
})
},
updateSettings() {
if (this.enabled) {
// Show loading spinner
this.loading = true
Logger.debug('Starting settings update')
// Confirm password before updating settings
return confirmPassword()
.then(() => this.$store.dispatch('updateSettings', {
algorithm: this.algorithm,
digits: this.digits,
period: this.period,
}))
.then(() => {
this.loading = false
this.settingsChanged = false
OC.Notification.showTemporary(
this.t('twofactor_totp', 'Settings updated successfully'),
)
})
.catch((e) => {
OC.Notification.showTemporary(
this.t('twofactor_totp', 'Could not update settings'),
)
Logger.error('Could not update settings', e)
this.loading = false
})
}
},
onMouseLeave() {
this.checkIfSettingsChanged()
event.target.blur()
},
checkIfSettingsChanged() {
this.settingsChanged
= this.algorithm !== this.initialSettings.algorithm
|| this.digits !== this.initialSettings.digits
|| this.period !== this.initialSettings.period
},
toggleAdvancedSettings() {
this.showAdvanced = !this.showAdvanced
if (this.showAdvanced && !this.initialSettings.algorithm) {
this.fetchSettings()
}
},
hideAdvancedSettings() {
this.showAdvanced = false
},
storeInitialSettings() {
this.initialSettings = {
algorithm: this.algorithm,
digits: this.digits,
period: this.period,
}
},
updateQR({ secret, qrUrl }) {
updateQr({ secret, qrUrl }) {
this.secret = secret
this.qrUrl = qrUrl
},
Expand All @@ -359,56 +179,4 @@ export default {
margin-left: -2px;
margin-right: 4px;
}
.row {
display: flex;
align-items: center;
justify-content: flex-start;
}
.checkbox-container {
display: flex;
align-items: center;
}
.advanced-settings-container {
margin-left: 80px;
}
.advanced-settings-btn {
padding: 5px 10px;
}
.advanced-settings {
margin-top: 20px;
}
.warning-message {
color: var(--color-warning);
font-weight: bold;
}
.instruction-message {
color: var(--color-info);
margin-bottom: 10px;
}
.form-group {
display: flex;
align-items: center;
margin: 5px 0;
label {
margin-right: 10px;
white-space: nowrap;
}
input, select {
width: auto;
}
.custom-secret-input {
width: 100%;
}
}
</style>
2 changes: 1 addition & 1 deletion src/components/SetupConfirmation.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ export default {
this.localSecret = secret
this.localQrUrl = qrUrl
this.settingsChanged = false
this.$emit('updateQR', { secret, qrUrl })
this.$emit('updateQr', { secret, qrUrl })

Check failure on line 265 in src/components/SetupConfirmation.vue

View workflow job for this annotation

GitHub Actions / NPM lint 

Custom event name 'updateQr' must be kebab-case
// Set focus to the confirmation input field when QRCode is recreated
this.$nextTick(() => {
this.$refs.confirmationInput.focus()
Expand Down

0 comments on commit ac97afb

Please sign in to comment.