feat: allow stdin for sfdx-url #886

k-capehart · 2023-12-13T21:29:09Z

What does this PR do?

oclif has been updated to include a new option for flags: allowStdin
Another oclif update allows a flag to be designated as stdin 'only', meaning that it will error out if anything other than the '-' value is provided
A new flag has been introduced per recommendation called --sfdx-url-stdin and has allowStdin: 'only'
This means --sfdx-url-file is no longer required, so an error is thrown if neither --sfdx-url-stdin or --sfdx-url-file are given
Original discussion on this solution found here: Allow flags to read its value from stdin oclif/core#761

$ echo url | sf org login sfdx-url --sfdx-url-stdin -

What issues does this PR fix or reference?

closes: forcedotcom/cli#2120
[skip-validate-pr]

jshackell-sfdc

Just a light edit. This is no guarantee that we'll accept your contribution! But I noticed it and thought I'd give you some suggestions. Thx.

messages/sfdxurl.store.md

Co-authored-by: Juliet Shackell <63259011+jshackell-sfdc@users.noreply.github.com>

mdonnalley · 2023-12-14T16:18:54Z

@k-capehart Thanks for taking this on!

As you noted, this change would allow someone to pass the url directly to the flag. I just made a PR to oclif/core that adds an only option to allowStdin. I think we should create a separate flag for the stdin (--sfdx-url-stdin) and use allowStdin: 'only' to force users to supply the - value

That way we don't open up the possibility for people to provide the url directly

k-capehart · 2023-12-14T16:37:37Z

@k-capehart Thanks for taking this on!

As you noted, this change would allow someone to pass the url directly to the flag. I just made a PR to oclif/core that adds an only option to allowStdin. I think we should create a separate flag for the stdin (--sfdx-url-stdin) and use allowStdin: 'only' to force users to supply the - value

That way we don't open up the possibility for people to provide the url directly

Perfect! I'll update this PR to reflect that new change in oclif. Thanks.

mdonnalley · 2024-01-02T17:25:58Z

@k-capehart FYI, I merged the oclif/core PR. Changes should be available in 3.16.0

k-capehart · 2024-01-08T14:21:50Z

@k-capehart FYI, I merged the oclif/core PR. Changes should be available in 3.16.0

@mdonnalley Thanks for your help. This PR has been updated to reflect the new changes and is ready for review now.

k-capehart · 2024-01-08T14:22:58Z

@jshackell-sfdc Also, I updated the documentation if you don't mind reviewing again. Thank you!

mdonnalley · 2024-01-08T16:42:14Z

src/commands/org/login/sfdx-url.ts

@@ -33,10 +33,18 @@ export default class LoginSfdxUrl extends AuthBaseCommand<AuthFields> {
    'sfdx-url-file': Flags.file({
      char: 'f',
      summary: messages.getMessage('flags.sfdx-url-file.summary'),
-      required: true,
+      required: false,


We should add this to both sfdx-url-file and sfdx-url-stdin to ensure that people provide one of those

exactlyOne: ['sfdx-url-file', 'sfdx-url-stdin']

Doing that would allow you to drop the validation logic you have on lines 88-96

We should also add exclusive: ['sfdx-url-stdin'] to make sure that people don't provide both

@mdonnalley I added these suggestions but kept the validation logic, otherwise I get a compiler error that sfdxAuthUrl could be undefined. I think that's because its not smart enough to know that one of the flags are guaranteed to be provided. Let me know if you know a work-around, but I think that's consistent with other implementations.

@k-capehart I think exclusive is implied in exactlyOne so there's not actually a need for both - you only need exactlyOne on both flags

Sorry about that - should have tested it all out before asking you to make a change!

@mdonnalley Oh makes sense, that explains the repetition in error messages I was seeing. I removed it 👍

mdonnalley · 2024-01-08T16:42:47Z

src/commands/org/login/sfdx-url.ts

      deprecateAliases: true,
      aliases: ['sfdxurlfile'],
    }),
+    'sfdx-url-stdin': Flags.file({


We should add exclusive: ['sfdx-url-file'] to make sure that people don't provide both

@mdonnalley, question: It sounds to me like --sfdx-url-stdin can take only one value (-), and if it's present then the user is doing the piping-through-stdin thing. So wouldn't it make more sense to make --sfdx-url-stdin a Boolean flag? Or maybe I'm missing something.

I can provide some context, this is what I based the solution on: oclif/core#761 (comment)

In this case, the value should always be - but that is consistent with CLI standards because oclif requires the - value in order to read from standard input.

messages/sfdxurl.store.md

Co-authored-by: Juliet Shackell <63259011+jshackell-sfdc@users.noreply.github.com>

messages/sfdxurl.store.md

Co-authored-by: Juliet Shackell <63259011+jshackell-sfdc@users.noreply.github.com>

k-capehart · 2024-01-08T21:03:35Z

@mdonnalley @jshackell-sfdc I appreciate you both! It's exciting to get this one working.

mdonnalley · 2024-01-08T21:59:18Z

QA

Setup:

pulled changes
linked to sf
put auth url into auth.txt and auth.json file

✅ echo <URL> | sf org login sfdx-url --sfdx-url-stdin -
✅ echo <URL> | sf org login sfdx-url --sfdx-url-stdin throws validation error
✅ sf org login sfdx-url --sfdx-url-file auth.txt
✅ sf org login sfdx-url --sfdx-url-file auth.json
✅ sf org login sfdx-url --sfdx-url-file auth.txt --sfdx-url-stdin - throws validation error
✅ sf org login sfdx-url throws validation error

Great work @k-capehart 🏆

mdonnalley · 2024-01-08T22:02:11Z

@k-capehart I'm going to merge this into a separate feature branch first so that all our integration tests can run. Once those pass I'll merge to main

* feat: allow stdin for sfdx-url * Update messages/sfdxurl.store.md * Update messages/sfdxurl.store.md * Update messages/sfdxurl.store.md * Update messages/sfdxurl.store.md * feat: create new --sfdx-url-stdin flag * feat: remove unnecessary error * feat: allowStdin set to 'only' * Update messages/sfdxurl.store.md * feat: add suggestions to make flags exclusive and required * feat: remove exclusive option, as it is implied with exactlyOne * Update messages/sfdxurl.store.md --------- Co-authored-by: Kyle Capehart <kyle.capehart@torqata.com> Co-authored-by: Juliet Shackell <63259011+jshackell-sfdc@users.noreply.github.com>

feat: allow stdin for sfdx-url

76e2ccc

k-capehart requested a review from a team as a code owner December 13, 2023 21:29

salesforce-cla bot added the cla:signed label Dec 13, 2023

jshackell-sfdc reviewed Dec 13, 2023

View reviewed changes

messages/sfdxurl.store.md Outdated Show resolved Hide resolved

messages/sfdxurl.store.md Outdated Show resolved Hide resolved

messages/sfdxurl.store.md Outdated Show resolved Hide resolved

messages/sfdxurl.store.md Outdated Show resolved Hide resolved

Kyle Capehart and others added 4 commits December 13, 2023 16:48

Update messages/sfdxurl.store.md

24cf4b9

Co-authored-by: Juliet Shackell <63259011+jshackell-sfdc@users.noreply.github.com>

Update messages/sfdxurl.store.md

a50fc02

Co-authored-by: Juliet Shackell <63259011+jshackell-sfdc@users.noreply.github.com>

Update messages/sfdxurl.store.md

7c4b5fa

Co-authored-by: Juliet Shackell <63259011+jshackell-sfdc@users.noreply.github.com>

Update messages/sfdxurl.store.md

dba0e4b

Co-authored-by: Juliet Shackell <63259011+jshackell-sfdc@users.noreply.github.com>

mdonnalley added the enhancement New feature or request label Dec 14, 2023

k-capehart added 2 commits December 20, 2023 13:07

feat: create new --sfdx-url-stdin flag

3089130

feat: remove unnecessary error

0d83459

Kyle Capehart and others added 2 commits January 8, 2024 09:07

Merge branch 'salesforcecli:main' into allow-stdin-sfdx-url

76b5250

feat: allowStdin set to 'only'

010509c

mdonnalley requested changes Jan 8, 2024

View reviewed changes

jshackell-sfdc reviewed Jan 8, 2024

View reviewed changes

messages/sfdxurl.store.md Outdated Show resolved Hide resolved

Update messages/sfdxurl.store.md

5302946

Co-authored-by: Juliet Shackell <63259011+jshackell-sfdc@users.noreply.github.com>

jshackell-sfdc reviewed Jan 8, 2024

View reviewed changes

messages/sfdxurl.store.md Outdated Show resolved Hide resolved

k-capehart added 2 commits January 8, 2024 12:56

feat: add suggestions to make flags exclusive and required

a532ed1

feat: remove exclusive option, as it is implied with exactlyOne

0454ec9

mdonnalley approved these changes Jan 8, 2024

View reviewed changes

jshackell-sfdc reviewed Jan 8, 2024

View reviewed changes

messages/sfdxurl.store.md Outdated Show resolved Hide resolved

jshackell-sfdc approved these changes Jan 8, 2024

View reviewed changes

Update messages/sfdxurl.store.md

7c34fc6

Co-authored-by: Juliet Shackell <63259011+jshackell-sfdc@users.noreply.github.com>

mdonnalley changed the base branch from main to mdonnalley/886 January 8, 2024 22:01

mdonnalley merged commit 38220ca into salesforcecli:mdonnalley/886 Jan 8, 2024
1 check passed

mdonnalley mentioned this pull request Jan 8, 2024

feat: allow stdin for sfdx-url (#886) #910

Merged

k-capehart mentioned this pull request Feb 2, 2024

stdin functionality not working for sf org login sfdx-url forcedotcom/cli#2690

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: allow stdin for sfdx-url #886

feat: allow stdin for sfdx-url #886

k-capehart commented Dec 13, 2023 •

edited

Loading

jshackell-sfdc left a comment

mdonnalley commented Dec 14, 2023

k-capehart commented Dec 14, 2023

mdonnalley commented Jan 2, 2024

k-capehart commented Jan 8, 2024

k-capehart commented Jan 8, 2024

mdonnalley Jan 8, 2024 •

edited

Loading

k-capehart Jan 8, 2024

mdonnalley Jan 8, 2024

k-capehart Jan 8, 2024

mdonnalley Jan 8, 2024

jshackell-sfdc Jan 8, 2024

k-capehart Jan 8, 2024 •

edited

Loading

k-capehart commented Jan 8, 2024

mdonnalley commented Jan 8, 2024 •

edited

Loading

mdonnalley commented Jan 8, 2024

feat: allow stdin for sfdx-url #886

feat: allow stdin for sfdx-url #886

Conversation

k-capehart commented Dec 13, 2023 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

jshackell-sfdc left a comment

Choose a reason for hiding this comment

mdonnalley commented Dec 14, 2023

k-capehart commented Dec 14, 2023

mdonnalley commented Jan 2, 2024

k-capehart commented Jan 8, 2024

k-capehart commented Jan 8, 2024

mdonnalley Jan 8, 2024 • edited Loading

Choose a reason for hiding this comment

k-capehart Jan 8, 2024

Choose a reason for hiding this comment

mdonnalley Jan 8, 2024

Choose a reason for hiding this comment

k-capehart Jan 8, 2024

Choose a reason for hiding this comment

mdonnalley Jan 8, 2024

Choose a reason for hiding this comment

jshackell-sfdc Jan 8, 2024

Choose a reason for hiding this comment

k-capehart Jan 8, 2024 • edited Loading

Choose a reason for hiding this comment

k-capehart commented Jan 8, 2024

mdonnalley commented Jan 8, 2024 • edited Loading

mdonnalley commented Jan 8, 2024

k-capehart commented Dec 13, 2023 •

edited

Loading

mdonnalley Jan 8, 2024 •

edited

Loading

k-capehart Jan 8, 2024 •

edited

Loading

mdonnalley commented Jan 8, 2024 •

edited

Loading