Skip to content

feat(login): standardize all sign-ins on the device-code flow #995

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

Open
codyde wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(login): standardize all sign-ins on the device-code flow #995

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
53 changes: 18 additions & 35 deletions src/commands/login.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,16 @@ use super::*;

/// Sign in to Railway — also creates a new account if you don't have one.
///
/// Uses a single OAuth flow for both sign-in and sign-up. Brand-new
/// accounts are detected automatically and land on a welcome page;
/// existing users see the standard sign-in confirmation. Use
/// --browserless for SSH sessions, remote dev boxes, or any
/// environment where a local browser can't open.
/// Uses a single OAuth device-code flow for both sign-in and sign-up:
/// the CLI prints a one-click sign-in link (plus a short code) that you
/// complete in a browser on any device. Brand-new accounts are detected
/// automatically and land on a welcome page; existing users see the
/// standard sign-in confirmation.
#[derive(Parser)]
pub struct Args {
/// Use a device-code flow instead of opening a browser. Prints
/// a sign-in link + short code to use from any device. Only
/// needed when this machine truly has no browser — the CLI
/// already auto-detects SSH, CI, and missing DISPLAY. If a human
/// is at this machine (including under a coding agent), omit
/// this flag: the browser flow completes far more reliably.
/// Deprecated: the device-code flow is now always used, so this flag
/// no longer changes behavior. Retained for backward compatibility
/// with scripts that still pass it.
#[clap(short, long)]
pub browserless: bool,
}
Expand Down Expand Up @@ -67,43 +64,29 @@ pub async fn command(args: Args) -> Result<()> {

let host = configs.get_host();

// `login` is an explicit auth request, so we always attempt it —
// only the transport varies. Device-code when the user asked for it
// (--browserless) or no local browser is reachable (CI/SSH/no
// DISPLAY); otherwise open a browser. browser_login itself falls
// back to device-code if `open` fails. Neither path needs a TTY.
// `login` is an explicit auth request, so we always attempt it.
// Every sign-in now uses the device-code flow (see
// `ExecutionContext::login_transport`); the transport no longer
// varies by environment. Neither path needs a TTY.
let ctx = crate::exec_context::ExecutionContext::detect(false, false);
let transport_choice = ctx.login_transport(args.browserless);

// Why this transport was chosen, for funnel telemetry. Env
// constraints win over the flag: "flag_browserless" is reported
// only when a browser was otherwise reachable, so it counts exactly
// the sessions a bare `railway login` would have sent down the
// (far more reliable) browser path. browser_login overwrites both
// labels if its `open` attempt fails and it falls back.
// Why this transport was chosen, for funnel telemetry. Device-code
// is now the default for every sign-in ("default_device_code"); the
// environment labels (env_ci/env_ssh/no_display) and the explicit
// "flag_browserless" are still recorded so we can see how sessions
// would have been routed under the old browser-preferring logic.
let mut reason = match transport_choice {
crate::exec_context::AuthTransport::Browser => "browser",
crate::exec_context::AuthTransport::DeviceCode => {
headless_reason().unwrap_or(if args.browserless {
"flag_browserless"
} else {
"unknown"
"default_device_code"
})
}
};

// --browserless on a machine that has a browser: honor the flag,
// but say so in the output. Agents (the dominant source of this
// combination) read command output at exactly this moment, and the
// browser path completes far more often for watched sessions.
if reason == "flag_browserless" {
println!(
" {} A browser is available on this machine — `railway login` without {} opens it directly and completes more reliably.",
"→".cyan(),
"--browserless".bold(),
);
}

let mut transport = match transport_choice {
crate::exec_context::AuthTransport::DeviceCode => "device_code",
crate::exec_context::AuthTransport::Browser => "browser",
Expand Down
70 changes: 43 additions & 27 deletions src/exec_context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ use is_terminal::IsTerminal;
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum AuthTransport {
/// Open a local browser (authorization-code + PKCE).
///
/// Retained but no longer constructed: sign-in is standardized on
/// `DeviceCode` (see [`ExecutionContext::login_transport`]). The
/// browser flow in `commands::login` and its match arms are kept as
/// runtime-unreachable code so the policy can be reverted by
/// restoring `login_transport`'s body. `allow(dead_code)` covers the
/// now-unconstructed variant.
#[allow(dead_code)]
Browser,
/// Print a verification URL + device code (RFC 8628) for a human to
/// use on any device.
Expand Down Expand Up @@ -91,22 +99,29 @@ impl ExecutionContext {
self.agent_harness && !self.stdin_tty
}

/// Transport for an *explicit* sign-in (`railway login`). The user
/// asked to authenticate, so we always attempt it; only the
/// transport varies.
pub fn login_transport(&self, browserless: bool) -> AuthTransport {
if browserless || !self.browser_reachable {
AuthTransport::DeviceCode
} else {
AuthTransport::Browser
}
/// Transport for a sign-in (`railway login`, or the implicit sign-in
/// from an unauthenticated `railway up`). Standardized on the
/// device-code flow: the CLI prints a one-click sign-in link (plus a
/// short code) that the human completes in a browser on any device.
///
/// Every sign-in now uses one transport regardless of environment.
/// Auto-opening a local browser was the higher-converting path for
/// watched desktop sessions, but a single transport removes the
/// branch (no display detection, no flaky `open()` fallback) and
/// makes behavior identical across desktop, SSH, CI, and agent
/// harnesses. The `browserless` argument is retained for call-site
/// compatibility but no longer changes the choice; the browser
/// machinery in `commands::login` is kept as runtime-unreachable code
/// so this can be reverted by restoring the original body.
pub fn login_transport(&self, _browserless: bool) -> AuthTransport {
AuthTransport::DeviceCode
}

/// Decision for *implicit* sign-in triggered as a side effect (an
/// unauthenticated `railway up`). Fails fast only when there's no
/// human to complete a flow; otherwise proceeds with the appropriate
/// transport — a browser when one is reachable, or a device code
/// (which the human completes on another device) when it isn't.
/// human to complete a flow; otherwise proceeds with the standardized
/// device-code transport (which the human completes in a browser on
/// any device).
pub fn auto_auth(&self, browserless: bool) -> AutoAuth {
// Machine contexts have no human in the loop: JSON output is
// consumed by a tool, and --ci is non-interactive by definition.
Expand All @@ -118,12 +133,11 @@ impl ExecutionContext {
if !self.stdout_tty && !self.agent_implicit_consent() {
return AutoAuth::FailFast;
}
// A human is present. Pick the transport: a browser if one can
// open, otherwise device-code (SSH / no DISPLAY) — the same
// fallback `railway login` uses here, so an unauthenticated `up`
// can sign the user in and deploy in one shot even on a remote
// box. (When there's genuinely no human, we already failed fast
// above rather than print a code into the void.)
// A human is present. Use the standardized device-code transport
// (the same one `railway login` uses), so an unauthenticated `up`
// can sign the user in and deploy in one shot in any environment.
// (When there's genuinely no human, we already failed fast above
// rather than print a code into the void.)
AutoAuth::Proceed(self.login_transport(browserless))
}
}
Expand Down Expand Up @@ -152,12 +166,14 @@ mod tests {
}
}

// --- login_transport: explicit `railway login` always attempts ---
// --- login_transport: every explicit `railway login` uses device-code ---

#[test]
fn login_uses_browser_when_reachable_and_not_browserless() {
fn login_uses_device_code_even_when_browser_reachable() {
// Standardized policy: a desktop with a reachable browser still
// uses device-code, same as every other environment.
let c = ctx(false, false, true, true, false, true);
assert_eq!(c.login_transport(false), AuthTransport::Browser);
assert_eq!(c.login_transport(false), AuthTransport::DeviceCode);
}

#[test]
Expand All @@ -168,8 +184,8 @@ mod tests {

#[test]
fn login_uses_device_code_when_no_browser_reachable() {
// SSH / no-DISPLAY / CI env: browser can't open, but the user
// explicitly asked to log in, so device-code (not fail-fast).
// SSH / no-DISPLAY / CI env: browser can't open, and the
// standardized flow is device-code regardless.
let c = ctx(false, false, true, true, false, false);
assert_eq!(c.login_transport(false), AuthTransport::DeviceCode);
}
Expand All @@ -181,18 +197,18 @@ mod tests {
let c = ctx(false, false, true, true, false, true);
assert_eq!(
c.auto_auth(false),
AutoAuth::Proceed(AuthTransport::Browser)
AutoAuth::Proceed(AuthTransport::DeviceCode)
);
}

#[test]
fn auto_auth_proceeds_under_an_agent_harness_with_piped_stdio() {
// Agent harness with captured stdout/stdin: a human is watching
// and can complete the browser sign-in.
// and can complete the device-code sign-in.
let c = ctx(false, false, false, false, true, true);
assert_eq!(
c.auto_auth(false),
AutoAuth::Proceed(AuthTransport::Browser)
AutoAuth::Proceed(AuthTransport::DeviceCode)
);
}

Expand Down Expand Up @@ -256,7 +272,7 @@ mod tests {
assert!(!c.agent_implicit_consent());
assert_eq!(
c.auto_auth(false),
AutoAuth::Proceed(AuthTransport::Browser)
AutoAuth::Proceed(AuthTransport::DeviceCode)
);
}
}
Loading