search

pnp / cli-microsoft365

Manage Microsoft 365 and SharePoint Framework projects on any platform
https://aka.ms/cli-m365
MIT License
909 stars 317 forks source link

Bug report: weird error when providing wrong values for `login` #6334

Open milanholemans opened 1 week ago

milanholemans commented 1 week ago

Priority

(Low) Something is a little off

Description

I tried to log in using a new connection and got the error message below:

image

As you can see the error is really vague, not telling anything, and Chili 🌶️is saying "undefined". I noticed that I forgot to specify the --tenant option since the app registration is not multi-tenant. I noticed that you get the same behavior when you are providing a clientId that doesn't exist.

Steps to reproduce

Use m365 login --appId b61be999-91c2-415b-a9ec-d34e57124548 (the app ID doesn't exist).

Expected results

A clear and obvious message to the user.

Actual results

Chili saying "undefined" and a vague error

Diagnostics

No response

CLI for Microsoft 365 version

v9.0.0

nodejs version

v18.18.2

Operating system (environment)

Windows

Shell

PowerShell

cli doctor

No response

Additional Info

No response

Adam-it commented 1 week ago

indeed. I managed to reproduce the issue as well image

not nice. Lets fix it up and check if it is not a general problem connected to ZOD usage in commands

waldekmastykarz commented 5 days ago

Let me have a look at it

waldekmastykarz commented 3 days ago

The vague error is coming from MSAL. I suggest that we don't intercept because that might lead to us unnecessarily obfuscating the issue and making it harder to understand what's wrong. I don't think that we can check if the specified app is valid/exists because we might not have permissions to do so.

I'll check separately what's wrong with chili and why it's showing undefined.

waldekmastykarz commented 3 days ago

If you use browser auth rather than device code, you get a clearer error indicating that the app doesn't exist in your tenant. The best way to solve it would be to submit an issue with MSAL.

As for chili, I found the issue: because of the error, MSAL returns an empty response which we're logging as-is, hence the undefined. I'll update it to make printing the response conditional only when it's set.