client: Group SASL EXTERNAL under SASL, test cap
authorShane Synan <digitalcircuit36939@gmail.com>
Sat, 25 Jul 2020 01:49:21 +0000 (21:49 -0400)
committerManuel Nickschas <sputnick@quassel-irc.org>
Tue, 15 Dec 2020 13:47:50 +0000 (14:47 +0100)
Move the "SASL EXTERNAL" notice into the "Use SASL Authentication"
group box.  This makes it more clear that "SASL EXTERNAL" is only used
when SASL is enabled.

Change the SASL IRCv3 capability check to look for SASL PLAIN for
username/password authentication, and SASL EXTERNAL for SSL
certificate authentication.  Update the Details... dialog box info to
match.

This should hopefully address some confusion about when SASL is
actually in use, versus things like NickServ certificate fingerprint
sign-in.

Change the wording for SASL EXTERNAL from "Supported by network" to
"May be supported by network", as SASL v3.1 does not provide in the
capability negotiation message which SASL variants are supported.
SASL EXTERNAL seems less common than SASL PLAIN.

src/qtui/settingspages/networkssettingspage.cpp
src/qtui/settingspages/networkssettingspage.h
src/qtui/settingspages/networkssettingspage.ui

index d4fdca1..de84f37 100644 (file)
@@ -372,11 +372,15 @@ void NetworksSettingsPage::setNetworkCapStates(NetworkId id)
         if (net->connectionState() != Network::Disconnected) {
             // Network exists and is connected, check available capabilities...
             // [SASL]
-            if (net->saslMaybeSupports(IrcCap::SaslMech::PLAIN)) {
-                setSASLStatus(CapSupportStatus::MaybeSupported);
+            // Quassel switches between SASL PLAIN and SASL EXTERNAL based on the existence of an
+            // SSL certificate on the identity - check EXTERNAL if CertID exists, PLAIN if not
+            bool usingSASLExternal = displayedNetworkHasCertId();
+            if ((usingSASLExternal && net->saslMaybeSupports(IrcCap::SaslMech::EXTERNAL))
+                    || (!usingSASLExternal && net->saslMaybeSupports(IrcCap::SaslMech::PLAIN))) {
+                setCapSASLStatus(CapSupportStatus::MaybeSupported, usingSASLExternal);
             }
             else {
-                setSASLStatus(CapSupportStatus::MaybeUnsupported);
+                setCapSASLStatus(CapSupportStatus::MaybeUnsupported, usingSASLExternal);
             }
 
             // Add additional capability-dependent interface updates here
@@ -384,7 +388,7 @@ void NetworksSettingsPage::setNetworkCapStates(NetworkId id)
         else {
             // Network is disconnected
             // [SASL]
-            setSASLStatus(CapSupportStatus::Disconnected);
+            setCapSASLStatus(CapSupportStatus::Disconnected);
 
             // Add additional capability-dependent interface updates here
         }
@@ -393,7 +397,7 @@ void NetworksSettingsPage::setNetworkCapStates(NetworkId id)
         // Capability negotiation is not supported and/or network doesn't exist.
         // Don't assume anything and reset all capability-dependent interface elements to neutral.
         // [SASL]
-        setSASLStatus(CapSupportStatus::Unknown);
+        setCapSASLStatus(CapSupportStatus::Unknown);
 
         // Add additional capability-dependent interface updates here
     }
@@ -730,11 +734,12 @@ void NetworksSettingsPage::clientNetworkCapsUpdated()
     }
 }
 
-void NetworksSettingsPage::setSASLStatus(const CapSupportStatus saslStatus)
+void NetworksSettingsPage::setCapSASLStatus(const CapSupportStatus saslStatus, bool usingSASLExternal)
 {
-    if (_saslStatusSelected != saslStatus) {
+    if (_capSaslStatusSelected != saslStatus || _capSaslStatusUsingExternal != usingSASLExternal) {
         // Update the cached copy of SASL status used with the Details dialog
-        _saslStatusSelected = saslStatus;
+        _capSaslStatusSelected = saslStatus;
+        _capSaslStatusUsingExternal = usingSASLExternal;
 
         // Update the user interface
         switch (saslStatus) {
@@ -750,14 +755,23 @@ void NetworksSettingsPage::setSASLStatus(const CapSupportStatus saslStatus)
             ui.saslStatusIcon->setPixmap(questionIcon.pixmap(16));
             break;
         case CapSupportStatus::MaybeUnsupported:
-            // The network doesn't advertise support for SASL PLAIN.  Here be dragons.
+            // The network doesn't advertise support for SASL PLAIN/EXTERNAL.  Here be dragons.
             ui.saslStatusLabel->setText(QString("<i>%1</i>").arg(tr("Not currently supported by network")));
             ui.saslStatusIcon->setPixmap(unavailableIcon.pixmap(16));
             break;
         case CapSupportStatus::MaybeSupported:
-            // The network advertises support for SASL PLAIN.  Encourage using it!
+            // The network advertises support for SASL PLAIN/EXTERNAL.  Encourage using it!
             // Unfortunately we don't know for sure if it's desired or functional.
-            ui.saslStatusLabel->setText(QString("<i>%1</i>").arg(tr("Supported by network")));
+            if (usingSASLExternal) {
+                // SASL EXTERNAL is used
+                // With SASL v3.1, it's not possible to reliably tell if SASL EXTERNAL is supported,
+                // or just SASL PLAIN.  Use less assertive phrasing.
+                ui.saslStatusLabel->setText(QString("<i>%1</i>").arg(tr("May be supported by network")));
+            }
+            else {
+                // SASL PLAIN is used
+                ui.saslStatusLabel->setText(QString("<i>%1</i>").arg(tr("Supported by network")));
+            }
             ui.saslStatusIcon->setPixmap(successIcon.pixmap(16));
             break;
         }
@@ -766,25 +780,29 @@ void NetworksSettingsPage::setSASLStatus(const CapSupportStatus saslStatus)
 
 void NetworksSettingsPage::sslUpdated()
 {
-    if (_cid && !_cid->sslKey().isNull()) {
-        ui.saslContents->setDisabled(true);
+    if (displayedNetworkHasCertId()) {
+        ui.saslPlainContents->setDisabled(true);
         ui.saslExtInfo->setHidden(false);
     }
     else {
-        ui.saslContents->setDisabled(false);
+        ui.saslPlainContents->setDisabled(false);
         // Directly re-enabling causes the widgets to ignore the parent "Use SASL Authentication"
         // state to indicate whether or not it's disabled.  To workaround this, keep track of
         // whether or not "Use SASL Authentication" is enabled, then quickly uncheck/recheck the
         // group box.
         if (!ui.sasl->isChecked()) {
-            // SASL is not enabled, uncheck/recheck the group box to re-disable saslContents.
-            // Leaving saslContents disabled doesn't work as that prevents it from re-enabling if
+            // SASL is not enabled, uncheck/recheck the group box to re-disable saslPlainContents.
+            // Leaving saslPlainContents disabled doesn't work as that prevents it from re-enabling if
             // sasl is later checked.
             ui.sasl->setChecked(true);
             ui.sasl->setChecked(false);
         }
         ui.saslExtInfo->setHidden(true);
     }
+    // Update whether SASL PLAIN or SASL EXTERNAL is used to detect SASL status
+    if (currentId != 0) {
+        setNetworkCapStates(currentId);
+    }
 }
 
 /*** Network list ***/
@@ -968,7 +986,7 @@ void NetworksSettingsPage::on_saslStatusDetails_clicked()
         bool useWarningIcon = false;
 
         // Determine which explanation to show
-        switch (_saslStatusSelected) {
+        switch (_capSaslStatusSelected) {
         case CapSupportStatus::Unknown:
             saslStatusHeader = tr("Could not check if SASL supported by network");
             saslStatusExplanation = tr("Quassel could not check if \"%1\" supports SASL.  This may "
@@ -984,17 +1002,41 @@ void NetworksSettingsPage::on_saslStatusDetails_clicked()
                                         .arg(netName);
             break;
         case CapSupportStatus::MaybeUnsupported:
-            saslStatusHeader = tr("SASL not currently supported by network");
-            saslStatusExplanation = tr("The network \"%1\" does not currently support SASL.  "
-                                       "However, support might be added later on.")
-                                        .arg(netName);
+            if (displayedNetworkHasCertId()) {
+                // SASL EXTERNAL is used
+                saslStatusHeader = tr("SASL EXTERNAL not currently supported by network");
+                saslStatusExplanation = tr("The network \"%1\" does not currently support SASL "
+                                           "EXTERNAL for SSL certificate authentication.  However, "
+                                           "support might be added later on.")
+                                           .arg(netName);
+            }
+            else {
+                // SASL PLAIN is used
+                saslStatusHeader = tr("SASL not currently supported by network");
+                saslStatusExplanation = tr("The network \"%1\" does not currently support SASL.  "
+                                           "However, support might be added later on.")
+                                           .arg(netName);
+            }
             useWarningIcon = true;
             break;
         case CapSupportStatus::MaybeSupported:
-            saslStatusHeader = tr("SASL supported by network");
-            saslStatusExplanation = tr("The network \"%1\" supports SASL.  In most cases, you "
-                                       "should use SASL instead of NickServ identification.")
-                                        .arg(netName);
+            if (displayedNetworkHasCertId()) {
+                // SASL EXTERNAL is used
+                // With SASL v3.1, it's not possible to reliably tell if SASL EXTERNAL is supported,
+                // or just SASL PLAIN.  Caution about this in the details dialog.
+                saslStatusHeader = tr("SASL EXTERNAL may be supported by network");
+                saslStatusExplanation = tr("The network \"%1\" may support SASL EXTERNAL for SSL "
+                                           "certificate authentication.  In most cases, you should "
+                                           "use SASL instead of NickServ identification.")
+                                            .arg(netName);
+            }
+            else {
+                // SASL PLAIN is used
+                saslStatusHeader = tr("SASL supported by network");
+                saslStatusExplanation = tr("The network \"%1\" supports SASL.  In most cases, you "
+                                           "should use SASL instead of NickServ identification.")
+                                            .arg(netName);
+            }
             break;
         }
 
@@ -1095,6 +1137,12 @@ IdentityId NetworksSettingsPage::defaultIdentity() const
     return defaultId;
 }
 
+bool NetworksSettingsPage::displayedNetworkHasCertId() const
+{
+    // Check if the CertIdentity exists and that it has a non-null SSL key set
+    return (_cid && !_cid->sslKey().isNull());
+}
+
 /**************************************************************************
  * NetworkAddDlg
  *************************************************************************/
index 73bee52..d6614d9 100644 (file)
@@ -150,7 +150,8 @@ private:
     // Status icons
     QIcon infoIcon, successIcon, unavailableIcon, questionIcon;
 
-    CapSupportStatus _saslStatusSelected;  /// Status of SASL support for currently-selected network
+    CapSupportStatus _capSaslStatusSelected;  ///< Status of SASL support for selected network
+    bool _capSaslStatusUsingExternal{false};  ///< Whether SASL support status is for SASL EXTERNAL
 
     void reset();
     bool testHasChanged();
@@ -160,12 +161,20 @@ private:
     void saveToNetworkInfo(NetworkInfo&);
     IdentityId defaultIdentity() const;
 
+    /**
+     * Get whether or not the displayed network's identity has SSL certs associated with it
+     *
+     * @return True if the currently displayed network has SSL certs set, otherwise false
+     */
+    bool displayedNetworkHasCertId() const;
+
     /**
      * Update the SASL settings interface according to the given SASL state
      *
-     * @param[in] saslStatus Current status of SASL support.
+     * @param saslStatus         Current status of SASL support.
+     * @param usingSASLExternal  If true, SASL support status is for SASL EXTERNAL, else SASL PLAIN
      */
-    void setSASLStatus(const CapSupportStatus saslStatus);
+    void setCapSASLStatus(const CapSupportStatus saslStatus, bool usingSASLExternal = false);
 };
 
 class NetworkAddDlg : public QDialog
index 03a8321..1f7983a 100644 (file)
@@ -682,7 +682,7 @@ Note that Quassel IRC automatically rejoins channels, so /join will rarely be ne
             </property>
             <layout class="QVBoxLayout" name="verticalLayout_11">
              <item>
-              <widget class="QFrame" name="saslContents">
+              <widget class="QFrame" name="saslPlainContents">
                <property name="frameShape">
                 <enum>QFrame::NoFrame</enum>
                </property>
@@ -755,53 +755,53 @@ Note that Quassel IRC automatically rejoins channels, so /join will rarely be ne
                   </item>
                  </layout>
                 </item>
-                <item>
-                 <layout class="QHBoxLayout" name="horizontalLayout_7">
-                  <item>
-                   <widget class="QLabel" name="saslStatusIcon">
-                    <property name="text">
-                     <string notr="true">[icon]</string>
-                    </property>
-                   </widget>
-                  </item>
-                  <item>
-                   <widget class="QLabel" name="saslStatusLabel">
-                    <property name="sizePolicy">
-                     <sizepolicy hsizetype="Expanding" vsizetype="Preferred">
-                      <horstretch>0</horstretch>
-                      <verstretch>0</verstretch>
-                     </sizepolicy>
-                    </property>
-                    <property name="text">
-                     <string>Could not detect if supported by server</string>
-                    </property>
-                   </widget>
-                  </item>
-                  <item>
-                   <widget class="QPushButton" name="saslStatusDetails">
-                    <property name="text">
-                     <string>Details...</string>
-                    </property>
-                   </widget>
-                  </item>
-                 </layout>
-                </item>
                </layout>
               </widget>
              </item>
+             <item>
+              <widget class="QLabel" name="saslExtInfo">
+               <property name="text">
+                <string>&lt;html&gt;&lt;head/&gt;&lt;body&gt;&lt;p&gt;&lt;span style=&quot; font-weight:600;&quot;&gt;Note:&lt;/span&gt; because the identity has an ssl certificate set, SASL EXTERNAL will be used.&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</string>
+               </property>
+               <property name="wordWrap">
+                <bool>true</bool>
+               </property>
+              </widget>
+             </item>
+             <item>
+              <layout class="QHBoxLayout" name="horizontalLayout_7">
+               <item>
+                <widget class="QLabel" name="saslStatusIcon">
+                 <property name="text">
+                  <string notr="true">[icon]</string>
+                 </property>
+                </widget>
+               </item>
+               <item>
+                <widget class="QLabel" name="saslStatusLabel">
+                 <property name="sizePolicy">
+                  <sizepolicy hsizetype="Expanding" vsizetype="Preferred">
+                   <horstretch>0</horstretch>
+                   <verstretch>0</verstretch>
+                  </sizepolicy>
+                 </property>
+                 <property name="text">
+                  <string>Could not detect if supported by server</string>
+                 </property>
+                </widget>
+               </item>
+               <item>
+                <widget class="QPushButton" name="saslStatusDetails">
+                 <property name="text">
+                  <string>Details...</string>
+                 </property>
+                </widget>
+               </item>
+              </layout>
+             </item>
             </layout>
            </widget>
           </item>
-          <item>
-           <widget class="QLabel" name="saslExtInfo">
-            <property name="text">
-             <string>&lt;html&gt;&lt;head/&gt;&lt;body&gt;&lt;p&gt;&lt;span style=&quot; font-weight:600;&quot;&gt;Note:&lt;/span&gt; because the identity has an ssl certificate set, SASL EXTERNAL will be used.&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</string>
-            </property>
-            <property name="wordWrap">
-             <bool>true</bool>
-            </property>
-           </widget>
-          </item>
           <item>
            <widget class="QGroupBox" name="autoIdentify">
             <property name="enabled">