CLI-1212: [auth:logout] remove secrets

[danepowell]

@anavarre can you take this for a spin and provide feedback on the new UX and messaging?

I'm trying to give customers the flexibility to either "unset" the active key (so that they can re-use it by running auth:login again without having to re-input credentials) or to "delete" it entirely.

I'm not sure if "unset" and "delete" are entirely clear in this context. I'm open to feedback.

Delete credentials (the default option):

Screen.Recording.2023-11-28.at.1.10.39.PM.mov

Unset credentials (interactive):

Screen.Recording.2023-11-28.at.1.11.18.PM.mov

Unset credentials (via --no-delete option)

Screen.Recording.2023-11-28.at.1.11.45.PM.mov

[codecov]

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (6917e4a) 91.81% compared to head (258d5a5) 91.52%.

Files	Patch %	Lines
src/Command/Auth/AuthLoginCommand.php	69.23%	4 Missing ⚠️
src/Command/Auth/AuthLogoutCommand.php	80.00%	3 Missing ⚠️
src/Command/CommandBase.php	90.90%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1643      +/-   ##
============================================
- Coverage     91.81%   91.52%   -0.29%     
- Complexity     1778     1782       +4     
============================================
  Files           122      122              
  Lines          6390     6410      +20     
============================================
  Hits           5867     5867              
- Misses          523      543      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[danepowell]

Maybe "unselect" is less ambiguous than "unset".

[anavarre]

Thanks for the recordings. That's really helpful to understand what you're trying to achieve here.

It feels to me like we should:

• Give the name of the subscription in bold, and the UUID in parenthesis. Can we achieve this?
• Set expectations as to what the command intends to do, by also defining what is unsetting or unselecting (I still am unsure myself)
• Then continue with the deletion prompt by first asking the user to confirm whether they want to unset/unselect first (alternative is to quit by doing nothing)

[danepowell]

The subscription isn't relevant here, since the key is attached to the user account, not a subscription.

I'll work to clean up the language.

Multiple prompts aren't necessary since the user clearly intended to do something by running the command in the first place, and unsetting the credentials is easy to reverse by running auth:login again (I can mention that in the command output). If someone really freaks out and wants to abort the command, they can ctrl+c on the prompt and no settings will be changed.

[danepowell]

Give the name of the subscription in bold

Oh I see what you mean though, the API key does have an associated label that we should use.

[danepowell]

I think I've got this the way I like.

I found another significant bug in these commands, which is that they rely on isMachineAuthenticated when they really should only be considering keys in the datastore (since isMachineAuthenticated can also return true based on environment variables).

And that revealed that our tests were just mocking isMachineAuthenticated rather than properly mocking the Cloud datastore and credentials. I don't know how to fix that offhand, so unfortunately I have to remove those tests for now.