zowe / zowe-cli

Zowe CLI
Eclipse Public License 2.0
114 stars 86 forks source link

Clarify credential-related behavior of zowe zosmf check status #724

Open ChrisBoehmCA opened 4 years ago

ChrisBoehmCA commented 4 years ago

Hi,

The z/OSMF REST API behind zowe zosmf check status does not raise an error if the username and/or password provided are incorrect. I think this should be reflected in the help text and output of the command. Currently you receive a message like this, unconditionally:

$ zowe zosmf check status --user fake --password fakepass
The user 'fake' successfully connected to z/OSMF on 'hostname.xxx'.

This might lead users to believe that their credentials are being validated on this command when they currently are not.

Reference: https://github.com/zowe/zowe-cli/blob/master/packages/zosmf/src/cli/check/status/Status.handler.ts

zFernand0 commented 4 years ago

I agree with you @ChrisBoehmCA. Espacially in the https://github.com/zowe/zowe-cli/blob/master/packages/zosmf/src/api/CheckStatus.ts JSDoc, given that People building applications (e.g. vscode extensions, webapps, ...) could communicate a wrong message to their users.

dkelosky commented 4 years ago

This is beyond the scope of the issue (updating --help content).

But, there is a new API for validating credentials and we also had code for "validating" credentials on check status:

/*
* This program and the accompanying materials are made available under the terms of the
* Eclipse Public License v2.0 which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-v20.html
*
* SPDX-License-Identifier: EPL-2.0
*
* Copyright Contributors to the Zowe Project.
*
*/
import { AbstractSession, ImperativeExpect, Logger, ImperativeError, RestConstants } from "@zowe/imperative";
import { ZosmfRestClient } from "../../../rest";
import { LoginConstants } from "./LoginConstants";

/**
 * Class to handle logging onto z/OSMF.
 * @export
 * @class Login
 */
export class Login {

    /**
     * Perform z/OSMF login to obtain LTPA2 or other token types.
     * @static
     * @param {AbstractSession} session
     * @returns
     * @memberof Login
     */
    public static async login(session: AbstractSession) {
        Logger.getAppLogger().trace("Login.login()");
        ImperativeExpect.toNotBeNullOrUndefined(session, "Required session must be defined");

        // we eventually will hope to use a real z/OSMF authentication endpoint
        // get client instance and perform a get on /zosmf/info
        const client = new ZosmfRestClient(session);
        await client.request({
            request: "GET",
            resource: LoginConstants.RESOURCE,
        });

        // this endpoint doesn't require authentication, we treat a missing LTPA2 token
        // as unauthorized and simulate a 401 (so that the error messaging will not change when we have a
        // true authentication endpoint)
        if (session.ISession.tokenValue === undefined) {

            // pretend it was a basic auth error with 401 when obtaining the token
            client.response.statusCode = RestConstants.HTTP_STATUS_401;
            (session as any).mISession.type = "basic";

            // throw as HTTP(S) error
            throw (client as any).populateError({
                msg: "Rest API failure with HTTP(S) status 401",
                source: "http"
            });
        }

        // return token to the caller
        return session.ISession.tokenValue;
    }

}
ChrisBoehmCA commented 4 years ago

That's cool. Maybe something like that could be added to the command. This issue is about both the stdout output of the command and the help text though just to be clear.

dkelosky commented 3 years ago

Following up nearly a year later 😄 . Yes, I understand the issue is about the wording of the command output.

We could resolve that by indicating that credentials are not validated, or the command could be updated to actually validate credentials and notify the user accordingly.

github-actions[bot] commented 2 years ago

Thank you for raising this issue. The community has 90 days to upvote 👍 the issue. If it receives 5 upvotes, we will move it to our backlog. If not, we will close it.

github-actions[bot] commented 1 year ago

Thank you for raising this enhancement request. The community has 90 days to vote on it. If the enhancement receives at least 5 upvotes, it is added to our development backlog. If it receives fewer votes, the issue is closed.