feat(connect): Tier 2 bring-your-own OAuth app profiles (#146)#147
Merged
Conversation
Let an org point `aware connect` at its own registered Google / M365 / Trimble OAuth app (its tenant, client_id, secret, scopes) without env vars, overriding the bundled first-party (Tier 1) app. - profile.rs: OAuthAppProfile loaded from ~/.aware/oauth/<int>[.<alias>].yaml (alias-shadowing, UTF-8 BOM tolerant for Windows-edited files) - keychain: store/load/delete_app_secret under a dedicated oauth-app.<int> slot (file fallback); BYO secret never written to plaintext config - config: with_profile overlay; resolution precedence profile > env > bundled; pure resolve_client_secret (a BYO client_id suppresses the bundled secret) - PKCE / refresh / device-code all consume the resolved config; device-code honors the profile tenant when --tenant is absent - connect: `--set-app-secret` reads the secret from stdin into the keychain; `--list` / `doctor` report the active app (first-party | byo-profile | byo-env) - docs: Tier 2 org self-registration runbook in oauth-registration.md
…eview) load_app_secret treated a keyring-unavailable backend (headless Linux/CI with no libsecret/dbus) as a hard error. Because with_profile reads the app-secret slot on every connect path, this aborted normal first-party flows — even --from-file — before the token file-fallback. A BYO app secret is optional config, so any keyring error now falls through to the file fallback and yields None when absent. Found by Codex review.
…point overrides (#146 review) Two P2s from Codex review: - client_secret resolution gated on an active BYO client (profile/env id). A keychain app secret left over from a removed BYO profile no longer pairs with the first-party client id; first-party now resolves env > bundled and ignores the stale keychain secret. BYO resolves keychain > env, never bundled. - device-code now honors an explicit profile token_url / device_authorization_url override (sovereign cloud / proxy) instead of rebuilding the public-cloud tenant URL; falls back to tenant-built URLs (still honoring CLI --tenant) when no override is set. New optional profile field device_authorization_url + docs.
…s lines (#146 review) Two more from Codex review: - AppSource is now derived from client_id ownership, not mere profile presence. A profile that only customizes scopes/tenant/endpoints (no client_id) stays first-party, so its bundled secret is no longer wrongly suppressed in release builds. BYO (profile/env client_id) still suppresses the bundled secret. - `aware connect --list` text output now shows [app: ...] on missing, expired, and keyring-unavailable lines too, so a BYO app can be verified before the first connect.
…ides, isolate keychain in tests (#146 review) Round 3 of Codex review: - connect: with_profile is now loaded only on the --oauth / --device-code paths, so a malformed BYO profile no longer blocks token-import paths (--from-file, --from-env, paste) that don't use OAuth config. - device-code: profile token_url / device_authorization_url overrides are now honored for all providers (Google proxy/alternate endpoints), not just M365. - keychain: add AWARE_DISABLE_KEYRING (and cfg!(test) default) to force the credentials-file fallback. The OS keychain is global and not scoped by AWARE_HOME, so this is needed for test isolation (integration tests no longer pollute the real keychain) and is useful on headless/locked-down machines. - tests: new cli/tests/connect_byo.rs covering the malformed-profile import regression, --set-app-secret stdin, and --list app-source reporting.
load_profile falls back from <int>.<alias>.yaml to <int>.yaml, but load_app_secret only checked the alias-specific keychain slot. An aliased account inheriting the default profile's client_id therefore got no secret, breaking OAuth/refresh unless the secret was duplicated per alias. Now the secret lookup mirrors the profile fallback: alias-specific slot, then default.
…back (#146 review) The previous alias→default secret fallback was unconditional, which would pair an alias's own client_id with the default app's secret. Now the secret slot mirrors the resolved profile scope: with_profile reads the alias secret slot only when an alias-specific profile file exists, else the default slot (matching load_profile's alias→default fallback). load_app_secret is back to an exact-account lookup; the policy lives in config::with_profile via profile::alias_profile_exists.
…#146 review) - The keychain oauth-app.<int> secret slot belongs to a profile-based BYO app, so it is now loaded only for ByoProfile. Env-sourced (ByoEnv) apps no longer pick up a stale keychain secret over their AWARE_OAUTH_<INT>_CLIENT_SECRET; the resolution stays keychain(None) > env for them. - New with_tenant() applies a CLI --tenant override (M365) with precedence over a profile tenant, matching the device-code path; connect --oauth now applies it before PKCE builds the auth/token URLs (unless the profile set explicit sovereign-cloud endpoints).
…review) --set-app-secret now uses the same alias/default scope decision as with_profile: the alias slot only when an alias-specific profile exists, else the default slot. Previously `--as personal --set-app-secret` with only a default profile wrote an orphaned alias slot the OAuth flow never reads, so the subsequent `--as personal --oauth` would fail. Prints a note when an aliased secret is stored for the default app. Covered by a new integration test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements Tier 2 of the hybrid cloud-auth model (#146): orgs can point
aware connectat their own registered Google / M365 / Trimble OAuth app — their tenant, client_id, secret, scopes — as first-class config, overriding the bundled first-party (Tier 1) app. Closes #146.~/.aware/oauth/<integration>[.<alias>].yamlholds non-secret app config (client_id, tenant, scopes, optional endpoint URLs). UTF-8 BOM tolerant.aware connect <int> --set-app-secretreads the client secret from stdin into the OS keychain — never plaintext config, never argv.AppSourceis keyed to client_id ownership (first-party/byo-profile/byo-env).--tenantoverrides a profile tenant for PKCE.aware connect --list/doctorreport the active app per integration.AWARE_DISABLE_KEYRINGforces the credentials-file fallback (headless / locked-down machines; also isolates tests, since the OS keychain is global and not scoped byAWARE_HOME).10-core/oauth-registration.md.Review
Primary review by Codex (
codex exec review --base main) — 7 rounds, every finding addressed, final pass clean. Notable fixes: keychain-unavailable no longer aborts connect; stale/mismatched secrets never paired with the wrong client; app-secret storage/read scopes aligned for aliases; CLI--tenantprecedence in PKCE. Two real bugs caught en route: a Windows BOM footgun in profile YAML, and global keychain pollution from integration tests (now isolated).Test plan
cargo test— 369 unit + newcli/tests/connect_byo.rs(5) + all other integration tests greenrustfmt- andclippy --all-targets -D warnings-clean (pre-existing crate-wide drift in untouched files unchanged; CI builds only)connect --listshowsbyo-profilefor a profile with client_id andfirst-partyotherwise;--set-app-secretstores via stdinv0.43.0to trigger the release (GitHub + npm)Note: the first commit relaxes the CLAUDE.md "never auto-commit" rule to standing commit approval, per repo owner request.