Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,19 @@ from KGoose for the workstation owner and injects it on the wire. Any other
URL, including an explicit `--base-url` outside that origin, must be an
approved HTTPS ingress and requires `bl auth login`.

## Apps access errors

A valid CLI session does not guarantee Apps access. For an empty HTTP 403,
the CLI uses the account information already verified by `/auth/me` to provide
`account_setup_required` when setup is incomplete, or `builderlab_access_required`
when BuilderLab product access is absent. Follow `next_action` using the same
account and environment. Repeating login does not finish account setup.

Other permission denials retain their server code and recovery guidance. A 403
uses exit code 4; an expired session (HTTP 401) uses exit code 3 and login guidance.
Account information only adds recovery guidance after a denial; the server
continues to decide access.

## Apps validation errors

Apps HTTP failures include `error.details.http_status` in JSON output and retain
Expand Down
2 changes: 2 additions & 0 deletions crates/builderlab-auth/src/auth_login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ pub struct AuthMeResponse {
pub name: Option<String>,
pub expires_at: Option<String>,
pub workspaces: AuthMeWorkspaces,
pub account_status: Option<String>,
pub enabled_products: Option<Vec<String>>,
}

impl AuthMeResponse {
Expand Down
273 changes: 270 additions & 3 deletions src/bl/apps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ use std::path::{Path, PathBuf};
use std::time::Duration;

use anyhow::{Context, Result};
use builderlab_auth::auth_login::auth_url;
#[cfg(test)]
use builderlab_auth::auth_login::build_auth_http_client;
use builderlab_auth::auth_login::{auth_url, AuthMeResponse};
use builderlab_auth::auth_storage::StoredSessionCredential;
use clap::{Arg, ArgGroup, ArgMatches, Command};
use reqwest::blocking::{multipart, Client, Request, RequestBuilder, Response};
Expand Down Expand Up @@ -1086,6 +1086,7 @@ enum ComposeSessionCredential {
Session {
authorization: HeaderValue,
secret: String,
access_recovery: Option<AppsAccessRecovery>,
},
Proxy,
}
Expand All @@ -1098,7 +1099,15 @@ impl ComposeSessionCredential {
let storage = default_session_storage(config)?;
let verified =
verify_stored_session(config, storage.as_ref())?.ok_or_else(auth_required_error)?;
Self::from_stored(verified.credential)
let mut credential = Self::from_stored(verified.credential)?;
if let Self::Session {
access_recovery, ..
} = &mut credential
{
*access_recovery =
AppsAccessRecovery::from_auth_me(&verified.me, &config.kgoose_base_url);
}
Ok(credential)
}

fn from_stored(credential: StoredSessionCredential) -> Result<Self> {
Expand All @@ -1115,6 +1124,7 @@ impl ComposeSessionCredential {
Ok(Self::Session {
authorization,
secret,
access_recovery: None,
})
}

Expand All @@ -1131,6 +1141,66 @@ impl ComposeSessionCredential {
Self::Proxy => value.to_string(),
}
}

fn access_recovery(&self) -> Option<&AppsAccessRecovery> {
match self {
Self::Session {
access_recovery, ..
} => access_recovery.as_ref(),
Self::Proxy => None,
}
}
}

struct AppsAccessRecovery {
code: &'static str,
next_action: String,
}

impl AppsAccessRecovery {
fn from_auth_me(me: &AuthMeResponse, base_url: &str) -> Option<Self> {
// Use only the coarse, caller-visible status. Pending or unknown status
// must not be diagnosed from legacy screening fields or product absence.
if me.account_status.as_deref() == Some("ACCOUNT_STATUS_NEEDS_DETAILS") {
let destination = account_setup_url(base_url)
.unwrap_or_else(|| "the BuilderLab web app for this environment".to_string());
return Some(Self {
code: "account_setup_required",
next_action: format!(
"Your CLI session is valid, but account setup is incomplete. Finish account setup at {destination} using the same account, then retry this command. Repeating `bl auth login` will not complete account setup."
),
});
}
if matches!(
me.account_status.as_deref(),
None | Some("ACCOUNT_STATUS_OK")
) && me.enabled_products.as_ref().is_some_and(|products| {
!products
.iter()
.any(|product| product == "BUILDERLAB_PRODUCT_BUILDERLAB")
}) {
return Some(Self {
code: "builderlab_access_required",
next_action: "Your CLI session is valid, but BuilderLab product access is not enabled for this account. Ask your administrator or the BuilderLab team to confirm Apps access for this account and environment, then retry this command.".to_string(),
});
}
None
}
}

fn account_setup_url(base_url: &str) -> Option<String> {
let url = url::Url::parse(base_url).ok()?;
// These app hosts serve the supported /account/setup web route. Do not
// invent a web route on direct API hosts, proxies, or custom deployments.
match url.origin().ascii_serialization().as_str() {
"https://app.builderlab.xyz" => {
Some("https://app.builderlab.xyz/account/setup".to_string())
}
"https://app.builderlabstaging.xyz" => {
Some("https://app.builderlabstaging.xyz/account/setup".to_string())
}
_ => None,
}
}

struct ControlPlaneClient {
Expand Down Expand Up @@ -2021,12 +2091,20 @@ fn control_plane_http_failure(
body: &str,
credential: &ComposeSessionCredential,
) -> anyhow::Error {
// The ingress intentionally returns an empty denial. Enrich only that
// response using the already verified session; preserve explicit API errors
// (including workspace/publisher denials) and never retry a mutation.
let access_recovery = (status == StatusCode::FORBIDDEN && body.trim().is_empty())
.then(|| credential.access_recovery())
.flatten();
let parsed = serde_json::from_str::<Value>(body).ok();
let code = parsed
.as_ref()
.and_then(|value| value.pointer("/error/code"))
.and_then(Value::as_str)
.unwrap_or("control_plane_request_failed");
.unwrap_or_else(|| {
access_recovery.map_or("control_plane_request_failed", |recovery| recovery.code)
});
let validation = manifest_validation_diagnostic(status, code, parsed.as_ref());
let code = terminal_safe_text(&credential.redact(code));
let mut details = json!({"http_status": status.as_u16()});
Expand All @@ -2049,6 +2127,10 @@ fn control_plane_http_failure(
Some(json!(
"Run `bl auth logout`, then `bl auth login` to replace your session."
))
} else if let Some(recovery) = access_recovery {
Some(json!(terminal_safe_text(
&credential.redact(&recovery.next_action)
)))
} else {
parsed
.as_ref()
Expand All @@ -2058,6 +2140,9 @@ fn control_plane_http_failure(
.or_else(|| value.pointer("/error/next_action"))
})
.and_then(|value| sanitize_recovery_field(value, credential))
.or_else(|| (status == StatusCode::FORBIDDEN).then(|| json!(
"Check Apps access for this account and environment, and your workspace and app permissions. Use `bl workspace list` and `bl workspace switch` to check the active workspace."
)))
};
let mut message = format!("{method} {path} failed with {status} ({code})");
if let Some((diagnostic, guidance)) = validation {
Expand Down Expand Up @@ -2560,6 +2645,188 @@ mod tests {
assert_eq!(requests[0].body, Value::Null);
}

#[test]
fn bl_apps_process_distinguishes_account_setup_from_other_denials() {
let credential = "apps-e2e-only.account-access.session";
let needs_details = json!({
"account_status": "ACCOUNT_STATUS_NEEDS_DETAILS", "enabled_products": []
});
let cases = [
(
needs_details.clone(),
403,
"",
"account_setup_required",
"Finish account setup",
),
(
json!({"account_status": "ACCOUNT_STATUS_OK", "enabled_products": []}),
403,
"",
"builderlab_access_required",
"product access is not enabled",
),
(
json!({"enabled_products": []}),
403,
"",
"builderlab_access_required",
"product access is not enabled",
),
(
json!({"account_status": "ACCOUNT_STATUS_PENDING", "enabled_products": []}),
403,
"",
"control_plane_request_failed",
"workspace and app permissions",
),
(
json!({"account_status": "ACCOUNT_STATUS_REJECTED", "enabled_products": []}),
403,
"",
"control_plane_request_failed",
"workspace and app permissions",
),
(
json!({"account_status": "ACCOUNT_STATUS_FUTURE", "enabled_products": []}),
403,
"",
"control_plane_request_failed",
"workspace and app permissions",
),
(
json!({}),
403,
"",
"control_plane_request_failed",
"workspace and app permissions",
),
(
needs_details.clone(),
403,
r#"{"error":{"code":"owner_required"},"next_action":"Ask the app owner for access."}"#,
"owner_required",
"Ask the app owner",
),
(
needs_details,
401,
r#"{"error":{"code":"session_expired"}}"#,
"session_expired",
"replace your session",
),
];
for (account_fields, status, body, code, guidance) in cases {
for json_output in [false, true] {
let mut auth_body: Value =
serde_json::from_str(&process_auth_response().body).unwrap();
for (key, value) in account_fields.as_object().unwrap() {
auth_body[key] = value.clone();
}
// Legacy detail/reason fields and identity must never be copied
// into recovery text, including for pending and unknown states.
auth_body["account_standing"] = json!({"restriction_reason": "private-diagnostic"});
let auth_server = ProcessServer::start(vec![ProcessResponse::json(auth_body)]);
let control_plane = ProcessServer::start(vec![
ProcessResponse::raw(status, body).with_trace_id(REQUEST_TRACE_ID)
]);
let mut args = vec!["apps", "list", "--base-url", APPROVED_TEST_BASE_URL];
if json_output {
args.push("--json");
}
let output = process_command(&auth_server, &control_plane, &args, credential)
.output()
.expect("run account access denial");
let expected_exit = if status == 401 {
exit_codes::AUTH_REQUIRED
} else {
exit_codes::FORBIDDEN
};
assert_eq!(output.status.code(), Some(expected_exit));
let stderr = String::from_utf8(output.stderr).unwrap();
assert!(stderr.contains(guidance), "{stderr}");
assert!(stderr.contains(REQUEST_TRACE_ID), "{stderr}");
assert!(!stderr.contains(credential));
assert!(!stderr.contains("private-diagnostic"));
assert!(!stderr.contains("[email protected]"));
if code != "account_setup_required" {
assert!(!stderr.contains("Finish account setup"));
}
if json_output {
let payload = stderr
.lines()
.find_map(|line| serde_json::from_str::<Value>(line).ok())
.unwrap();
assert_eq!(payload["error"]["code"], code);
assert_eq!(payload["error"]["details"], json!({"http_status": status}));
assert_eq!(payload["error"]["trace_id"], REQUEST_TRACE_ID);
assert!(payload["next_action"].as_str().unwrap().contains(guidance));
}
let auth_requests = auth_server.finish();
assert_eq!(auth_requests.len(), 1);
assert_process_auth(&auth_requests[0], credential);
let requests = control_plane.finish();
assert_eq!(requests.len(), 1);
assert_process_control_plane(&requests[0], "GET", "/v1/agent/apps", credential);
}
}
}

#[test]
fn account_setup_links_use_only_known_web_origins() {
for host in ["app.builderlab.xyz", "app.builderlabstaging.xyz"] {
assert_eq!(
account_setup_url(&format!("https://{host}/api/goose")),
Some(format!("https://{host}/account/setup"))
);
}
for base_url in [
"https://kgoose.example.com",
"http://app.builderlab.xyz",
"https://app.builderlab.xyz:8443",
"https://app.builderlab.xyz.example.com",
"http://127.0.0.1:1234",
"invalid",
] {
assert!(account_setup_url(base_url).is_none());
}
}

#[test]
fn account_status_does_not_override_successful_apps_responses() {
let mut auth_body: Value = serde_json::from_str(&process_auth_response().body).unwrap();
auth_body["account_status"] = json!("ACCOUNT_STATUS_NEEDS_DETAILS");
auth_body["enabled_products"] = json!([]);
let auth_server = ProcessServer::start(vec![ProcessResponse::json(auth_body)]);
let inventory = json!({"apps": []});
let control_plane = ProcessServer::start(vec![ProcessResponse::json(inventory.clone())]);
let output = process_command(
&auth_server,
&control_plane,
&[
"apps",
"list",
"--base-url",
APPROVED_TEST_BASE_URL,
"--json",
],
"apps-e2e-only.advisory.session",
)
.output()
.unwrap();
assert!(
output.status.success(),
"{}",
String::from_utf8_lossy(&output.stderr)
);
assert_eq!(
serde_json::from_str::<Value>(&process_stdout(&output)).unwrap(),
inventory
);
assert_eq!(auth_server.finish().len(), 1);
assert_eq!(control_plane.finish().len(), 1);
}

#[test]
fn bl_apps_owned_list_process_preserves_quota_without_deriving_usage() {
let credential = "apps-e2e-only.quota-list.session+credential";
Expand Down
4 changes: 4 additions & 0 deletions src/bl/auth_login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,8 @@ mod tests {
email: None,
name: None,
expires_at: None,
account_status: None,
enabled_products: None,
workspaces: AuthMeWorkspaces {
active: vec![AuthMeWorkspace {
name: "Test Workspace".to_string(),
Expand Down Expand Up @@ -641,6 +643,8 @@ mod tests {
email: None,
name: None,
expires_at: None,
account_status: None,
enabled_products: None,
workspaces: AuthMeWorkspaces { active: vec![] },
},
};
Expand Down
Loading