Added IpAddress GraphQL scalar for IP addresses

Close #353

`Nic` and `NicInput` that are not used as additional content have been
removed.

CHANGELOG.md

@@ -7,6 +7,17 @@ Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
 
+### Added
+
+- Added `IpAddress` GraphQL custom scalar for IP addresses.
+  - Applied it to the GraphQL APIs `ipLocation`, `ipLocationList`,
+    `insertAccount`, `updateAccount`, `insertSamplingPolicy`, and
+    `updateSamplingPolicy`.
+  - The API returns a specific error message if the value cannot be parsed
+    into `IpAddr`.
+    For example: `Failed to parse "IpAddress": Invalid IP address: abc(occurred
+while parsing "[IpAddress!]")`
+
 ### Changed
 
 - Updated the encoding and decoding method for GraphQL cursors by removing

src/graphql.rs

@@ -36,14 +36,17 @@ mod trusted_user_agent;
 
 use std::fmt;
 use std::future::Future;
+use std::net::IpAddr;
 #[cfg(test)]
 use std::net::SocketAddr;
 use std::sync::{Arc, Mutex};
 
-use async_graphql::connection::{ConnectionNameType, CursorType, EdgeNameType, OpaqueCursor};
+use async_graphql::connection::{
+    Connection, ConnectionNameType, CursorType, Edge, EdgeNameType, EmptyFields, OpaqueCursor,
+};
 use async_graphql::{
-    connection::{Connection, Edge, EmptyFields},
-    Context, Guard, MergedObject, MergedSubscription, ObjectType, OutputType, Result,
+    Context, Guard, InputValueError, InputValueResult, MergedObject, MergedSubscription,
+    ObjectType, OutputType, Result, Scalar, ScalarType, Value,
 };
 use chrono::TimeDelta;
 use num_traits::ToPrimitive;
@@ -510,6 +513,26 @@ impl Guard for RoleGuard {
     }
 }
 
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct IpAddress(pub IpAddr);
+
+#[Scalar]
+impl ScalarType for IpAddress {
+    fn parse(value: Value) -> InputValueResult<Self> {
+        match value {
+            Value::String(s) => s
+                .parse::<IpAddr>()
+                .map(IpAddress)
+                .map_err(|_| InputValueError::custom(format!("Invalid IP address: {s}"))),
+            _ => Err(InputValueError::expected_type(value)),
+        }
+    }
+
+    fn to_value(&self) -> Value {
+        Value::String(self.0.to_string())
+    }
+}
+
 fn fill_vacant_time_slots(series: &[database::TimeCount]) -> Vec<database::TimeCount> {
     let mut filled_series: Vec<database::TimeCount> = Vec::new();
 

src/graphql/account.rs

@@ -1,6 +1,6 @@
 use std::{
     env,
-    net::{AddrParseError, IpAddr, SocketAddr},
+    net::{IpAddr, SocketAddr},
 };
 
 use anyhow::anyhow;
@@ -18,7 +18,7 @@ use review_database::{
 use serde::Serialize;
 use tracing::info;
 
-use super::RoleGuard;
+use super::{IpAddress, RoleGuard};
 use crate::auth::{create_token, decode_token, insert_token, revoke_token, update_jwt_expires_in};
 use crate::graphql::query_with_constraints;
 
@@ -157,7 +157,7 @@ impl AccountMutation {
         name: String,
         department: String,
         language: Option<String>,
-        allow_access_from: Option<Vec<String>>,
+        allow_access_from: Option<Vec<IpAddress>>,
         max_parallel_sessions: Option<u32>,
     ) -> Result<String> {
         let store = crate::graphql::get_store(ctx).await?;
@@ -166,7 +166,7 @@ impl AccountMutation {
             return Err("account already exists".into());
         }
         let allow_access_from = if let Some(ip_addrs) = allow_access_from {
-            let ip_addrs = strings_to_ip_addrs(&ip_addrs)?;
+            let ip_addrs = to_ip_addr(&ip_addrs);
             Some(ip_addrs)
         } else {
             None
@@ -272,16 +272,8 @@ impl AccountMutation {
         let dept = department.map(|d| (d.old, d.new));
         let language = language.map(|d| (d.old, d.new));
         let allow_access_from = if let Some(ip_addrs) = allow_access_from {
-            let old = if let Some(old) = ip_addrs.old {
-                Some(strings_to_ip_addrs(&old)?)
-            } else {
-                None
-            };
-            let new = if let Some(new) = ip_addrs.new {
-                Some(strings_to_ip_addrs(&new)?)
-            } else {
-                None
-            };
+            let old = ip_addrs.old.map(|old| to_ip_addr(&old));
+            let new = ip_addrs.new.map(|new| to_ip_addr(&new));
             Some((old, new))
         } else {
             None
@@ -651,13 +643,14 @@ impl From<types::Account> for Account {
     }
 }
 
-fn strings_to_ip_addrs(ip_addrs: &[String]) -> Result<Vec<IpAddr>, AddrParseError> {
+fn to_ip_addr(ip_addrs: &[IpAddress]) -> Vec<IpAddr> {
     let mut ip_addrs = ip_addrs
         .iter()
-        .map(|ip_addr| ip_addr.parse::<IpAddr>())
-        .collect::<Result<Vec<_>, _>>()?;
-    ip_addrs.sort();
-    Ok(ip_addrs)
+        .map(|ip_addr| ip_addr.0)
+        .collect::<Vec<IpAddr>>();
+    ip_addrs.sort_unstable();
+    ip_addrs.dedup();
+    ip_addrs
 }
 
 #[derive(SimpleObject)]
@@ -705,8 +698,8 @@ struct UpdateLanguage {
 /// The old and new values of `allowAccessFrom` to update.
 #[derive(InputObject)]
 struct UpdateAllowAccessFrom {
-    old: Option<Vec<String>>,
-    new: Option<Vec<String>>,
+    old: Option<Vec<IpAddress>>,
+    new: Option<Vec<IpAddress>>,
 }
 
 /// The old and new values of `maxParallelSessions` to update.
@@ -1372,7 +1365,8 @@ mod tests {
                         role: "SECURITY_ADMINISTRATOR",
                         name: "John Doe",
                         department: "Security",
-                        language: "en-US"
+                        language: "en-US",
+                        allowAccessFrom: ["127.0.0.1"]
                     )
                 }"#,
             )
@@ -1422,6 +1416,10 @@ mod tests {
                         language: {
                             old: "en-US",
                             new: "ko-KR"
+                        },
+                        allowAccessFrom: {
+                            old: "127.0.0.1",
+                            new: "127.0.0.2"
                         }
                     )
                 }"#,
@@ -1440,14 +1438,53 @@ mod tests {
                         name
                         department
                         language
+                        allowAccessFrom
                     }
                 }"#,
             )
             .await;
 
         assert_eq!(
             res.data.to_string(),
-            r#"{account: {username: "username", role: SYSTEM_ADMINISTRATOR, name: "Loren Ipsum", department: "Admin", language: "ko-KR"}}"#
+            r#"{account: {username: "username", role: SYSTEM_ADMINISTRATOR, name: "Loren Ipsum", department: "Admin", language: "ko-KR", allowAccessFrom: ["127.0.0.2"]}}"#
+        );
+
+        let res = schema
+            .execute(
+                r#"
+                mutation {
+                    updateAccount(
+                        username: "username",
+                        password: "password",
+                        role: {
+                            old: "SECURITY_ADMINISTRATOR",
+                            new: "SYSTEM_ADMINISTRATOR"
+                        },
+                        name: {
+                            old: "John Doe",
+                            new: "Loren Ipsum"
+                        },
+                        department: {
+                            old: "Security",
+                            new: "Admin"
+                        },
+                        language: {
+                            old: "en-US",
+                            new: "ko-KR"
+                        },
+                        allowAccessFrom: {
+                            old: "127.0.0.2",
+                            new: "127.0.0.x"
+                        }
+                    )
+                }"#,
+            )
+            .await;
+        assert_eq!(
+            res.errors.first().unwrap().message.to_string(),
+            "Failed to parse \"IpAddress\": Invalid IP address: 127.0.0.x (occurred while \
+            parsing \"[IpAddress!]\") (occurred while parsing \"UpdateAllowAccessFrom\")"
+                .to_string()
         );
     }
 
@@ -1599,6 +1636,34 @@ mod tests {
         assert!(res.is_err());
     }
 
+    #[tokio::test]
+    async fn invalid_ip_allow_access_from() {
+        let agent_manager: BoxedAgentManager = Box::new(MockAgentManager {});
+        let test_addr: SocketAddr = "127.0.0.1:8080".parse().unwrap();
+
+        let schema = TestSchema::new_with(agent_manager, Some(test_addr)).await;
+        let res = schema
+            .execute(
+                r#"mutation {
+                    insertAccount(
+                        username: "u1",
+                        password: "pw1",
+                        role: "SECURITY_ADMINISTRATOR",
+                        name: "User One",
+                        department: "Test",
+                        allowAccessFrom: ["127.0.0.x"]
+                    )
+                }"#,
+            )
+            .await;
+        assert_eq!(
+            res.errors.first().unwrap().message.to_string(),
+            "Failed to parse \"IpAddress\": Invalid IP address: 127.0.0.x (occurred while \
+            parsing \"[IpAddress!]\")"
+                .to_string()
+        );
+    }
+
     #[tokio::test]
     async fn language() {
         let schema = TestSchema::new().await;

src/graphql/ip_location.rs

@@ -1,11 +1,8 @@
-use std::{
-    net::IpAddr,
-    sync::{Arc, Mutex},
-};
+use std::sync::{Arc, Mutex};
 
 use async_graphql::{Context, Object, Result, SimpleObject};
 
-use super::{Role, RoleGuard};
+use super::{IpAddress, Role, RoleGuard};
 const MAX_NUM_IP_LOCATION_LIST: usize = 200;
 
 #[derive(Default)]
@@ -18,10 +15,12 @@ impl IpLocationQuery {
         .or(RoleGuard::new(Role::SecurityAdministrator))
         .or(RoleGuard::new(Role::SecurityManager))
         .or(RoleGuard::new(Role::SecurityMonitor))")]
-    async fn ip_location(&self, ctx: &Context<'_>, address: String) -> Result<Option<IpLocation>> {
-        let Ok(addr) = address.parse::<IpAddr>() else {
-            return Err("invalid IP address".into());
-        };
+    async fn ip_location(
+        &self,
+        ctx: &Context<'_>,
+        address: IpAddress,
+    ) -> Result<Option<IpLocation>> {
+        let addr = address.0;
         let Ok(mutex) = ctx.data::<Arc<Mutex<ip2location::DB>>>() else {
             return Err("IP location database unavailable".into());
         };
@@ -47,7 +46,7 @@ impl IpLocationQuery {
     async fn ip_location_list(
         &self,
         ctx: &Context<'_>,
-        mut addresses: Vec<String>,
+        mut addresses: Vec<IpAddress>,
     ) -> Result<Vec<IpLocationItem>> {
         let Ok(mutex) = ctx.data::<Arc<Mutex<ip2location::DB>>>() else {
             return Err("IP location database unavailable".into());
@@ -59,19 +58,17 @@ impl IpLocationQuery {
             .map_err(|_| "Failed to lock IP location database")?;
         let records = addresses
             .iter()
-            .filter_map(|address| {
-                address.parse::<IpAddr>().ok().and_then(|addr| {
-                    locator
-                        .ip_lookup(addr)
-                        .ok()
-                        .map(std::convert::TryInto::try_into)
-                        .and_then(|r| {
-                            r.ok().map(|location| IpLocationItem {
-                                address: address.clone(),
-                                location,
-                            })
+            .filter_map(|addr| {
+                locator
+                    .ip_lookup(addr.0)
+                    .ok()
+                    .map(std::convert::TryInto::try_into)
+                    .and_then(|r| {
+                        r.ok().map(|location| IpLocationItem {
+                            address: addr.0.to_string(),
+                            location,
                         })
-                })
+                    })
             })
             .collect();