Skip to content
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

feat: sf-ify plugin-auth #604

Merged
merged 20 commits into from
Feb 10, 2023
Merged

feat: sf-ify plugin-auth #604

merged 20 commits into from
Feb 10, 2023

Conversation

peternhale
Copy link
Contributor

What does this PR do?

Applies sf migration styles to all commands and messages

What issues does this PR fix or reference?

@W-12083545@

test/commands/auth/device/login.test.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@mshanemc mshanemc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice. those UT were a lot of work.

.eslintrc.js Outdated Show resolved Hide resolved
command-snapshot.json Outdated Show resolved Hide resolved
messages/messages.md Outdated Show resolved Hide resolved
src/authBaseCommand.ts Outdated Show resolved Hide resolved
src/commands/auth/accesstoken/store.ts Outdated Show resolved Hide resolved
src/commands/auth/device/login.ts Outdated Show resolved Hide resolved
src/commands/auth/device/login.ts Show resolved Hide resolved
src/commands/auth/logout.ts Outdated Show resolved Hide resolved
test/commands/auth/device/login.test.ts Outdated Show resolved Hide resolved
test/commands/auth/list.test.ts Outdated Show resolved Hide resolved
…e/sf-ify-auth

# Conflicts:
#	package.json
#	yarn.lock
@cristiand391 cristiand391 self-requested a review February 6, 2023 19:20

# deviceWarning

auth:web:login doesn't work when authorizing to a headless environment. Use auth:device:login instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we start using spaces instead of colons in messages? I know both work, just wondering if that will start pushing users towards the new style (also new autocomplete will only work with spaces).

@cristiand391
Copy link
Member

cristiand391 commented Feb 8, 2023

QA notes:

auth accesstoken store

sf auth accesstoken store --instance-url <url> doesn't prompt for AT and successfully exit with SFDX_ACCESS_TOKEN env var.
unset SFDX_ACCESS_TOKEN && sf auth accesstoken store --instance-url <url> ask for AT and successfully logs in.
✅ get help msg in error when the AT is in a wrong format.
❓ the prompt changed and hides what I type compared to current plugin-auth, was this on purpose?
new:
? Access token of user to use for authentication [input is hidden
old:
Access token of user to use for authentication: and see * as I type
❓ what's --no-prompt supposed to do? I don't see any difference in the execution comparing to latest plugin-auth/sfdx

This code seems to suggest I should be asked if I'm trying to use an AT from an org I already auth'd, but I don't see anything:
https://github.com/salesforcecli/plugin-auth/blob/phale/sf-ify-auth/src/commands/auth/accesstoken/store.ts#L107

auth device login

sf auth device login --alias cd --set-default-dev-hub sets alias and org as target-hub
sf auth device login --alias cd --set-default-dev-hub --json > device-code.json doesn't send the code and url as json.

details

If I run sfdx auth device login --setalias cd --setdefaultdevhubusername --json > old-device-code.json nothing gets printed but I can still check the json for the code:

jq 'keys' old-device-code.json
[
  "device_code",
  "interval",
  "user_code",
  "verification_uri"
]

auth jwt grant

sf auth jwt grant --alias na40 --jwt-key-file <path> --set-default --username <username> --client-id <id> sets alias and org as target-org
✅ JSON output matches latest plugin-auth

auth list

✅ alias to force:auth:list, shows deprecation warning.
✅ can see all auth listed (even deleted scratch orgs)
✅ prints No results found if no auth'd orgs are found.
✅ JSON output matches latest plugin-auth
👎🏼 description in help text is duplicated.

details

sf auth list --help
...
DESCRIPTION
  list auth connection information

  list auth connection information

👎🏼 doesn't show loglevel deprecation warning (sf auth list --loglevel debug)
❌ doesn't throw if I pass an invalid flag (sf auth list --lalala)

auth logout

sf auth logout picks the default target org if --target-org isn't specified.
--no-prompt skips prompt and successfully logs out.
--all logs out of all orgs
👎🏼 --target-org is missing its summary in help text:

details

sf auth logout -h
...
FLAGS
  -a, --all                 include all authenticated orgs
  -o, --target-org=<value>
  -p, --no-prompt           do not prompt for confirmation

👎🏼 using --target-org and --all at the same time produces an error as expected but the org flag msg contains object Object:

details

➜  plugin-auth git:(phale/sf-ify-auth) sf auth logout --all --target-org cd
Error: The following error occurred:
  --target-org=[object Object] cannot also be provided when using --all
See more help with --help

👎🏼 logout prompt shows y/n text twice (prob the prompt helper already renders it):

details

➜  plugin-auth git:(phale/sf-ify-auth) sf auth logout --target-org cd
? Are you sure you want to log out from these org(s)?
[email protected]

Important: You need a password to reauthorize scratch orgs. By default, scratch orgs have no password. If you still need your scratch orgs, run "sf
org:generate:password" before logging out. If you don't need the scratch orgs anymore, run "sf org:delete:scratch" or "sf org:delete:sandbox"instead of
logging out.

Log out (y/n)? (Y/n)
Error: Timed out after 10000 ms.

❌ can't logout from an expired scratch org.

details top: this branch linked in `sf`
bottom: `sfdx-cli/7.187.1 darwin-x64 node-v18.12.1`, no linked plugins.
both usernames used were from expired scratch orgs.

Screenshot 2023-02-08 at 10 40 33

auth sfdxurl store

sf auth sfdxurl store --sfdx-url-file authFile.json --alias na40 --set-default sets alias and target-org
✅ JSON output matches latest plugin-auth

auth web login

sf auth web login --alias cd --set-default-dev-hub set alias and set the org as the target hub.
question:
In sf we use target-dev-hub to refer to the hub that a command will default to, should these flags also reflect that name change?
✅ JSON output matches latest plugin-auth

@cristiand391
Copy link
Member

QA updates

auth accesstoken store

❓ the prompt changed and hides what I type compared to current plugin-auth, was this on purpose?
new:

✅ change was reverted, behavior matches latest plugin-auth

auth device login

sf auth device login --alias cd --set-default-dev-hub --json > device-code.json doesn't send the code and url as json.

✅ fixed

auth list

❌ doesn't throw if I pass an invalid flag (sf auth list --lalala)

✅ fixed

auth logout

can't logout from an expired scratch org.

✅ fixed

NEW
❌ If I don't pass --no-prompt the command ask for confirmation but the default answer is yes.
latest plugin-auth/sfdx doesn't set a default answer so pressing Enter makes it re-render the prompt. I would prefer the default answer to be no as this action will delete auth files.

@cristiand391
Copy link
Member

QA updates

auth logout

NEW
❌ If I don't pass --no-prompt the command ask for confirmation but the default answer is yes.
latest plugin-auth/sfdx doesn't set a default answer so pressing Enter makes it re-render the prompt. I would prefer the default answer to be no as this action will delete auth files.

✅ fixed

@cristiand391 cristiand391 merged commit b303fce into main Feb 10, 2023
@cristiand391 cristiand391 deleted the phale/sf-ify-auth branch February 10, 2023 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants