Merge branch 'staging' into stable

docs/release_notes.rst

@@ -17,6 +17,7 @@ Features
 * The GPG key ID fields now refuse key IDs shorter than 16 characters, and
   allow up to 40 characters (the full fingerprint) (:issue:`556`).
 * Paginate the group members list (:issue:`580`).
+* Handle separately OTP from password in UI (:issue:`572`).
 
 Bug Fixes
 ^^^^^^^^^

noggin/controller/authentication.py

@@ -21,6 +21,9 @@ def handle_login_form(form):
     username = form.username.data.lower()
     password = form.password.data
 
+    if form.otp.data:
+        password += form.otp.data
+
     try:
         # This call will set the cookie itself, we don't have to.
         ipa = maybe_ipa_login(current_app, session, username, password)

noggin/controller/user.py

@@ -198,20 +198,23 @@ def user_settings_otp(ipa, username):
     confirmotpform = UserSettingsConfirmOTPForm(prefix="confirm-")
     user = User(user_or_404(ipa, username))
     secret = None
+
     if addotpform.validate_on_submit():
+        description = addotpform.description.data
+        password = addotpform.password.data
+        if addotpform.otp.data:
+            password += addotpform.otp.data
+
         try:
-            maybe_ipa_login(current_app, session, username, addotpform.password.data)
+            maybe_ipa_login(current_app, session, username, password)
         except python_freeipa.exceptions.InvalidSessionPassword:
             addotpform.password.errors.append(_("Incorrect password"))
         else:
             secret = b32encode(os.urandom(OTP_KEY_LENGTH)).decode('ascii')
             # Prefill the form for the next step
             confirmotpform.process(
                 MultiDict(
-                    {
-                        "confirm-secret": secret,
-                        "confirm-description": addotpform.description.data,
-                    }
+                    {"confirm-secret": secret, "confirm-description": description}
                 )
             )
     if confirmotpform.validate_on_submit():

noggin/form/edit_user.py

@@ -112,6 +112,12 @@ class UserSettingsAddOTPForm(ModestForm):
         description=_("please reauthenticate so we know it is you"),
     )
 
+    otp = PasswordField(
+        _('One-Time Password'),
+        validators=[Optional()],
+        description=_("Enter your One-Time Password"),
+    )
+
     submit = SubmitButtonField(_("Generate OTP Token"))
 
 

noggin/form/login_user.py

@@ -1,6 +1,6 @@
 from flask_babel import lazy_gettext as _
 from wtforms import PasswordField, StringField
-from wtforms.validators import DataRequired
+from wtforms.validators import DataRequired, Optional
 
 from .base import ModestForm, SubmitButtonField
 
@@ -16,4 +16,6 @@ class LoginUserForm(ModestForm):
         validators=[DataRequired(message=_('You must provide a password'))],
     )
 
+    otp = StringField(_('One-Time Password'), validators=[Optional()])
+
     submit = SubmitButtonField(_('Log In'))

noggin/form/password_reset.py

@@ -1,6 +1,6 @@
 from flask_babel import lazy_gettext as _
 from wtforms import PasswordField, StringField
-from wtforms.validators import DataRequired, EqualTo
+from wtforms.validators import DataRequired, EqualTo, Optional
 
 from .base import BaseForm
 from .validators import PasswordLength
@@ -19,17 +19,14 @@ class NewPasswordForm(BaseForm):
 
     password_confirm = PasswordField(_('Confirm New Password'))
 
-    otp = StringField(
-        _('OTP Token'), description=_("Enter your OTP token if you have enrolled one")
-    )
+    otp = StringField(_('One-Time Password'), validators=[Optional()])
 
 
 class PasswordResetForm(NewPasswordForm):
 
     current_password = PasswordField(
         _('Current Password'),
         validators=[DataRequired(message=_('Current password must not be empty'))],
-        description=_("Just the password, don't add the OTP token if you have one"),
     )
 
 

noggin/templates/_login_form.html

@@ -5,8 +5,11 @@
     <div class="form-group">
       {{ macros.with_errors(login_form.username, class="validate", placeholder="Username", tabindex="1", label=False) }}
     </div>
+    <div class="form-group">
+      {{ macros.with_errors(login_form.password, class="validate", placeholder="Password", tabindex="2", label=False) }}
+    </div>
     <div class="form-group mb-0">
-      {{ macros.with_errors(login_form.password, class="validate", placeholder="Password or Password + One-Time-Password", tabindex="2", label=False) }}
+      {{ macros.with_errors(login_form.otp, class="validate", placeholder="One-Time Password (if you have one)", tabindex="3", label=False) }}
     </div>
     <div class="form-group mb-0 text-right">
     {% if lost_otp_token is not defined %}
@@ -20,6 +23,6 @@
   </div>
   <div class="card-footer d-flex justify-content-between">
     <div>{{ macros.non_field_errors(login_form) }}</div>
-    {{ login_form.submit(color="primary", tabindex="3") }}
+    {{ login_form.submit(color="primary", tabindex="4") }}
   </div>
 </form>

noggin/templates/password-reset.html

@@ -17,7 +17,9 @@ <h5 id="pageheading">{{ _('Expired Password Reset for %(username)s', username=us
                 <div class="form-group">{{ macros.with_errors(password_reset_form.current_password, tabindex="2")}}</div>
                 <div class="form-group">{{ macros.with_errors(password_reset_form.password, tabindex="3")}}</div>
                 <div class="form-group">{{ macros.with_errors(password_reset_form.password_confirm, tabindex="4")}}</div>
-                <div class="form-group">{{ macros.with_errors(password_reset_form.otp, tabindex="5")}}</div>
+                {% if tokens %}
+                  <div class="form-group">{{ macros.with_errors(password_reset_form.otp, tabindex="5")}}</div>
+                {% endif %}
               </div>
               <div class="card-footer d-flex justify-content-between">
                 <div>{{ macros.non_field_errors(password_reset_form) }}</div>

noggin/templates/user-settings-otp.html

@@ -61,13 +61,12 @@ <h5 class="modal-title">{{ _("Add OTP Token") }}</h5>
         <form action="{{ url_for('.user_settings_otp', username=current_user.username) }}" method="post" class="px-4 py-3" novalidate>
           <input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>
           <div class="form-group">{{ macros.with_errors(addotpform.description, label=False, tabindex="2", placeholder=_("Token name"))}}</div>
-          {% if tokens %}
-          <div class="form-group">{{ macros.with_errors(addotpform.password, label=False, tabindex="3", placeholder=_("Password + One-Time-Password"))}}</div>
-          {% else %}
           <div class="form-group">{{ macros.with_errors(addotpform.password, label=False, tabindex="3", placeholder=_("Password"))}}</div>
+          {% if tokens %}
+          <div class="form-group">{{ macros.with_errors(addotpform.otp, label=False, tabindex="4", placeholder=_("One-Time Password"))}}</div>
           {% endif %}
           <div>{{ macros.non_field_errors(addotpform) }}</div>
-          {{ addotpform.submit(color="primary", class="btn-block", tabindex="4") }}
+          {{ addotpform.submit(color="primary", class="btn-block", tabindex="5") }}
         </form>
       </div>
     </div>

noggin/tests/unit/conftest.py

@@ -223,7 +223,18 @@ def dummy_user_with_gpg_key(client, dummy_user):
 
 
 @pytest.fixture
-def dummy_user_with_otp(client, logged_in_dummy_user):
+def dummy_user_with_otp(client, dummy_user):
+    result = ipa_admin.otptoken_add(
+        o_ipatokenowner="dummy", o_description="dummy's token"
+    )
+    token = OTPToken(result["result"])
+    yield token
+    # Deletion needs to be done as admin to remove the last token
+    ipa_admin.otptoken_del(token.uniqueid)
+
+
+@pytest.fixture
+def logged_in_dummy_user_with_otp(client, logged_in_dummy_user):
     ipa = logged_in_dummy_user
     result = ipa.otptoken_add(o_ipatokenowner="dummy", o_description="dummy's token",)
     token = OTPToken(result['result'])