Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hide otp #7926

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

hide otp #7926

wants to merge 1 commit into from

Conversation

cpalv
Copy link

@cpalv cpalv commented Oct 1, 2024

#7911 hide the OTP seed after it has been generated. Tested in a vm. Not a big deal if this gets swept up in migration for #7904. I'd be happy to help chip away on that

@fichtner
Copy link
Member

fichtner commented Oct 1, 2024

Wouldn't it be easier to move the OTP seed to the QR code unhide button by clustering the sections together?

@@ -434,6 +434,19 @@ function get_user_privdesc(& $user)
$('#otp_qrcode').show();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cluster the sections together as in bundle the "unhide" action into this method??

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, so you don't have to manipulate the existing input / add the textarea

@cpalv
Copy link
Author

cpalv commented Oct 4, 2024

I tried to move the OTP seed into the otp_qrcode div

<tr>
<td><a id="help_for_otp_code" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?= gettext('OTP QR code') ?></td>
<td>
<label class="btn btn-primary" id="otp_unhide"><?= gettext('Click to unhide') ?></label>
<div style="display:none;" id="otp_qrcode"></div>
<script>
$('#otp_qrcode').qrcode('<?= $otp_url ?>');
</script>
<div class="hidden" data-for="help_for_otp_code">
<?= gettext('Scan this QR code for easy setup with external apps.') ?>
</div>
</td>
</tr>

like so

<tr>
    <td><a id="help_for_otp_code" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?= gettext('OTP QR code') ?></td>
     <td>
        <label class="btn btn-primary" id="otp_unhide"><?= gettext('Click to unhide') ?></label>
        <div style="display:none;" id="otp_qrcode">
           <td rowspan="3"><a id="help_for_otp_seed" href="#" class="showhelp" style="display;none;"><i class="fa fa-info-circle"></i></a> <?= gettext('OTP seed') ?></td>
            <td>
               <input id="otp_seed" name="otp_seed" value="<?=$pconfig['otp_seed'];?>"/>
               <small ><?= gettext('Generate new secret (160 bit)') ?></small>
                  <div class="hidden" data-for="help_for_otp_seed"><?=gettext("OTP (base32) seed to use when a one time password authenticator is used");?><br/></div>
            </td>
         </div>
         <script>
                $('#otp_qrcode').qrcode('<?= $otp_url ?>');
          </script>
          <div class="hidden" data-for="help_for_otp_code">
                    <?= gettext('Scan this QR code for easy setup with external apps.') ?>
           </div>
   </td>
</tr>

but i guess when $('#otp_qrcode').qrcode() creates the QR code image and inserts a canvas into the same otp_qrcode div all of the OTP seed nodes get moved outside the div.

There is another problem. When a new user is created the OTP field is gone entirely from the form and can no longer be generated. The only way I see around this is to allow generating an OTP seed at user creation, but then the field would only appear IFF the user has an OTP seed at creation.

@fichtner
Copy link
Member

fichtner commented Oct 4, 2024

I’ll take a look on Monday. Thanks for looking into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants