-
Notifications
You must be signed in to change notification settings - Fork 51
CLI: Implement auth login command
#2017
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
| cmd = 'PowerShell'; | ||
| args = [ | ||
| '-NoProfile', | ||
| '-NonInteractive', | ||
| '-ExecutionPolicy', | ||
| 'Bypass', | ||
| '-Command', | ||
| 'Start', | ||
| `"${ url }"`, | ||
| ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have yet to test this. This was taken from Sindre Sorhus's open module.
📊 Performance Test ResultsComparing 0f75d13 vs trunk site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change |
|
Working on a test right now. |
|
Looking at how many changes I had to make today, I see that I opened this PR a little early. In any case, it's good for a full review now 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a CLI authentication command (studio auth login) to allow users to authenticate with WordPress.com directly from the command line. The implementation opens a browser for OAuth authentication and stores the resulting token with user information.
Key changes:
- Adds
@inquirer/promptsdependency for interactive token input - Implements OAuth-based login flow with browser integration
- Enhances auth token storage to include expiration time and user details
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json, package-lock.json | Added @inquirer/prompts dependency for interactive CLI prompts |
| common/logger-actions.ts | Added AuthCommandLoggerAction enum for login/logout tracking |
| common/lib/oauth.ts | Made redirect URI configurable for CLI authentication flow |
| cli/lib/i18n.ts | Extracted getAppLocale() function for reusability |
| cli/lib/browser.ts | New utility to open default browser across platforms |
| cli/lib/appdata.ts | Enhanced auth token schema with expiration, email, and display name |
| cli/lib/api.ts | Added getUserInfo() function to fetch user details from WordPress.com |
| cli/index.ts | Registered auth command group with login subcommand |
| cli/commands/auth/login.ts | Main login command implementation |
| cli/commands/auth/tests/login.test.ts | Comprehensive test coverage for login command |
| cli/lib/tests/snapshots.test.ts, appdata.test.ts | Updated test mocks to include new auth token fields |
Comments suppressed due to low confidence (1)
cli/lib/browser.ts:2
- Unused import sprintf.
import { __, sprintf } from '@wordpress/i18n';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cli/commands/auth/login.ts
Outdated
| email: user.email, | ||
| displayName: user.display_name, | ||
| expiresIn: twoWeeksInSeconds, | ||
| expirationTime: now.getTime() + twoWeeksInSeconds, |
Copilot
AI
Nov 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The expirationTime calculation is incorrect. twoWeeksInSeconds is in seconds (1209600), but getTime() returns milliseconds. This should be now.getTime() + twoWeeksInSeconds * 1000 to convert seconds to milliseconds, or the calculation should use twoWeeksInSeconds * 1000 directly.
| expirationTime: now.getTime() + twoWeeksInSeconds, | |
| expirationTime: now.getTime() + twoWeeksInSeconds * 1000, |
| let cmd: string; | ||
| let args: string[]; |
Copilot
AI
Nov 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variables cmd and args are not initialized and will be undefined if process.platform doesn't match 'darwin', 'win32', or 'linux'. This will cause the spawn() call on line 37 to fail. Add a default case to the switch statement that throws an error for unsupported platforms.
cli/lib/browser.ts
Outdated
| @@ -0,0 +1,49 @@ | |||
| import { spawn } from 'child_process'; | |||
| import { __, sprintf } from '@wordpress/i18n'; | |||
Copilot
AI
Nov 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sprintf function is imported but never used in this file. Remove the unused import.
| import { __, sprintf } from '@wordpress/i18n'; | |
| import { __ } from '@wordpress/i18n'; |
cli/lib/browser.ts
Outdated
| import { LoggerError } from 'cli/logger'; | ||
|
|
Copilot
AI
Nov 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused import LoggerError.
| import { LoggerError } from 'cli/logger'; |
sejas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I confirm that the login works as expected, and I was able to authenticate by copying and pasting the token. Here are a couple of suggestions to improve the experience:
- We could avoid hiding the token so the user can see they pasted the correct value.
- We could automatically try three times when the user enters an invalid token, similarly to SSH connections. Currently the feedback is very rough
✖ Failed to fetch user info: The OAuth2 token is invalid., we could at least invite the user to try again.
common/logger-actions.ts
Outdated
|
|
||
| export enum AuthCommandLoggerAction { | ||
| LOGIN = 'login', | ||
| LOGOUT = 'logout', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: We don't really need the logout at this point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll move this to #2027
cli/commands/auth/login.ts
Outdated
| const userData = await readAppdata(); | ||
|
|
||
| const now = new Date(); | ||
| const twoWeeksInSeconds = 2 * 7 * 24 * 60 * 60; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just saw that the expiresIn in appData is 1209600 seconds or two weeks, but I always thought the token was valid for months. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is governed by the back-end. We could look into this further if we think two weeks is too short
|
I like how simple this is. Good work :) |
Related issues
Proposed Changes
This PR is based on @youknowriad's work in #1958. It implements a
studio auth logincommand that:Testing Instructions
developer.wordpress.comnpm run cli:buildnode dist/cli/main.js auth loginPre-merge Checklist