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.31%
=======================================
Files ? 620
Lines ? 40426
Branches ? 8196
=======================================
Hits ? 23575
Misses ? 14896
Partials ? 1955 ☔ 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. |
8fd9577 to
48031bd
Compare
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
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