Feat/appcheck debugtoken cli#10801
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces Firebase App Check debug token management commands (appcheck:debugtoken, appcheck:debugtoken:create, appcheck:debugtoken:list, and appcheck:debugtoken:delete) along with their corresponding API client methods, CLI command registrations, and unit tests. Feedback focuses on improving robustness and error handling: safely accessing res.body properties via optional chaining in listDebugTokens to prevent potential TypeErrors; explicitly handling non-interactive mode when prompting for a display name in the creation command; failing early in non-interactive mode if --force is omitted during deletion; validating that full resource names belong to the specified app and project; and aborting with a FirebaseError if a user declines to overwrite an existing token with the same display name.
1804817 to
f6b9b82
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #10801 +/- ##
=======================================
Coverage ? 58.33%
=======================================
Files ? 620
Lines ? 40416
Branches ? 8194
=======================================
Hits ? 23577
Misses ? 14881
Partials ? 1958 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
93e4a1e to
ee6de4f
Compare
Introduces CLI support for managing Firebase App Check debug tokens via the App Check REST API (`projects.apps.debugTokens`). Key changes: * api: Added `appCheckOrigin` to `src/api.ts` (`https://firebaseappcheck.googleapis.com`). * appcheck: Created the `src/appcheck/index.ts` API client supporting `create`, `list`, `get`, `update`, and `delete` operations, along with comprehensive unit tests in `index.spec.ts`. * commands: Added a suite of management commands under the `appcheck:debug-tokens` namespace: - `appcheck:debug-tokens:create <appId> [debugToken]`: Creates a debug token (with auto-generated UUIDv4 fallback and interactive display name prompting). - `appcheck:debug-tokens:list <appId>`: Lists all debug tokens for an app in a formatted table. - `appcheck:debug-tokens:get <appId> <debugTokenId>`: Fetches and displays metadata for a specific debug token. - `appcheck:debug-tokens:update <appId> <debugTokenId>`: Updates the display name of a debug token via `patch`. - `appcheck:debug-tokens:delete <appId> <debugTokenId>`: Revokes/deletes a debug token with interactive confirmation support. * commands: Registered the new commands in `src/commands/index.ts`. feat: Add App Check debug token commands
…rebase-debug-token and update .gitignore
…ames in non-interactive mode
…ate.ts and remove old files
ee6de4f to
acd5428
Compare
andrewheard
left a comment
There was a problem hiding this comment.
LGTM. I'd recommend replacing the any with a type like in create and cleaning up get / update if we don't need them but no big concerns on my end.
727ff90 to
8fd9577
Compare
joehan
left a comment
There was a problem hiding this comment.
Left some comments, but also:
- Please move this to a branch on the main repo - go/firebase-github-access will get you access if needed. That will allow the tests to run automatically for you.
- Please add a changelog entry - users need to know about this change!
| /** | ||
| * | ||
| */ | ||
| export async function getOrPromptProjectAndAppId( |
There was a problem hiding this comment.
We shouldn't be prompting for project ID here. The CLI has a well established way of handling project selection (either using the active project system, or the -P flag), and we do not want this command to subvert that.
If a project ID is not specified, the command should jsut error our with a clear message.
| } | ||
|
|
||
| const projectDir = options.cwd || process.cwd(); | ||
| let apps = await listFirebaseApps(projectId, AppPlatform.ANY); |
There was a problem hiding this comment.
As I'm revieiwng this, I'm realizing that there is duplicated logic for selecting an app from the list of exisitng apps in 3 places in the CLI already 🤦 That's an oversight on my part, but we really ought to centralize this . I'm gonna send you a PR shortly to do so - please review and we can merge that before this one.
| @@ -0,0 +1,74 @@ | |||
| import { Options } from "../options"; | |||
There was a problem hiding this comment.
The src/commands directory should only include files that implement and export a command. Please move the types to src/appcheck/types.ts, and the prompt function (if it is still needed after that pr to centralize logic) to src/appcheck/prompts.ts
| @@ -0,0 +1,67 @@ | |||
| import { appCheckOrigin } from "../api"; | |||
There was a problem hiding this comment.
index.ts is a very generic name for a file that tells us nothing. We do it in a few other places in the codebase, but it is a code smell. This file just has API methods and types. Please split it into 2 files: src/appcheck/types.ts and src/appcheck/api.ts
Then, we can also move the AppCheckDebugOptions to the types file too.
I have requested access and will update the change log. |
48031bd to
c203f4d
Compare
| throw new FirebaseError("Must specify an App ID using --app."); | ||
| } | ||
| .action(async (debugTokenId: string, options: AppCheckDebugOptions): Promise<void> => { | ||
| const { appId } = await getOrPromptProjectAndAppId(options); |
There was a problem hiding this comment.
Moe directly - use needProjectID here instead of rolling your own
4425171 to
c203f4d
Compare
c203f4d to
6e6d3ac
Compare
Description
Adding to the Firebase CLI App Check debug token workflows under the unified
appcheck:debugtokensnamespace, aligning directly with the backend REST API (projects.apps.debugTokens).Key changes:
appcheck:debugtokens:createfor both interactive wizard registration and headless/non-interactive script execution in CI/CD automation pipelines.appcheck:debugtokens:listto render registered tokens in tabular format (Display Name,Resource Name,Update Time) with full backend pagination.appcheck:debugtokens:deleteto permanently revoke tokens by ID or resource path, supporting interactive confirmation and--forceflag overrides.Scenarios Tested
appcheck:debugtokens:create): Tested running without--appto verify automatic app detection and interactive choice selection.appcheck:debugtokens:create [debugToken] --app <appId>): Tested supplying--appanddebugTokento confirm prompt skipping.appcheck:debugtokens:create --non-interactive): Verified execution with global--non-interactiveflag using fallback defaults without blocking prompts, and correctly enforcing--forcefor display name collision overwrites.appcheck:debugtokens:list --app <appId>): Verified table rendering of returned tokens and multi-page pagination handling.appcheck:debugtokens:delete <debugTokenId> --app <appId>): Tested interactive deletion confirmation as well as non-interactive--forcedeletion.npm run buildandnpx mocha src/appcheck/index.spec.ts&src/commands/appcheck-debugtokens-create.spec.ts(12/12 unit tests passing).Sample Commands
Interactive wizard: Register a token copied from SDK logs
firebase appcheck:debugtokens:create e3d9b1a0-cf48-4389-8d19-482a177ffec8 --app 1:1234567890:ios:0a1b2c3d4e5f6a7b8c9d0e
Interactive wizard: Auto-detect project & prompt for app selection
firebase appcheck:debugtokens:create
Non-interactive creation for CI/CD pipelines
firebase appcheck:debugtokens:create e3d9b1a0-cf48-4389-8d19-482a177ffec8 --app 1:1234567890:ios:0a1b2c3d4e5f6a7b8c9d0e --display-name "CI Runner Token" --non-interactive
List registered tokens for an app
firebase appcheck:debugtokens:list --app 1:1234567890:ios:0a1b2c3d4e5f6a7b8c9d0e
Delete / revoke a token by ID
firebase appcheck:debugtokens:delete 7890-abcd --app 1:1234567890:ios:0a1b2c3d4e5f6a7b8c9d0e --force