#234 don't verify users' bugzilla accounts
Merged 4 months ago by kparal. Opened 4 months ago by kparal.

@@ -21,8 +21,8 @@ 

  

  from flask_wtf import FlaskForm

  from wtforms import StringField, SelectField, BooleanField, IntegerField

- from wtforms import TextAreaField, PasswordField, ValidationError

- from wtforms.validators import DataRequired, Email

+ from wtforms import TextAreaField, ValidationError

+ from wtforms.validators import DataRequired

  

  

  def one_proposal(form, field):
@@ -31,14 +31,9 @@ 

  

  

  class BugProposeForm(FlaskForm):

-     bugid = IntegerField(u'Bug ID', [DataRequired()])

-     bz_user = StringField(u'Bugzilla Login', [DataRequired(), Email()])

+     bugid = IntegerField(u'Bug number', [DataRequired()])

+     fas_login = StringField(u'Fedora account', [DataRequired()])

      milestone = SelectField(u'Milestone', [DataRequired()], coerce=int)

      blocker = BooleanField(u'Blocker', [one_proposal])

      freeze_exception = BooleanField(u'Freeze Exception', [one_proposal])

      justification = TextAreaField(u'Justification', [DataRequired()])

- 

- 

- class FasBugzillaForm(FlaskForm):

-     bz_user = StringField(u'Login', [DataRequired(), Email()])

-     bz_pass = PasswordField(u'Password', [DataRequired()])

file modified
+15 -53
@@ -19,22 +19,20 @@ 

  

  """Interface for the main page"""

  

- from flask import Blueprint, render_template, redirect, url_for, abort, g, flash, make_response, request

+ from flask import Blueprint, render_template, redirect, url_for, abort, g, make_response, request

  import datetime

- from sqlalchemy import func, desc, or_

- import bugzilla

+ from sqlalchemy import or_

  from flask_fas_openid import fas_login_required

  import json

  import itertools

  

- from blockerbugs import app, db, __version__

+ from blockerbugs import app, __version__

  from blockerbugs.util import bz_interface, pagure_bot

  from blockerbugs.models.bug import Bug

  from blockerbugs.models.milestone import Milestone

  from blockerbugs.models.update import Update

  from blockerbugs.models.release import Release

- from blockerbugs.models.userinfo import UserInfo

- from blockerbugs.controllers.forms import BugProposeForm, FasBugzillaForm

+ from blockerbugs.controllers.forms import BugProposeForm

  

  main = Blueprint('main', __name__)

  
@@ -414,46 +412,9 @@ 

      return bz_interface.create_bugzilla()

  

  

- @main.route('/fas_bugzilla', methods=['GET', 'POST'])

- @fas_login_required

- def fas_bugzilla():

-     fas_bz_form = FasBugzillaForm()

- 

-     if fas_bz_form.validate_on_submit():

-         try:

-             # try logging into bz if user/pass are valid

-             bz = bugzilla.RHBugzilla4(url=app.config['BUGZILLA_URL'],

-                                       user=fas_bz_form.bz_user.data,

-                                       password=fas_bz_form.bz_pass.data,

-                                       use_creds=False,

-                                       force_xmlrpc=True)

-             bz.disconnect()

- 

-             #create new user_info if user with given FAS-login does not exist

-             #if user_with given FAS-login exists just update user_info.bz_user

-             user_info = UserInfo.query.filter_by(fas_login=g.fas_user.username).first()

-             if user_info:

-                 user_info.bz_user = fas_bz_form.bz_user.data

-             else:

-                 user_info = UserInfo(g.fas_user.username, fas_bz_form.bz_user.data)

-             db.session.add(user_info)

-             db.session.commit()

-             return redirect(url_for('.propose_bug'))

-         except bugzilla.BugzillaError as e:

-             app.logger.info(e)

-             flash(e)

- 

-     return render_template('fas_bugzilla.html', title='Link your FAS and Red Hat Bugzilla Accounts',

-                             fas_bz_form=fas_bz_form)

- 

- 

  @main.route('/propose_bug', methods=['GET', 'POST'])

  @fas_login_required

  def propose_bug():

-     user_info = UserInfo.query.filter_by(fas_login=g.fas_user.username).first()

-     if not user_info or not user_info.bz_user:

-         return redirect(url_for('.fas_bugzilla'))

- 

      current_milestone = get_current_milestone()

      bugform = BugProposeForm()

      bugform.milestone.choices = [(m.id, m.name) for m in Milestone.query.filter_by(active=True).order_by(Milestone.id).all()]
@@ -461,11 +422,11 @@ 

  

      # we have to process the form after setting the milestone default so that it is properly set

      # by passing in the request.form, anything submitted as part of a POST will be inserted as form data

-     bugform.process(request.form, bz_user = user_info.bz_user)

+     bugform.process(request.form, fas_login=g.fas_user.username)

  

      if bugform.validate_on_submit():

          app.logger.debug('bugid: %i' % bugform.bugid.data)

-         app.logger.debug('bz_user: %s' % user_info.bz_user)

+         app.logger.debug('fas_login: %s' % bugform.fas_login.data)

          app.logger.debug('milestone: %i' % bugform.milestone.data)

          app.logger.debug('blocker: %s' % bugform.blocker.data)

          app.logger.debug('freeze_exception: %s' % bugform.freeze_exception.data)
@@ -482,34 +443,35 @@ 

          # The form does basic datatype validation but we need to check for valid

          # data

          bz = get_bugzilla()

-         proposer = bz_interface.BlockerProposal(bz, bugid, trackers, bugform.blocker.data,

-                                                 bugform.freeze_exception.data, user_info.bz_user)

+         proposal = bz_interface.BlockerProposal(bz, bugid, trackers, bugform.blocker.data,

+                                                 bugform.freeze_exception.data)

  

          # check bug to make sure it exists and that it isn't closed

          try:

-             proposer.check_proposed_bug()

+             proposal.check_proposed_bug()

          except bz_interface.BZInterfaceError as e:

              bugform.bugid.errors = [e.msg]

  

          # check to make sure that any proposals aren't being re-done

          if bugform.blocker.data:

              app.logger.debug('checking for valid blocker proposal')

-             if not proposer.check_blocker_proposal():

+             if not proposal.check_blocker_proposal():

                  bugform.blocker.errors = ['Bug %i is already proposed as a blocker' % bugid]

          if bugform.freeze_exception.data:

              app.logger.debug('checking for valid freeze exception proposal')

-             if not proposer.check_fe_proposal():

+             if not proposal.check_fe_proposal():

                  bugform.freeze_exception.errors = ['Bug %i is already proposed as a freeze exception' % bugid]

  

          if len(bugform.errors) == 0:

              user = 'Fedora user %s' % g.fas_user.username

              app.logger.info('%s proposing %i as %s for %s' % (

-                 user, bugid, proposer.get_tracker_type(), selected_milestone.name))

+                 user, bugid, proposal.get_tracker_type(), selected_milestone.name))

              try:

-                 proposer.propose_bugs(user, selected_milestone.name, bugform.justification.data)

+                 proposal.propose_bugs(user, selected_milestone.name, bugform.justification.data)

                  return render_template('thanks.html', bugid=bugid,

                                         isblocker=bugform.blocker.data,

-                                        isfe=bugform.freeze_exception.data)

+                                        isfe=bugform.freeze_exception.data,

+                                        bz_url=app.config['BUGZILLA_URL'])

              except bz_interface.BZInterfaceError as e:

                  bugform.bugid.errors = [e.msg]

                  app.logger.info('bug proposal form errors: %s' % e.msg)

@@ -25,6 +25,9 @@ 

  

  from blockerbugs import db, BaseModel

  

+ # FIXME: This is not needed anymore after https://pagure.io/fedora-qa/blockerbugs/issue/230

+ # It also supports storing and checking API keys, but we don't use it anywhere.

+ # Get rid of this table?

  

  class UserInfo(BaseModel):

      id = db.Column(db.Integer, primary_key=True)

@@ -1,80 +0,0 @@ 

- {% extends "layout.html" %}

- 

- {% block jsheader %}

- {% endblock %}

- 

- {% block info %}

- {% endblock %}

- 

- {% block navigation %}

- {% endblock %}

- 

- {% block body %}

- <div class="row">

-     <div class="col-auto">

-         <h2>Link your FAS and Red Hat Bugzilla Accounts</h2>

-     </div>

- </div>

- <div class="row justify-content-center">

-     <div class="col-auto" style="padding: 10px;"id="login">

-         <form method="post" action="{{ url_for('main.fas_bugzilla') }}">

-             {{ fas_bz_form.csrf_token }}

-             <fieldset id="bugzilla-credentials">

-             <legend>Bugzilla Credentials</legend>

-             <table>

-                 <tr>

-                     <th scope="col"> {{ fas_bz_form.bz_user.label }}</th>

-                     <td> {{ fas_bz_form.bz_user(size=20) }}</td>

-                 </tr>

-                 {% if fas_bz_form.bz_user.errors %}

-                 <tr>

-                     <td></td>

-                     <td>

-                         {% if fas_bz_form.bz_user.errors %}

-                         <div class="alert-danger alert" role="alert">

-                             <ul>

-                                 {% for error in fas_bz_form.bz_user.errors %}

-                                 <li>{{ error }}</li>

-                                 {% endfor %}

-                             </ul>

-                         </div>

-                         {% endif %}

-                     </td>

-                 </tr>

-                 {% endif %}

-                 <tr>

-                     <th scope="col">{{ fas_bz_form.bz_pass.label }}</th>

-                     <td>{{ fas_bz_form.bz_pass(size=20) }}</td>

-                 </tr>

-                 {% if fas_bz_form.bz_pass.errors %}

-                 <tr>

-                     <td></td>

-                     <td>

-                         {% if fas_bz_form.bz_pass.errors %}

-                         <div class="alert-danger alert" role="alert">

-                             <ul>

-                                 {% for error in fas_bz_form.bz_pass.errors %}

-                                 <li>{{ error }}</li>

-                                 {% endfor %}

-                             </ul>

-                         </div>

-                         {% endif %}

-                     </td>

-                 </tr>

-                 {% endif %}

-                 <tr>

-                     <td>

-                         <input type="submit" value="Submit"/>

-                     </td>

-                 </tr>

-             </table>

-             </fieldset>

-         </form>

-     </div>

- </div>

- <div class="row">

-     <div class="col-md-12">

-         Your Red Hat bugzilla password is used only to verify that you control the account which will be linked to your FAS account. Your password will <b>NOT</b> be stored anywhere in this application.

-     </div>

- </div>

- {% endblock %}

@@ -27,6 +27,17 @@ 

  

  <form method="POST" action="{{ url_for('main.propose_bug') }}">

      {{ bugform.csrf_token }}

+ 

+     <fieldset>

+         <legend>Your identification</legend>

+         <div class="row">

+             <div class="form-group col-md-3">

+                 <label for="fas_login">{{ bugform.fas_login.label.text }}</label>

+                 {{ bugform.fas_login(readonly=True, class_="form-control") }} {{ form_error(bugform.fas_login) }}

+             </div>

+         </div>

+     </fieldset>

+ 

      <fieldset>

          <legend>Blocker Information</legend>

          <div class="row">
@@ -34,10 +45,6 @@ 

                  <label for="bugid">{{ bugform.bugid.label.text }}</label>

                  {{ bugform.bugid(class_="form-control") }} {{ form_error(bugform.bugid) }}

              </div>

-             <div class="form-group col-md-3">

-                 <label for="bz_user">{{ bugform.bz_user.label.text }}</label>

-                 {{ bugform.bz_user(readonly=True, class_="form-control") }} {{ form_error(bugform.bz_user) }}

-             </div>

              <div class="form-group col-md-2">

                  <label for="milestone">{{ bugform.milestone.label.text }}</label>

                  <br /> {{ bugform.milestone(class_="form-control") }} {{ form_error(bugform.milestone) }}

@@ -1,23 +1,19 @@ 

  {% extends "base_nav.html" %}

  

  {% macro blockertype(blocker, fe) -%}

- {%- if blocker -%}

- blocker

- {%- endif %}

- {%- if fe %}

- {%- if blocker %}

-  and freeze exception

- {%- else -%}

- freeze exception

- {%- endif %}

- {%- endif %}

+     {%- if blocker and fe -%}

+     blocker and a freeze exception

+     {%- elif blocker -%}

+     blocker

+     {%- elif fe -%}

+     freeze exception

+     {%- endif %}

  {%- endmacro %}

  

  {% block jsheader %}

  {% endblock %}

  

  {% block info %}

- 

  {% endblock %}

  

  {% block body %}
@@ -25,11 +21,25 @@ 

  <div class="row">

      <div class="col-md-12" id="index">

          <h2>Proposal Complete</h2>

+         <p></p>

          <p>

-         Thank you for proposing {{ bugid }} as a {{  blockertype(isblocker, isfe) }}.

-         Bug {{ bugid }} has been updated but the changes may not show up on

-         the bug list until the next sync operation (every 30 minutes).

+   <b>Thank you for proposing

+   <a href="{{ bz_url }}/show_bug.cgi?id={{ bugid }}" target="_blank" rel="noopener noreferrer">bug {{ bugid }}</a>

+   as a {{ blockertype(isblocker, isfe) }}.</b> This bug has been updated in Bugzilla, but the

+   changes may not show up here in the BlockerBugs application until the next sync operation (every

+   30 minutes). Once the bug is visible here, there will be also a link to a proposal review

+   discussion, where you can participate.

          </p>

+         <div class="alert-info alert">

+   <b>IMPORTANT:</b> Please subscribe to that bug (if you're not already) to receive updates on the

+   problem and also on the proposal decision.<br/>

+   <button id="howto-toggle" type="button" class="btn btn-link" data-toggle="collapse" data-target="#howto">How to do that?</button>

+             <div id="howto" class="collapse">

+   Open <a href="{{ bz_url }}/show_bug.cgi?id={{ bugid }}" target="_blank" rel="noopener noreferrer">bug {{ bugid }}</a>,

+   log in, and make sure you're listed in the <i>CC List</i> field. If you're not, click <i>Add me to

+   CC list</i>, click <i>This is a minor update (do not send email)</i> and click <i>Save Changes</i>.

+             </div>

+         </div>

      </div>

  </div>

  

@@ -198,13 +198,12 @@ 

  

  

  class BlockerProposal():

-     def __init__(self, bz, bugid, trackers, is_blocker=False, is_fe=False, bz_user=''):

+     def __init__(self, bz, bugid, trackers, is_blocker=False, is_fe=False):

          self.bz = bz

          self.bugid = bugid

          self.trackers = trackers

          self.is_blocker = is_blocker

          self.is_fe = is_fe

-         self.bz_user = bz_user

          self.bugid_ok = False

          self.proposal_ok = False

          self.bugdata = None
@@ -232,9 +231,9 @@ 

          if self.is_prioritized:

              return 'PrioritizedBug'

  

-     def propose_bugs(self, bz_user, milestone_name, justification):

+     def propose_bugs(self, proposer, milestone_name, justification, cc_add=None):

          comment = ['Proposed as a', self.get_tracker_type(), 'for', milestone_name, 'by',

-                    bz_user, 'using the blocker tracking app because:\n\n',

+                    proposer, 'using the blocker tracking app because:\n\n',

                     justification]

          tracker_bugs = []

          if self.is_blocker:
@@ -243,7 +242,7 @@ 

              tracker_bugs.append(self.trackers['fe'])

          self.log.info('comment: %s' % comment)

          try:

-             self._do_proposal(tracker_bugs, self.bugid, ' '.join(comment), self.bz_user)

+             self._do_proposal(tracker_bugs, self.bugid, ' '.join(comment), cc_add)

          except Fault as e:

              if e.faultCode == 51:

                  # bugzilla account does not exist, this should happen very rarely, if ever
@@ -253,9 +252,9 @@ 

              else:

                  raise BZInterfaceError(e.faultString)

  

-     def _do_proposal(self, tracker, proposed_bugid, comment, bz_user):

+     def _do_proposal(self, tracker, proposed_bugid, comment, cc_add):

          bug_update = self.bz.build_update(blocks_add=tracker,

-                                           cc_add=[bz_user],

+                                           cc_add=cc_add or [],

                                            comment=comment)

          self.bz.update_bugs(proposed_bugid, bug_update)

  

file modified
+4 -4
@@ -53,9 +53,9 @@ 

          stubbz.build_update.return_value = bz_success

  

          test_bz = BlockerProposal(stubbz, self.ref_proposed, self.ref_trackers,

-                                   is_blocker=True, bz_user=self.ref_cc)

+                                   is_blocker=True)

          test_bz.propose_bugs(self.ref_user, self.ref_milestone,

-                              self.ref_justification)

+                              self.ref_justification, cc_add=self.ref_cc)

  

          test_cc = stubbz.build_update.mock_calls[0][2]['cc_add']

  
@@ -101,11 +101,11 @@ 

          stubbz.update_bugs = mock.Mock(side_effect=Fault(50, 'not 51 exception'))

  

          test_bz = BlockerProposal(stubbz, self.ref_proposed, self.ref_trackers,

-                                     is_blocker = True, bz_user = self.ref_cc)

+                                   is_blocker=True)

  

          with pytest.raises(BZInterfaceError) as excinfo:

              test_bz.propose_bugs(self.ref_user, self.ref_milestone,

-                                     self.ref_justification)

+                                  self.ref_justification, cc_add=self.ref_cc)

  

          assert excinfo.value.msg == 'not 51 exception'

  

After Bugzilla auth changes [1] we can no longer test users' logins and passwords
for validity. Because we already have the user logged in through FAS, which
combats spam, the easiest solution is to simply drop the bugzilla login
verification process. The downside is that the user needs to manually CC himself
to that bug (if not CCed already).

This patch drops the verification process and adjusts the form templates so that
the user is advised to CC himself to the bug.

[1] https://listman.redhat.com/archives/bugzilla-announce-list/2022-February/msg00000.html

Related: https://pagure.io/fedora-qa/blockerbugs/issue/230

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

rebased onto c7b4486

4 months ago

Build succeeded.

After a quick glance, seems like worth the try!

Commit 665875e fixes this pull-request

Pull-Request has been merged by kparal

4 months ago