add sso passcode option

[h0nIg]

What Need Does It Address?

we need to specify the sso passcode as an option on the command line. STDIN is not useful, because stty is used and it will give you errors. There is a password option so there is no reason for not adding an sso passcode option

echo PASSCODE | cf login -sso
will result in:
stty: 'standard input': Inappropriate ioctl for device

Possible Drawbacks

none, because if the user will see sso code via "ps" then its already too late / the code is valid only once

Why Should This Be In Core?

because CLI offers the sso login and therefore this should be part of the general CLI

Description of the Change

add sso passcode CLI option / added tests

[cfdreddbot]

Hey h0nIg!

Thanks for submitting this pull request!

All pull request submitters and commit authors must have a Contributor License Agreement (CLA) on-file with us. Please sign the appropriate CLA (individual or corporate).

When sending signed CLA please provide your github username in case of individual CLA or the list of github usernames that can make pull requests on behalf of your organization.

If you are confident that you're covered under a Corporate CLA, please make sure you've publicized your membership in the appropriate Github Org, per these instructions.

Once you've publicized your membership, one of the owners of this repository can close and reopen this pull request, and dreddbot will take another look.

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/137267221

The labels on this github issue will be updated when the story is started.

[cfdreddbot]

Hey h0nIg!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

[h0nIg]

ping @dkoper

[dkoper]

@h0nIg Sorry, I asked the UAA PM to take a look as well. I'll ping her again.

[h0nIg]

ping @dkoper again :(

[dkoper]

ping @sreetummidi again

[dkoper]

@h0nIg I happened to notice #686, which seems to indicate cf login -sso <<< XXXX once worked. I wonder if it's better to try and make that work again.

[h0nIg]

@dkoper i dont think get this working again is an option, because it seems that CF switched to stty for security reasons.

go support for terminals in combination with windows <--> linux toolsets like mingw / cygwin is horrible. checkout #171, therefore from my point of view implementing a cli option like the present option for password is a good way to go.

[dkoper]

@h0nIg UAA PM said she sees no issues with it.
Let's talk about the UX. I would expect the following:

USAGE:
   cf login [-a API_URL] [-u USERNAME] [-p PASSWORD] [-o ORG] [-s SPACE] [--sso | --sso-passcode PASSCODE]

EXAMPLES:
...
   cf login --sso (cf will provide a url to obtain a one-time passcode to login)

OPTIONS:
...
   --sso                  Prompt for a one-time passcode to login
   --sso-passcode         One-time passcode

I don't think this option needs an example (trivial enough), but we should be consistent in use of password vs. passcode in relation to SSO. (UAA PM agreed to using "passcode").

[h0nIg]

@dkoper @sreetummidi passcode vs password should be consistent after last commit (in favour of passcode). when do you expect this will be available as a release version (2 months between v6.23.0 and v6.22.2)?

[dkoper]

@h0nIg I think we'll be ready for the next release in the next one or two weeks.
If you want your PR incorporated, you will have to work with me on the UX. Please describe in detail what your PR is giving (incl. help text); if different from my suggestion above, convince me it's better.

[h0nIg]

@dkoper I did what you requested and changed password to passcode as requested by uaa pm. I created a description as part of my pull request description.
Whats wrong with my last commit or what needs to be improved?

[dkoper]

@h0nIg
Looking at the code, it seems you expect the user to type cf login --sso --sso-passcode CODE while I suggested that cf login --sso-passcode CODE should be enough for the user to express their intent.
And that makes its use easy enough not to need an entry in the examples.
You haven't fixed that, nor convinced me that your way is better.

Please copy & paste the help output here of this command with your patch applied and if there are any differences with what I mentioned I expected, enter into a discussion with me why yours is better.

[h0nIg]

@dkoper now i understand what this is about. the UX you proposed was my first solution but i had to change for some reason. as far as i can remember, there was a problem with the presence of the parameter on https://github.com/cloudfoundry/cli/blob/master/cf/commands/login.go#L43, because if the parameter is present but no value is specified, then the cf cli will complain about a missing parameter.

i will double check this tomorrow

[dkoper]

@h0nIg If you're OK with the UX and help text I proposed, you could make it as close to it as you can and my team can look into and fix the issue you're seeing (especially if it requires delving into existing code more than you'd care for)?

[h0nIg]

@dkoper , its working and i made a mistake the last time. I changed it to be one option with optional argument

[dkoper]

@h0nIg So can you paste the command help after your change?
(Note that I don't have a dev environment. I could put that on my to do list and merge your PR to try it out, but my to do list is fairly long)

[h0nIg]

@dkoper ok i was wrong again. i found the problem i came across with StringFlag without argument:

original branch is available as https://github.com/h0nIg/cli/tree/ssopasscode_multi, because i forced push the pull request branch with the new content

$ ./cli.exe login --sso
FAILED
Incorrect Usage

No value provided for flag: --sso

NAME:
   login - Log user in

USAGE:
   cf login [-a API_URL] [-u USERNAME] [-p PASSWORD] [-o ORG] [-s SPACE]

WARNING:
   Providing your password as a command line option is highly discouraged
   Your password may be visible to others and may be recorded in your shell history

EXAMPLES:
   cf login (omit username and password to login interactively -- cf will prompt for both)
   cf login -u name@example.com -p pa55woRD (specify username and password as arguments)
   cf login -u name@example.com -p "my password" (use quotes for passwords with a space)
   cf login -u name@example.com -p "\"password\"" (escape quotes if used in password)
   cf login --sso [PASSCODE] (cf will provide a url to obtain a one-time password to login)

ALIAS:
   l

OPTIONS:
   -a                         API endpoint (e.g. https://api.example.com)
   -o                         Org
   -p                         Password
   -s                         Space
   --skip-ssl-validation      Skip verification of the API endpoint. Not recommended!
   --sso                      Use a one-time password to login
   -u                         Username

[h0nIg]

this can only be solved with StringFLag ...(Value: "default") in combination with IsSet("sso") and String("sso") != default

[dkoper]

So what's the plan?

[h0nIg]

hi @dkoper ,

after checking the code again, i recognized the "default" feature is not a workaround. the problem is that its either "parameter with value present" or "parameter absent" but not "parameter present without value". see https://github.com/cloudfoundry/cli/blob/master/cf/flags/flags.go#L66 and https://github.com/cloudfoundry/cli/blob/master/cf/flags/flags.go#L135 (code is checking args directly). There are two possibilities:

• its ok for you to work with two parameters, so you can directly merge this pullrequest (did "git push -f" again with original branch)
• its not ok to work with two parameters, you can take my work and try to extend the framework with your team (check args directly instead of default, changing this is out of this pullrequest scope, branch https://github.com/h0nIg/cli/tree/ssopasscode_single)

help output / UX:

$ ./cli.exe help login
NAME:
   login - Log user in

USAGE:
   cli.exe login [-a API_URL] [-u USERNAME] [-p PASSWORD] [-o ORG] [-s SPACE]

WARNING:
   Providing your password as a command line option is highly discouraged
   Your password may be visible to others and may be recorded in your shell history

EXAMPLES:
   cli.exe login (omit username and password to login interactively -- cli.exe will prompt for both)
   cli.exe login -u name@example.com -p pa55woRD (specify username and password as arguments)
   cli.exe login -u name@example.com -p "my password" (use quotes for passwords with a space)
   cli.exe login -u name@example.com -p "\"password\"" (escape quotes if used in password)
   cli.exe login --sso [--sso-passcode PASSCODE] (cli.exe will provide a url to obtain a one-time password to login)

ALIAS:
   l

OPTIONS:
   -a                         API endpoint (e.g. https://api.example.com)
   -o                         Org
   -p                         Password
   -s                         Space
   --skip-ssl-validation      Skip verification of the API endpoint. Not recommended!
   --sso                      Use a one-time password to login
   --sso-passcode             One-time password
   -u                         Username

SEE ALSO:
   api, auth, target

[dkoper]

@h0nIg OK, thanks for your efforts so far. I'll ask my devs to take a look.

[sbachuMSOL]

We are very much interested to use this feature - any roadmap of the release with this feature --sso-passcode?

[dkoper]

@sbachuMSOL The PR's been merged so you can start using it with our edge binaries now.
I expect a release mid-next week.

[sbachuMSOL]

@dkoper Thank you for sharing the release date.