Skip to content

Commit 1038e25

Browse files
eddietejedaclaude
andauthored
feat(auth): require explicit login subcommand (#182)
* feat(auth): require explicit `login` subcommand instead of bare-`auth` shortcut Bare `hotdata auth` previously triggered an automatic browser login. Now it prints the `auth` help (listing login/register/logout/status), matching how other commands behave when invoked with no subcommand. Users must type `hotdata auth login` to authenticate. Also exempt bare `auth` from the update gate since it no longer hits the API. * docs(changelog): note auth login subcommand requirement (#182) * feat(auth): add `profiles` subcommand to scaffold profiles.yml `hotdata auth profiles` writes ~/.hotdata/profiles.yml seeded with a `default` profile pointing at the production API and app URLs: profiles: default: HOTDATA_API_URL: https://api.hotdata.dev/v1 HOTDATA_APP_URL: https://app.hotdata.dev Refuses to overwrite an existing file. Local-only, so it is exempt from the API update gate. * Revert "feat(auth): add `profiles` subcommand to scaffold profiles.yml" The profiles.yml duplicated existing behavior: api_url/app_url are already overridable via the HOTDATA_API_URL / HOTDATA_APP_URL env vars, and the scaffolded file restated the built-in defaults without anything reading it. * docs(auth): drop stale bare-`auth` login references after subcommand change Bare `hotdata auth` now prints help instead of logging in, so the runtime hints, bundled docs/skills, and internal comments that told users `hotdata auth` (no subcommand) authenticates were misleading. Point them all at `hotdata auth login`, and tighten two tests that asserted the weaker substring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(auth): cover bare `auth` printing help instead of logging in Adds an integration test for the new no-subcommand arm (the 0%-patch-coverage lines Codecov flagged): `hotdata auth` exits 0 with the clap help block listing its subcommands and does NOT start the browser login flow. Runs offline and without credentials. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style: rustfmt the auth-hint and test changes Collapse the now-shorter hint strings and sort the test import — fixes the fmt CI check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Eddie A Tejeda <669988+eddietejeda@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 468f093 commit 1038e25

9 files changed

Lines changed: 79 additions & 27 deletions

File tree

‎README.md‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,13 @@ cp target/release/hotdata /usr/local/bin/hotdata
3333

3434
## Connect
3535

36-
Run either of the following (they are equivalent):
36+
Run:
3737

3838
```sh
3939
hotdata auth login
40-
# or
41-
hotdata auth
4240
```
4341

44-
This launches a browser window where you can authorize the CLI to access your Hotdata account.
42+
This launches a browser window where you can authorize the CLI to access your Hotdata account. (Bare `hotdata auth` prints the `auth` subcommand help.)
4543

4644
Alternatively, authenticate with an API key using the `--api-key` flag:
4745

@@ -62,7 +60,7 @@ API key priority (lowest to highest): config file → `HOTDATA_API_KEY` env var
6260

6361
| Command | Subcommands | Description |
6462
| :-- | :-- | :-- |
65-
| `auth` | `login`, `status`, `logout` | `login` or bare `auth` opens browser login; `status` / `logout` manage the saved profile |
63+
| `auth` | `login`, `status`, `logout` | `login` opens browser login; `status` / `logout` manage the saved profile |
6664
| `workspaces` | `list`, `set` | Manage workspaces |
6765
| `connections` | `list`, `create`, `refresh`, `new` | Manage connections |
6866
| `databases` | `list`, `create`, `delete`, `tables` | Managed databases (create and load tables via parquet) |

‎skills/hotdata/SKILL.md‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@ Install all skills with **`hotdata skills install`**. Load specialized skills on
2727

2828
## Authentication
2929

30-
Run **`hotdata auth login`** (or **`hotdata auth`** with no subcommand—same behavior) to authenticate via browser login. Config is stored in `~/.hotdata/config.yml`.
30+
Run **`hotdata auth login`** to authenticate via browser login. Config is stored in `~/.hotdata/config.yml`.
3131

3232
API key resolution (lowest to highest priority):
33-
1. Config file (saved by `hotdata auth login` / `hotdata auth`)
33+
1. Config file (saved by `hotdata auth login`)
3434
2. `HOTDATA_API_KEY` environment variable (or `.env` file)
3535
3. `--api-key <key>` flag (works on any command)
3636

@@ -336,8 +336,7 @@ A newer release can be incompatible with the API, so in an **interactive termina
336336

337337
### Auth
338338
```
339-
hotdata auth login # Browser-based login (same as: hotdata auth)
340-
hotdata auth # Browser-based login (same as: hotdata auth login)
339+
hotdata auth login # Browser-based login
341340
hotdata auth status # Check current auth status
342341
hotdata auth logout # Remove saved auth for the default profile
343342
```

‎skills/hotdata/references/WORKFLOWS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ End-to-end checklists. Use the linked sections for command detail and guardrails
3434

3535
**Skill:** **`hotdata`** (optional **`hotdata-analytics`** for first queries)
3636

37-
1. [ ] `hotdata auth login` (or `hotdata auth`)
37+
1. [ ] `hotdata auth login`
3838
2. [ ] `hotdata workspaces list` → `hotdata workspaces set` if not on the right workspace
3939
3. [ ] `hotdata connections list` — note connection ids and names
4040
4. [ ] (Optional) `hotdata connections create …` — see **`hotdata`** skill **Create a Connection**

‎src/command.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ pub enum QueryCommands {
263263

264264
#[derive(Subcommand)]
265265
pub enum AuthCommands {
266-
/// Log in via browser (same as `hotdata auth` with no subcommand)
266+
/// Log in via browser
267267
Login,
268268

269269
/// Create a new account via browser (defaults to GitHub OAuth)

‎src/config.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ pub fn resolve_workspace_id(
264264
.workspaces
265265
.first()
266266
.map(|w| w.public_id.clone())
267-
.ok_or_else(|| "no workspace-id provided and no default workspace found. Run 'hotdata auth login' (or 'hotdata auth') or specify --workspace-id.".to_string())
267+
.ok_or_else(|| "no workspace-id provided and no default workspace found. Run 'hotdata auth login' or specify --workspace-id.".to_string())
268268
}
269269

270270
/// Global API key override set via --api-key flag.
@@ -283,9 +283,7 @@ pub fn load(profile: &str) -> Result<ProfileConfig, String> {
283283
.map_err(|e| format!("error reading config file: {e}"))?;
284284
let config_file: ConfigFile = serde_yaml::from_str(&content).unwrap_or_else(|_| {
285285
eprintln!("{}", "error parsing config file.".red());
286-
eprintln!(
287-
"Run 'hotdata auth login' (or 'hotdata auth') to generate a new config file."
288-
);
286+
eprintln!("Run 'hotdata auth login' to generate a new config file.");
289287
std::process::exit(1);
290288
});
291289
config_file

‎src/jwt.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
//! | Access token valid for > 30 s | return it directly |
1313
//! | Access expiring or expired, refresh token valid | call `/o/token/` with `grant_type=refresh_token` |
1414
//! | Refresh token dead, `api_key` present | re-mint via `grant_type=api_token` |
15-
//! | Refresh token dead, no `api_key` | return an error — user must `hotdata auth` again |
15+
//! | Refresh token dead, no `api_key` | return an error — user must `hotdata auth login` again |
1616
//!
1717
//! The raw `hd_...` API token (flow 3 in the design doc) is *never*
1818
//! persisted to the session file — it stays in the main config or the
@@ -302,7 +302,7 @@ pub fn ensure_access_token(
302302
// 0) An explicit identity override (`--api-key`, `HOTDATA_API_KEY`,
303303
// or `.env`) is asserting a specific identity for *this invocation*.
304304
// The on-disk session may belong to a completely different user
305-
// from a prior `hotdata auth` and must not be reused. Mint fresh
305+
// from a prior `hotdata auth login` and must not be reused. Mint fresh
306306
// and deliberately skip persisting so we don't clobber the
307307
// interactive session. Surface the real mint error here too — if
308308
// the override key is bad, "HTTP 401" is more useful than the
@@ -358,7 +358,7 @@ pub fn ensure_access_token(
358358
// API token rejected (revoked, expired, or invalid).
359359
// Fall through to the re-auth hint — hide the raw HTTP
360360
// error from the user; the api.rs caller appends a
361-
// `hotdata auth` hint.
361+
// `hotdata auth login` hint.
362362
}
363363
}
364364
}
@@ -435,7 +435,7 @@ impl hotdata::auth::BearerTokenProvider for CliTokenProvider {
435435
resolved.map_err(|body| {
436436
// Surface as a 401 so `Configuration::resolve_bearer_token` logs the
437437
// cause and the request proceeds to a 401 the wrapper shapes into
438-
// the "run hotdata auth" hint (the same end-state as the old
438+
// the "run hotdata auth login" hint (the same end-state as the old
439439
// ApiClient refresher returning None).
440440
hotdata::auth::TokenExchangeError::Status { status: 401, body }
441441
})
@@ -1235,7 +1235,7 @@ mod tests {
12351235
let (_tmp, _guard) = with_temp_config_dir();
12361236
// No session, no api_key fallback -> ensure_access_token errors, the
12371237
// provider maps it to a 401 so the request proceeds to the wrapper's
1238-
// "run hotdata auth" hint.
1238+
// "run hotdata auth login" hint.
12391239
let profile = mock_profile("http://127.0.0.1:1");
12401240
let provider = session_provider(&profile, None);
12411241
match bearer(&provider).unwrap_err() {

‎src/main.rs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,9 @@ fn main() {
177177
// never blocked (see `update::should_check`).
178178
let gate_update = !matches!(
179179
&cli.command,
180-
None | Some(Commands::Upgrade) | Some(Commands::Completions { .. })
180+
None | Some(Commands::Upgrade)
181+
| Some(Commands::Completions { .. })
182+
| Some(Commands::Auth { command: None })
181183
);
182184
if gate_update {
183185
update::enforce_latest_or_exit();
@@ -191,10 +193,19 @@ fn main() {
191193
}
192194
Some(cmd) => match cmd {
193195
Commands::Auth { command } => match command {
194-
None | Some(AuthCommands::Login) => auth::login(),
196+
Some(AuthCommands::Login) => auth::login(),
195197
Some(AuthCommands::Register { email }) => auth::register(email),
196198
Some(AuthCommands::Status) => auth::status("default"),
197199
Some(AuthCommands::Logout) => auth::logout("default"),
200+
None => {
201+
use clap::CommandFactory;
202+
let mut cmd = Cli::command();
203+
cmd.build();
204+
cmd.find_subcommand_mut("auth")
205+
.unwrap()
206+
.print_help()
207+
.unwrap();
208+
}
198209
},
199210
Commands::Query {
200211
sql,

‎src/sdk.rs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ impl ApiError {
249249
/// `ApiClient::fail_response`'s formatting.
250250
///
251251
/// On a 4xx, re-probe the auth status so a masked 404/403 is upgraded into
252-
/// the "run hotdata auth" hint; otherwise surface the server body. Split out
252+
/// the "run hotdata auth login" hint; otherwise surface the server body. Split out
253253
/// from [`exit`](Self::exit) so callers that want to append their own hint
254254
/// after the error (e.g. the query cross-source hint) can print, add the
255255
/// hint, then exit.
@@ -483,7 +483,7 @@ impl Api {
483483
eprintln!("{}", format!("error: {e}").red());
484484
eprintln!(
485485
"Run {} to log in, or pass --api-key.",
486-
"hotdata auth".cyan()
486+
"hotdata auth login".cyan()
487487
);
488488
std::process::exit(1);
489489
}
@@ -840,7 +840,8 @@ pub fn format_fail_message(
840840
if status.is_client_error()
841841
&& let Some(auth::AuthStatus::Invalid(_)) = auth_status
842842
{
843-
return "error: API key is invalid. Run 'hotdata auth login' (or 'hotdata auth') to re-authenticate.".to_string();
843+
return "error: API key is invalid. Run 'hotdata auth login' to re-authenticate."
844+
.to_string();
844845
}
845846
// A 403 ACCESS_DENIED is the allow-list guard rejecting an operation the
846847
// credential can't perform — typically a database API token (which is
@@ -886,7 +887,7 @@ mod tests {
886887
Some(&AuthStatus::Invalid(401)),
887888
);
888889
assert!(msg.contains("API key is invalid"));
889-
assert!(msg.contains("hotdata auth login") || msg.contains("hotdata auth"));
890+
assert!(msg.contains("hotdata auth login"));
890891
}
891892

892893
#[test]
@@ -962,7 +963,7 @@ mod tests {
962963
msg.to_lowercase().contains("database api token"),
963964
"got: {msg}"
964965
);
965-
assert!(msg.contains("hotdata auth"), "got: {msg}");
966+
assert!(msg.contains("hotdata auth login"), "got: {msg}");
966967
}
967968

968969
#[test]

‎tests/auth_no_subcommand_help.rs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
//! `hotdata auth` with no subcommand prints the `auth` help (listing its
2+
//! subcommands) instead of triggering a browser login — the behavior change
3+
//! from PR #182.
4+
//!
5+
//! Runs fully offline and without credentials: bare `auth` is exempt from the
6+
//! update gate and hits no API. Before the change this arm called
7+
//! `auth::login()`, which prints "Opening browser to log in..." and starts a
8+
//! local callback server; the test asserts that flow is *not* started.
9+
10+
mod common;
11+
12+
use common::{DEFAULT_API_URL, unauthenticated_output};
13+
14+
#[test]
15+
fn bare_auth_prints_subcommand_help() {
16+
let output = unauthenticated_output(DEFAULT_API_URL, &["auth"]);
17+
18+
assert!(
19+
output.status.success(),
20+
"`hotdata auth` should exit 0 printing help\n--- stderr ---\n{}",
21+
String::from_utf8_lossy(&output.stderr),
22+
);
23+
24+
let stdout = String::from_utf8_lossy(&output.stdout);
25+
26+
// Clap help block, listing every auth subcommand.
27+
assert!(
28+
stdout.contains("Usage"),
29+
"expected help output with a usage line; got:\n{stdout}"
30+
);
31+
for sub in ["login", "register", "status", "logout"] {
32+
assert!(
33+
stdout.contains(sub),
34+
"auth help missing `{sub}` subcommand; got:\n{stdout}"
35+
);
36+
}
37+
38+
// The login flow must NOT have started. `auth::login()` prints this banner
39+
// before opening a browser / spinning up the callback server.
40+
let combined = format!("{stdout}{}", String::from_utf8_lossy(&output.stderr));
41+
assert!(
42+
!combined.contains("Opening browser to log in"),
43+
"bare `auth` should print help, not start a login flow; got:\n{combined}"
44+
);
45+
}

0 commit comments

Comments
 (0)