Issue #680 : Crate owners can now remove themselves as owner if there is at least one other owner remaining.

Author: autodidaddict

src/krate.rs

@@ -1363,15 +1363,15 @@ fn modify_owners(req: &mut Request, add: bool) -> CargoResult<Response> {
             if owners.iter().any(|owner| owner.login() == *login) {
                 return Err(human(&format_args!("`{}` is already an owner", login)));
             }
-            krate.owner_add(req.app(), &conn, user, login)?;
+            krate.owner_add(req.app(), &conn, user, login)?;            
         } else {
             // Removing the team that gives you rights is prevented because
             // team members only have Rights::Publish
-            if *login == user.gh_login {
-                return Err(human("cannot remove yourself as an owner"));
-            }
-            krate.owner_remove(&conn, user, login)?;
-        }
+            if owners.len() == 1 {
+                return Err(human("cannot remove the sole owner of a crate"))
+            }            
+            krate.owner_remove(&conn, user, login)?;                    
+        }        
     }
 
     #[derive(Serialize)]

src/tests/krate.rs

@@ -1225,6 +1225,57 @@ fn following() {
     assert_eq!(::json::<CrateList>(&mut response).crates.len(), 0);
 }
 
+// Ensures that so long as at least one owner remains associated with the crate, 
+// a user can still remove their own login as an owner
+#[test]
+fn owners_can_remove_self() {
+  #[derive(Deserialize)]
+    struct R {
+        users: Vec<EncodablePublicUser>,
+    }
+    #[derive(Deserialize)]
+    struct O {
+        ok: bool,
+    }
+
+    let (_b, app, middle) = ::app();
+    let mut req = ::req(app.clone(), Method::Get, "/api/v1/crates/owners_selfremove/owners");
+    {
+        let conn = app.diesel_database.get().unwrap();
+        ::new_user("secondowner").create_or_update(&conn).unwrap();
+        let user = ::new_user("firstowner").create_or_update(&conn).unwrap();
+        ::sign_in_as(&mut req, &user);
+        ::CrateBuilder::new("owners_selfremove", user.id).expect_build(&conn);
+    }    
+
+    let mut response = ok_resp!(middle.call(&mut req));
+    let r: R = ::json(&mut response);
+    assert_eq!(r.users.len(), 1);
+
+    let mut response = ok_resp!(middle.call(req.with_method(Method::Get)));
+    let r: R = ::json(&mut response);
+    assert_eq!(r.users.len(), 1);
+
+    let body = r#"{"users":["secondowner"]}"#;
+    let mut response = ok_resp!(middle.call(req.with_method(Method::Put).with_body(
+        body.as_bytes(),
+    )));
+    assert!(::json::<O>(&mut response).ok);    
+
+    // This is a self delete from owners 
+    let body = r#"{"users":["firstowner"]}"#;  
+    let mut response = ok_resp!(middle.call(req.with_method(Method::Delete).with_body(
+        body.as_bytes(),
+    )));
+    assert!(::json::<O>(&mut response).ok);
+    
+    let body = r#"{"users":["secondowner"]}"#;  
+    let mut response = ok_resp!(middle.call(req.with_method(Method::Delete).with_body(
+        body.as_bytes(),
+    )));
+    ::json::<::Bad>(&mut response);
+}
+
 #[test]
 fn owners() {
     #[derive(Deserialize)]