Skip to content

feat(auth): add auth login --device-name to override the hostname #1887

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mergify merged 1 commit into from
Oct 6, 2026
Merged

feat(auth): add auth login --device-name to override the hostname #1887

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ restricted file under your configuration directory. `mergify auth status` says
which account you are signed in as, and `mergify auth logout` asks the Mergify
API to revoke the credential rather than only deleting the local copy.

The approval page labels the new token `Mergify CLI on <hostname>`. Pass
`--device-name "work laptop"` to name the machine yourself, or `--device-name=`
to keep the hostname from leaving the machine at all.

The commands that talk to the **Mergify API** resolve a credential in this
order:

Expand Down
105 changes: 80 additions & 25 deletions crates/mergify-auth/src/login.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ pub struct LoginOptions<'a> {
/// printed either way, and a browser that will not open does
/// not fail the login.
pub browser: Option<&'a dyn Browser>,
/// `--device-name`: what to call this machine on the approval
/// page instead of its hostname. `None` detects the hostname,
/// and a blank name sends none at all, for a user who does not
/// want the hostname to leave the machine: the grant request is
/// unauthenticated and the server holds the value before anyone
/// approves, so editing the field on the page is too late.
pub device_name: Option<&'a str>,
}

/// What `auth login` produced, for the JSON rendering `Output`
Expand Down Expand Up @@ -60,8 +67,13 @@ pub async fn run(opts: LoginOptions<'_>, output: &mut dyn Output) -> Result<(),
// every machine shares — the page's own helper text already asks
// the user to name it after the machine. Optional: a machine
// that cannot name itself, or a deployment that predates the
// field, falls back to that shared default.
let device_name = machine::name().await;
// field, falls back to that shared default. `--device-name`
// replaces the detected name, and a blank one is the opt-out:
// the same request a machine that cannot name itself makes.
let device_name = match opts.device_name {
Some(name) => (!name.trim().is_empty()).then(|| name.to_string()),
None => machine::name().await,
};
let authorization = device::authorize(&client, device_name.as_deref()).await?;
let opened = open_verification_page(opts.browser, verification_url(&authorization));
output.status(&instructions(&authorization, opened))?;
Expand Down Expand Up @@ -377,6 +389,7 @@ mod tests {
api_url: Some(&server.uri()),
store: &store,
browser: None,
device_name: None,
},
&mut captured.output,
)
Expand Down Expand Up @@ -417,6 +430,7 @@ mod tests {
api_url: Some(&server.uri()),
store: &store,
browser: None,
device_name: None,
},
&mut captured.output,
)
Expand Down Expand Up @@ -449,6 +463,7 @@ mod tests {
api_url: Some(&server.uri()),
store: &store,
browser: None,
device_name: None,
},
&mut captured.output,
)
Expand Down Expand Up @@ -480,6 +495,7 @@ mod tests {
api_url: Some(&server.uri()),
store: &store,
browser: None,
device_name: None,
},
&mut captured.output,
)
Expand Down Expand Up @@ -529,6 +545,7 @@ mod tests {
api_url: Some(&server.uri()),
store: &store,
browser: None,
device_name: None,
},
&mut captured.output,
)
Expand Down Expand Up @@ -564,6 +581,7 @@ mod tests {
api_url: Some(&server.uri()),
store: &store,
browser: None,
device_name: None,
},
&mut captured.output,
)
Expand Down Expand Up @@ -611,6 +629,7 @@ mod tests {
api_url: Some(&server.uri()),
store: &store,
browser: None,
device_name: None,
},
&mut captured.output,
)
Expand Down Expand Up @@ -661,6 +680,7 @@ mod tests {
api_url: Some(&server.uri()),
store: &store,
browser: None,
device_name: None,
},
&mut captured.output,
)
Expand Down Expand Up @@ -709,6 +729,7 @@ mod tests {
api_url: Some(&server.uri()),
store: &store,
browser: None,
device_name: None,
},
&mut captured.output,
)
Expand Down Expand Up @@ -753,6 +774,7 @@ mod tests {
api_url: Some(&server.uri()),
store: &store,
browser: Some(&browser),
device_name: None,
},
&mut captured.output,
)
Expand Down Expand Up @@ -792,6 +814,7 @@ mod tests {
api_url: Some(&server.uri()),
store: &store,
browser: Some(&browser),
device_name: None,
},
&mut captured.output,
)
Expand All @@ -816,6 +839,34 @@ mod tests {
});
}

/// The form body `run` sent on the grant request.
async fn grant_body(device_name: Option<&str>) -> String {
let server = MockServer::start().await;
mount_flow(&server, true).await;
let (dir, store) = file_store();
let mut captured = Captured::human();

run(
LoginOptions {
api_url: Some(&server.uri()),
store: &store,
browser: None,
device_name,
},
&mut captured.output,
)
.await
.unwrap();

let requests = server.received_requests().await.unwrap();
let grant = requests
.iter()
.find(|r| r.url.path() == "/v1/oauth/device/code")
.expect("the grant request");
drop(dir);
String::from_utf8_lossy(&grant.body).into_owned()
}

// The wiring, not the renderer: `device_name` has to leave this
// command on the grant request, or the approval page goes back
// to naming every machine "Mergify CLI". Asserted against what
Expand All @@ -824,33 +875,36 @@ mod tests {
#[test]
fn login_tells_the_server_which_machine_asked() {
with_mergify_token(None, async {
let server = MockServer::start().await;
mount_flow(&server, true).await;
let (dir, store) = file_store();
let mut captured = Captured::human();

run(
LoginOptions {
api_url: Some(&server.uri()),
store: &store,
browser: None,
},
&mut captured.output,
)
.await
.unwrap();

let requests = server.received_requests().await.unwrap();
let grant = requests
.iter()
.find(|r| r.url.path() == "/v1/oauth/device/code")
.expect("the grant request");
let body = String::from_utf8_lossy(&grant.body);
let body = grant_body(None).await;
match crate::machine::name().await {
Some(_) => assert!(body.contains("device_name="), "got {body:?}"),
None => assert_eq!(body, "client_id=mergify-cli", "got {body:?}"),
}
drop(dir);
});
}

// `--device-name` replaces the hostname rather than joining it,
// and goes out as typed: the server sanitizes, not the CLI.
#[test]
fn device_name_overrides_the_hostname() {
with_mergify_token(None, async {
let body = grant_body(Some("work laptop")).await;
assert_eq!(
body, "client_id=mergify-cli&device_name=work+laptop",
"got {body:?}"
);
});
}

// `--device-name ""` is the opt-out: no hostname leaves the
// machine, and the server keeps its own default label.
#[test]
fn empty_device_name_sends_none() {
with_mergify_token(None, async {
for blank in ["", " "] {
let body = grant_body(Some(blank)).await;
assert_eq!(body, "client_id=mergify-cli", "for {blank:?}, got {body:?}");
}
});
}

Expand All @@ -869,6 +923,7 @@ mod tests {
api_url: Some(&server.uri()),
store: &store,
browser: None,
device_name: None,
},
&mut captured.output,
)
Expand Down
49 changes: 45 additions & 4 deletions crates/mergify-cli/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ enum NativeCommand {
AuthLogin {
opts: AuthOpts,
no_browser: bool,
device_name: Option<String>,
},
/// `mergify auth logout [--api-url URL]` — revoke the stored
/// credential and forget it.
Expand Down Expand Up @@ -936,9 +937,14 @@ fn dispatch_from_parsed(parsed: CliRoot) -> Dispatch {
Subcommands::Auth(AuthArgs { api_url, command }) => {
let opts = AuthOpts { api_url };
Dispatch::Native(match command {
AuthSubcommand::Login(AuthLoginArgs { no_browser }) => {
NativeCommand::AuthLogin { opts, no_browser }
}
AuthSubcommand::Login(AuthLoginArgs {
no_browser,
device_name,
}) => NativeCommand::AuthLogin {
opts,
no_browser,
device_name,
},
AuthSubcommand::Logout => NativeCommand::AuthLogout(opts),
AuthSubcommand::Status => NativeCommand::AuthStatus(opts),
})
Expand Down Expand Up @@ -1567,7 +1573,11 @@ fn run_native(cmd: NativeCommand) -> ExitCode {
| NativeCommand::InternalManPage => {
unreachable!("introspection commands are handled before the runtime starts")
}
NativeCommand::AuthLogin { opts, no_browser } => {
NativeCommand::AuthLogin {
opts,
no_browser,
device_name,
} => {
let store = mergify_core::CredentialStore::discover();
let system_browser = mergify_auth::browser::SystemBrowser;
let browser: Option<&dyn mergify_auth::browser::Browser> =
Expand All @@ -1577,6 +1587,7 @@ fn run_native(cmd: NativeCommand) -> ExitCode {
api_url: opts.api_url.as_deref(),
store: &store,
browser,
device_name: device_name.as_deref(),
},
&mut output,
)
Expand Down Expand Up @@ -4566,6 +4577,11 @@ struct AuthLoginArgs {
/// Do not open a browser; only print the URL to open.
#[arg(long = "no-browser")]
no_browser: bool,

/// Name this machine on the approval page instead of sending its
/// hostname. `--device-name=` sends no name at all.
#[arg(long = "device-name", value_name = "NAME")]
device_name: Option<String>,
}

#[derive(Subcommand)]
Expand Down Expand Up @@ -5076,6 +5092,31 @@ mod tests {
assert!(!no_browser, "a browser is the default");
}

// `--device-name ""` is the opt-out, so clap must hand the empty
// string through rather than reject it or read it as absent:
// absent means "send the hostname", the opposite of what was asked.
#[test]
fn auth_login_carries_device_name_through_dispatch() {
for (args, expected) in [
(
&["auth", "login", "--device-name", "work laptop"][..],
Some("work laptop"),
),
(&["auth", "login", "--device-name", ""][..], Some("")),
// The portable spelling: Windows PowerShell 5.1 drops a
// `""` argument before it reaches the binary.
(&["auth", "login", "--device-name="][..], Some("")),
(&["auth", "login"][..], None),
] {
let Dispatch::Native(NativeCommand::AuthLogin { device_name, .. }) =
dispatch_from_parsed(parse(args))
else {
panic!("auth login must dispatch to the native AuthLogin variant");
};
assert_eq!(device_name.as_deref(), expected, "for {args:?}");
}
}

#[test]
fn events_dispatches_natively_with_every_flag() {
let parsed = parse(&[
Expand Down
18 changes: 18 additions & 0 deletions crates/mergify-cli/src/snapshots/mergify__tests__cli_schema_golden.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,24 @@ expression: schema
"short": null,
"valueHint": null,
"valueNames": []
},
{
"default": null,
"env": null,
"global": false,
"help": "Name this machine on the approval page instead of sending its hostname. `--device-name=` sends no name at all",
"id": "device_name",
"kind": "option",
"long": "device-name",
"longHelp": "Name this machine on the approval page instead of sending its hostname. `--device-name=` sends no name at all",
"numArgs": "1",
"possibleValues": [],
"required": false,
"short": null,
"valueHint": null,
"valueNames": [
"NAME"
]
}
],
"commands": [],
Expand Down
Loading