cleanup after pagination changes

nexus/src/external_api/http_entrypoints.rs

@@ -3208,7 +3208,11 @@ async fn silo_users_get(
             .into_iter()
             .map(|i| i.into())
             .collect();
-        Ok(HttpResponseOk(ScanById::results_page(&query, users)?))
+        Ok(HttpResponseOk(ScanById::results_page(
+            &query,
+            users,
+            &|_, user: &User| user.id,
+        )?))
     };
     apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await
 }

nexus/src/external_api/views.rs

@@ -379,55 +379,17 @@ impl From<model::Sled> for Sled {
 // SILO USERS
 
 /// Client view of a [`User`]
-#[derive(ObjectIdentity, Clone, Debug, Deserialize, Serialize, JsonSchema)]
-#[serde(from = "UserIncoming")]
+#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)]
 pub struct User {
     pub id: Uuid,
-    // See omicron_common::api::external::Saga.
-    #[serde(skip)]
-    pub identity: IdentityMetadata,
 }
 
 impl From<model::SiloUser> for User {
     fn from(user: model::SiloUser) -> Self {
-        let identity = user.identity();
-        let id = identity.id;
-        Self { id, identity }
-    }
-}
-
-// XXX-dap once I fix what's below, derive this
-impl Eq for User {}
-impl PartialEq for User {
-    fn eq(&self, other: &Self) -> bool {
-        self.id == other.id
+        Self { id: user.id() }
     }
 }
 
-// XXX-dap this whole thing is stupid -- see the TODO-cleanup in Saga and
-// implement that workaround
-impl From<UserIncoming> for User {
-    fn from(user: UserIncoming) -> Self {
-        let now = chrono::Utc::now();
-        Self {
-            id: user.id,
-            identity: IdentityMetadata {
-                id: user.id,
-                name: "no-name".parse().unwrap(),
-                description: String::from("no description"),
-                time_created: now,
-                time_modified: now,
-            }
-        }
-    }
-}
-
-#[derive(Deserialize)]
-struct UserIncoming {
-    pub id: Uuid,
-}
-
-
 // BUILT-IN USERS
 
 /// Client view of a [`UserBuiltin`]

nexus/tests/integration_tests/silos.rs

@@ -24,6 +24,8 @@ use nexus_test_utils_macros::nexus_test;
 use omicron_nexus::authz::SiloRole;
 
 use httptest::{matchers::*, responders::*, Expectation, Server};
+use omicron_nexus::authn::{USER_TEST_PRIVILEGED, USER_TEST_UNPRIVILEGED};
+use omicron_nexus::db::fixed_data::silo::SILO_ID;
 
 #[nexus_test]
 async fn test_silos(cptestctx: &ControlPlaneTestContext) {
@@ -1138,28 +1140,42 @@ async fn test_saml_idp_rsa_keypair_ok(cptestctx: &ControlPlaneTestContext) {
 #[nexus_test]
 async fn test_silo_users_list(cptestctx: &ControlPlaneTestContext) {
     let client = &cptestctx.external_client;
-    let silo_users = NexusRequest::iter_collection_authn::<views::User>(
-        client, "/users", "", None,
-    )
-    .await
-    .expect("failed to list silo users")
-    .all_items
-    .iter()
-    .map(|u| u.id.to_string())
-    .collect::<Vec<String>>();
+    let initial_silo_users: Vec<views::User> =
+        NexusRequest::iter_collection_authn(client, "/users", "", None)
+            .await
+            .expect("failed to list silo users (1)")
+            .all_items;
 
     // In the built-in Silo, we expect the test-privileged and test-unprivileged
     // users.
-    // XXX-dap should compare entire objects to make sure we're not missing
-    // other fields.
     assert_eq!(
-        silo_users,
+        initial_silo_users,
+        vec![
+            views::User { id: USER_TEST_PRIVILEGED.id },
+            views::User { id: USER_TEST_UNPRIVILEGED.id },
+        ]
+    );
+
+    // Now create another user and make sure we can see them.  While we're at
+    // it, use a small limit to check that pagination is really working.
+    let new_silo_user_id =
+        "bd75d207-37f3-4769-b808-677ae04eaf23".parse().unwrap();
+    nexus.silo_user_create(SILO_ID, new_silo_user_id).await.unwrap();
+
+    let silo_users: Vec<views::User> =
+        NexusRequest::iter_collection_authn(client, "/users", "", Some(1))
+            .await
+            .expect("failed to list silo users (2)")
+            .all_items;
+    assert_eq!(
+        initial_silo_users,
         vec![
-            "001de000-05e4-4000-8000-000000004007",
-            "001de000-05e4-4000-8000-000000060001"
+            views::User { id: USER_TEST_PRIVILEGED.id },
+            views::User { id: USER_TEST_UNPRIVILEGED.id },
+            views::User { id: new_silo_user_id },
         ]
     );
 
-    // XXX-dap TODO-coverage create a user, look for it, then remove it, then
-    // list them again and make sure it's gone.
+    // TODO-coverage When we have a way to remove or invalidate Silo Users, we
+    // should test that doing so causes them to stop appearing in the list.
 }