Make client_id configurable

src/cli/client.rs

@@ -4,28 +4,31 @@ use url::Url;
 #[derive(Debug, Parser)]
 #[clap(max_term_width = 100)]
 pub struct ClientOptions {
-    #[clap(flatten)]
-    pub connection: ConnectionOptions,
-    #[clap(subcommand)]
-    pub command: ClientCommand,
-}
-
-#[derive(Debug, Parser)]
-pub struct ConnectionOptions {
     /// The host address of the numtracker service
     ///
     /// This should be the root of the service address including the scheme and
     /// port (if non-standard) but not including the graphql path,
     /// eg https://numtracker.example.com
     #[clap(long, short = 'H', env = "NUMTRACKER_SERVICE_HOST")]
     pub host: Option<Url>,
+    #[clap(flatten)]
+    pub auth: AuthConfig,
+    #[clap(subcommand)]
+    pub command: ClientCommand,
+}
+
+#[derive(Debug, Parser, PartialEq)]
+pub struct AuthConfig {
     /// The host address of the authorisation provider
     ///
     /// This should be the domain that has the .well-known/openid-configuration
     /// endpoint including scheme and port (if non-standard).
     /// eg https://authn.example.com/realms/master
     #[clap(long, env = "NUMTRACKER_AUTH_HOST")]
     pub auth: Option<Url>,
+    /// The client ID to use when authenticating
+    #[clap(long, env = "NUMTRACKER_AUTHN_CLIENTID")]
+    pub client_id: Option<String>,
 }
 
 #[derive(Debug, Subcommand)]

src/cli/mod.rs

@@ -71,7 +71,7 @@ pub enum Command {
     Client { _ignored: Vec<String> },
     /// View and update beamline configurations provided by an instance of the service
     #[cfg(feature = "client")]
-    Client(client::ClientOptions),
+    Client(Box<client::ClientOptions>),
     /// Generate the graphql schema
     Schema,
 }
@@ -202,8 +202,7 @@ mod tests {
     use clap::Parser;
     use tracing::Level;
 
-    use super::Cli;
-    use crate::cli::Command;
+    use super::*;
     const APP: &str = "numtracker";
 
     #[test]
@@ -413,4 +412,20 @@ mod tests {
             panic!("Unexpected command: {:?}", cli.command);
         };
     }
+
+    #[test]
+    fn minimal_client_command() {
+        let cli = Cli::try_parse_from([APP, "client", "configuration"]).unwrap();
+        let Command::Client(opts) = cli.command else {
+            panic!("Client command returned {cli:?}");
+        };
+        assert_eq!(opts.host, None);
+        assert_eq!(
+            opts.auth,
+            client::AuthConfig {
+                auth: None,
+                client_id: None
+            }
+        );
+    }
 }

src/client/cli_auth.rs

@@ -96,7 +96,10 @@ pub enum AuthError {
 }
 
 impl AuthHandler {
-    pub async fn new(host: impl Into<Url>) -> Result<Self, AuthError> {
+    pub async fn new(
+        host: impl Into<Url>,
+        client_id: impl Into<String>,
+    ) -> Result<Self, AuthError> {
         let http_client = reqwest::ClientBuilder::new()
             .redirect(Policy::none())
             .build()?;
@@ -109,7 +112,7 @@ impl AuthHandler {
             .clone();
         let client = CoreClient::from_provider_metadata(
             meta_provider,
-            ClientId::new("numtracker".to_string()),
+            ClientId::new(client_id.into()),
             None,
         )
         .set_device_authorization_url(device_authorization_url)
@@ -203,9 +206,9 @@ async fn refresh_access_token(auth: &AuthHandler) -> Option<String> {
 
 /// Get a new access token from the auth server via the device flow.
 /// If successful, cache the refresh token to prevent needing to log in next time
-pub(crate) async fn get_access_token(h: &Url) -> Result<String, AuthError> {
+pub(crate) async fn get_access_token(host: &Url, client_id: &str) -> Result<String, AuthError> {
     debug!("Getting new access token");
-    let handler = AuthHandler::new(h.clone()).await?;
+    let handler = AuthHandler::new(host.clone(), client_id).await?;
     if let Some(token) = refresh_access_token(&handler).await {
         return Ok(token);
     }

src/client/config.rs

@@ -8,10 +8,13 @@ use tokio::fs;
 use tracing::debug;
 use url::Url;
 
+use crate::cli::client::AuthConfig;
+
 #[derive(Debug, Deserialize, Default)]
 pub struct ClientConfiguration {
     pub host: Option<Url>,
     pub auth: Option<Url>,
+    pub client_id: Option<String>,
 }
 
 #[derive(Debug, Display, Error, From)]
@@ -25,6 +28,8 @@ pub enum ConfigFileError {
 }
 
 impl ClientConfiguration {
+    const DEFAULT_CLIENT: &str = "numtracker";
+
     pub async fn from_file<P: AsRef<Path>>(path: P) -> Result<Self, ConfigFileError> {
         debug!("Reading client config from {:?}", path.as_ref());
         match fs::read_to_string(path.as_ref()).await {
@@ -51,13 +56,22 @@ impl ClientConfiguration {
         }
     }
 
+    pub(crate) fn auth_config(&self) -> Option<(&Url, &str)> {
+        let auth = self.auth.as_ref()?;
+        Some((
+            auth,
+            self.client_id.as_deref().unwrap_or(Self::DEFAULT_CLIENT),
+        ))
+    }
+
     pub(crate) fn with_host(mut self, host: Option<Url>) -> Self {
         self.host = host.or(self.host);
         self
     }
 
-    pub(crate) fn with_auth(mut self, auth: Option<Url>) -> Self {
-        self.auth = auth.or(self.auth);
+    pub(crate) fn with_auth(mut self, auth: AuthConfig) -> Self {
+        self.auth = auth.auth.or(self.auth);
+        self.client_id = auth.client_id.or(self.client_id);
         self
     }
 }
@@ -78,3 +92,72 @@ impl Display for ClientConfiguration {
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use std::fs::File;
+    use std::io::Write;
+
+    use tempfile::TempDir;
+
+    use super::*;
+
+    const HOST: &str = "http://numtracker.example.com";
+    const AUTH: &str = "https://auth.example.com";
+    const CLIENT_ID: &str = "custom_client";
+
+    #[tokio::test]
+    async fn load_from_file() {
+        let dir = TempDir::new().unwrap();
+        let cfg_file = dir.as_ref().join("config.toml");
+        let mut file = File::create_new(&cfg_file).unwrap();
+        writeln!(file, "host={HOST:?}").unwrap();
+
+        let cfg = ClientConfiguration::from_file(cfg_file).await.unwrap();
+        assert_eq!(cfg.host, Some(Url::parse(HOST).unwrap()));
+        assert_eq!(cfg.auth, None);
+        assert_eq!(cfg.client_id, None);
+
+        assert_eq!(cfg.auth_config(), None);
+    }
+
+    #[tokio::test]
+    async fn load_from_file_with_auth() {
+        let dir = TempDir::new().unwrap();
+        let cfg_file = dir.as_ref().join("config.toml");
+        let mut file = File::create_new(&cfg_file).unwrap();
+        writeln!(file, "host={HOST:?}\nauth={AUTH:?}").unwrap();
+
+        let cfg = ClientConfiguration::from_file(cfg_file).await.unwrap();
+        assert_eq!(cfg.host, Some(Url::parse(HOST).unwrap()));
+        assert_eq!(cfg.auth, Some(Url::parse(AUTH).unwrap()));
+        assert_eq!(cfg.client_id, None);
+
+        assert_eq!(
+            cfg.auth_config(),
+            Some((&Url::parse(AUTH).unwrap(), "numtracker"))
+        );
+    }
+
+    #[tokio::test]
+    async fn load_from_file_with_client_id() {
+        let dir = TempDir::new().unwrap();
+        let cfg_file = dir.as_ref().join("config.toml");
+        let mut file = File::create_new(&cfg_file).unwrap();
+        writeln!(
+            file,
+            "host={HOST:?}\nauth={AUTH:?}\nclient_id={CLIENT_ID:?}"
+        )
+        .unwrap();
+
+        let cfg = ClientConfiguration::from_file(cfg_file).await.unwrap();
+        assert_eq!(cfg.host, Some(Url::parse(HOST).unwrap()));
+        assert_eq!(cfg.auth, Some(Url::parse(AUTH).unwrap()));
+        assert_eq!(cfg.client_id, Some(CLIENT_ID.into()));
+
+        assert_eq!(
+            cfg.auth_config(),
+            Some((&Url::parse(AUTH).unwrap(), CLIENT_ID))
+        );
+    }
+}

src/client/mod.rs

@@ -21,14 +21,15 @@ pub enum ClientError {
 
 pub async fn run_client(options: ClientOptions) {
     let ClientOptions {
-        connection,
+        host,
+        auth,
         command,
     } = options;
 
     let conf = match ClientConfiguration::from_default_file().await {
         Ok(conf) => {
             info!("Configuration from file: {conf}");
-            conf.with_host(connection.host).with_auth(connection.auth)
+            conf.with_host(host).with_auth(auth)
         }
         Err(e) => {
             println!("Could not read configuration: {e}");
@@ -92,15 +93,15 @@ struct ConfigureMutation;
 
 impl NumtrackerClient {
     async fn from_config(config: ClientConfiguration) -> Result<Self, ClientError> {
+        let auth = match config.auth_config() {
+            Some((auth, client_id)) => Some(cli_auth::get_access_token(auth, client_id).await?),
+            None => None,
+        };
+
         let host = config.host.unwrap_or_else(|| {
             info!("No host specified, defaulting to localhost:8000");
             Url::parse("http://localhost:8000").expect("Constant URL is valid")
         });
-
-        let auth = match config.auth {
-            Some(auth) => Some(cli_auth::get_access_token(&auth).await?),
-            None => None,
-        };
         info!("Querying {host} with auth: {auth:?}");
         Ok(NumtrackerClient { auth, host })
     }

src/main.rs

@@ -38,7 +38,7 @@ async fn main() -> Result<(), Box<dyn Error>> {
             println!("Client subcommand requires 'client' feature to be enabled when building")
         }
         #[cfg(feature = "client")]
-        Command::Client(opts) => client::run_client(opts).await,
+        Command::Client(opts) => client::run_client(*opts).await,
         Command::Schema => {
             graphql::graphql_schema(std::io::stdout()).expect("Failed to write schema")
         }