zowe / zlux

The top-level superproject for zLUX. zLUX includes the Zowe Desktop framework in addition to several built-in apps and an example server implementation.
Eclipse Public License 2.0
38 stars 42 forks source link

Wrong handling promise object at `apimlHandler.js` #964

Closed pj892031 closed 1 year ago

pj892031 commented 1 year ago

Describe the bug

At the line https://github.com/zowe/zlux-server-framework/blob/v2.x/staging/plugins/sso-auth/lib/apimlHandler.js#LL142C1-L142C1 is very probably a bug:

  authenticate(request, sessionState) {
    if (request.body) {
      this.logger.debug(`Authenticate with body`);
      return new Promise((resolve, reject) => {
        this.doLogin(request, sessionState, false).then(result=> {
          resolve(result);
        }).catch(e=> {
          Promise.resolve({success: false});
        });
      });
    } else if (request.cookies && request.cookies[TOKEN_NAME]) {
      return this.authenticateViaCookie(request, sessionState);
    } else {
      return Promise.resolve({success: false});
    }
  }

The code Promise.resolve({success: false}); creates a new object and the result will disappear. There should be removed part Promise. to call the method of lambda.

1000TurquoisePogs commented 1 year ago

Looks like the test case is if you pass some invalid credentials into the desktop login, then you'd like to see success:false (such as on the other side, in webauth.js) but if you see undefined instead, then that's what you're pointing out?

hrshkshri commented 1 year ago

The catch block in the if (request.body) clause of the authenticate method returns a new Promise that resolves with an object {success: false}. However, this Promise is not returned from the authenticate method, and therefore, the result will not be used or processed by the calling code.

1000TurquoisePogs commented 1 year ago

Fix merged now, thank you @hrshkshri

hrshkshri commented 1 year ago

Happy to Contribute!