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

Consistent "login" experience for various configurations #923

Closed
MikeBauerCA opened this issue Jan 26, 2021 · 10 comments · Fixed by #1188
Closed

Consistent "login" experience for various configurations #923

MikeBauerCA opened this issue Jan 26, 2021 · 10 comments · Fixed by #1188
Assignees

Comments

@MikeBauerCA
Copy link
Contributor

Currently:

  • If you are going direct to service, you run zowe config secure to set up creds
  • If you are going through apiml, you run zowe auth login apiml to set up creds
  • If you have a mix, perhaps you run both

This issue proposes we standardize on zowe config secure but add custom logic to handle authTokens so we prompt you for login (since it is unlikely you know the authentication token). zowe auth login apiml can continue to work. As an example, let's say we have the following properties in our secure array:

        "profiles.myLPAR.properties.user",
        "profiles.myLPAR.properties.password",
        "profiles.myLPAR2.properties.user",
        "profiles.myLPAR2.properties.password",
        "profiles.my_base.properties.authToken"

Then zowe config secure could prompt for myLPAR.properties.user, myLPAR.properties.password, but instead of prompting for my_base.properties.authToken, prompt for a username/password combo to get the token... we'd need to iron the prompting language.

@zFernand0 zFernand0 added this to the Zowe vNext Backlog milestone Jul 22, 2021
@zFernand0
Copy link
Member

zFernand0 commented Jul 22, 2021

Estimate: (3 - 5) & (5 - 8) points respectively

We could accomplish this in a two step process:

  1. Standardize on zowe config secure

    prompt for a username/password combo to get the token

  2. Implement an auto-store configuration property.
    • Default value: false (to avoid changing existing behavior)
    • If auto-store ==false: User gets prompted every time for missing credentials until zowe config secure is executed.
    • If auto-store == true:
      • User executes any zowe command that requires credentials
      • We prompt for all missing properties, including user and password
      • We login to API ML based on the following scenarios:
        • If LPAR.secure.contains("authToken")
        • If LPAR.properties.basePath != null || LPAR.properties.tokenType == apimlAuthenticationToken
      • We store the credentials (user and password or token), as well as other properties provided in the command execution.
        • Storage will be secure for the properties in the LPAR.secure array.
        • Storage will be plain text for all other properties

Questions:

  1. Should the login step be more generalized?
    • Check for tokenType. If LPAR.properties.tokenType == xyz and call the loginToXYZ API
    • If LPAR.properties.tokenType == null && LPAR.properties.basePath != null: Assume APIML.
    • Otherwise, store user and password (based on LPAR.secure)

Update:

  • UX: Since this auto-store property only applies to secure values, it will be a good idea to rename it to
    auto-store-secure-properties.
  • Change default value to true

@MikeBauerCA
Copy link
Contributor Author

Recommend changing default value to true

@jellypuno
Copy link
Contributor

@crawr for awareness

@awharn
Copy link
Member

awharn commented Sep 30, 2021

I started looking into the second story here, and I have a couple questions that would probably need to be answered/considered.

  1. Should a configuration file be initialized if one does not already exist?
  2. In the event that both a service and base profile are defined (or both missing), and a property is prompted for, which 'profile' should the property be stored in?

@t1m0thyj
Copy link
Member

t1m0thyj commented Oct 1, 2021

  1. Should a configuration file be initialized if one does not already exist?

If the default value of the "auto-store" property is false (according to the comment above), would that mean our new behavior only takes effect when a configuration file already exists with "auto-store" set to true?

  1. In the event that both a service and base profile are defined (or both missing), and a property is prompted for, which 'profile' should the property be stored in?

I think I agree with the opinion @MikeBauerCA mentioned at standup, that we should add properties in the most specific place possible, which would be service profiles.

@t1m0thyj
Copy link
Member

t1m0thyj commented Oct 1, 2021

Sorry, I completely missed "Recommend changing default value to true". 😢 Thanks @zFernand0 for pointing this out.

Regarding # 1, in the case that a config file doesn't exist yet:

  • Pros of creating one:
    • Credentials will automatically be stored without users having to learn about the zowe config init command.
  • Cons of creating one:
    • Users may be confused where their credentials are stored. This could be alleviated by clearly telling them in the command output what we are doing when the config is created.
    • We have to decide which layer to add the properties in:
      • Global - would make it easy for users to find all the auto-stored properties in one place rather than having them scattered across various config files on disk
      • Project - may be more consistent our philosophy for # 2 - adding properties in the most specific place possible to avoid affecting a larger scope of profiles/projects than intended
      • User - auto-storing properties in user config may make it less likely for them to get accidentally checked in to source control

I'm not sure I see a clear winner here, but hopefully these points spark some further discussion 🙂

Regarding # 2, storing properties in service profiles would potentially store the same user/password values in multiple places in the same config file. But I think it would be too risky to assume that all services in the config file share the same credentials and store the values in the base profile.

@MikeBauerCA
Copy link
Contributor Author

MikeBauerCA commented Oct 1, 2021

For #1, we could consider having the default value be false (e.g. if zowe.config.json doesn't exist) but upon zowe config init or zowe config auto-init place a value of true for the property in the resulting zowe.config.json.

@t1m0thyj
Copy link
Member

t1m0thyj commented Nov 5, 2021

Before closing this issue, should we document the new autoStore behavior somewhere like in our Early Access documentation for Team Config? https://github.com/zowe/zowe-cli/blob/next/docs/Early%20Access%20-%20Using%20Global%20Profile%20Configuration.md

@t1m0thyj
Copy link
Member

To be addressed in zowe/docs-site#2059

@t1m0thyj
Copy link
Member

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 a pull request may close this issue.

5 participants