Merge PR #2568: Signatures: the message bar shows a problem before "Certified by"

crates/ui-egui/src/sign_ui.rs

@@ -1127,8 +1127,10 @@ pub(crate) fn banner(sigs: &[SignatureInfo]) -> Option<(&'static str, Color32, &
     if signed.is_empty() {
         return None;
     }
+    // A valid certification heads the bar only when every signature is valid: it says nothing
+    // about the signatures added after it, so their problems come first.
     if let Some(c) = signed.iter().find(|s| s.certify.is_some())
-        && c.status == SignatureStatus::Valid
+        && signed.iter().all(|s| s.status == SignatureStatus::Valid)
     {
         let by = c.certificate.as_ref().map(|x| {
             let org = x.subject.organization().map(|o| format!(", {o}")).unwrap_or_default();
@@ -1656,4 +1658,50 @@ mod tests {
         let nothing = Appearance { name: false, date: false, ..all };
         assert!(detail_lines(&nothing, &de, "Ada", "  ", "", "D").is_empty());
     }
+
+    fn signature(certify: Option<u8>, status: SignatureStatus) -> SignatureInfo {
+        SignatureInfo {
+            field: "Signature".into(),
+            signed: true,
+            page: None,
+            rect: None,
+            visible: false,
+            signer: None,
+            certificate: None,
+            chain: Vec::new(),
+            date: None,
+            signing_time: None,
+            reason: None,
+            location: None,
+            contact: None,
+            sub_filter: None,
+            doc_timestamp: false,
+            certify,
+            revision: 1,
+            signed_len: 0,
+            digest: None,
+            algorithm: None,
+            timestamp: false,
+            timestamp_time: None,
+            status,
+            modification: sign::Modification::None,
+            details: Vec::new(),
+        }
+    }
+
+    /// A valid certification doesn't hide a later signature that is invalid or can't be checked:
+    /// the message bar names the problem, and says "Certified by" only when all are valid.
+    #[test]
+    fn the_message_bar_puts_a_signature_problem_before_the_certification() {
+        use SignatureStatus::{Invalid, Unknown, Valid};
+        let bar = |sigs: &[SignatureInfo]| banner(sigs).map(|b| (b.0, b.2));
+        let certified = signature(Some(2), Valid);
+        assert_eq!(bar(std::slice::from_ref(&certified)), Some(("badge-check", "Certified by {by}")));
+        assert_eq!(bar(&[certified.clone(), signature(None, Valid)]), Some(("badge-check", "Certified by {by}")));
+        assert_eq!(bar(&[certified.clone(), signature(None, Invalid)]), Some(("circle-x", "At least one signature is invalid.")));
+        assert_eq!(bar(&[certified, signature(None, Unknown)]), Some(("triangle-alert", "At least one signature has problems.")));
+        assert_eq!(bar(&[signature(Some(2), Unknown), signature(None, Valid)]), Some(("triangle-alert", "At least one signature has problems.")));
+        assert_eq!(bar(&[signature(None, Valid)]), Some(("circle-check", "Signed and all signatures are valid.")));
+        assert_eq!(bar(&[SignatureInfo { signed: false, ..signature(None, Invalid) }]), None);
+    }
 }