OfficeDev / Office-Addin-Scripts

A set of scripts and packages that are consumed in Office add-ins projects.
MIT License
159 stars 100 forks source link

office-addin-sso configure error #659

Open kacomsrd opened 2 years ago

kacomsrd commented 2 years ago

Expected behavior

npm run configure-sso works.

Current behavior

Using Yeoman and generator-office I scaffold an Office addin

? Choose a project type: Office Add-in Task Pane project supporting single sign-on ? Choose a script type: (Use arrow keys) ? Choose a script type: TypeScript ? What do you want to name your add-in? foobar ? Which Office client application would you like to support? Outlook

Calling npm run configure-sso yields the following:

npm WARN config global `--global`, `--local` are deprecated. Use `--location=global` instead.

> office-addin-taskpane-sso@0.0.0 configure-sso
> office-addin-sso configure manifest.xml

Opening browser for authentication to Azure. Enter valid Azure credentials
Login was successful!
Registering new application in Azure
Application was successfully registered with Azure
Setting identifierUri
    Attempt 1
Itendifier Set
Setting signin audience
    Attempt 1
Sign In Audience Set
undefined:35
      "displayName": "���� �\�Y",
                             ^

SyntaxError: Unexpected token � in JSON at position 1189
    at JSON.parse (<anonymous>)
    at C:\Users\christian\Dev\Cotoha\yo\foobar\node_modules\office-addin-sso\lib\configure.js:185:40
    at Generator.next (<anonymous>)
    at C:\Users\christian\Dev\Cotoha\yo\foobar\node_modules\office-addin-sso\lib\configure.js:7:71
    at new Promise (<anonymous>)
    at __awaiter (C:\Users\christian\Dev\Cotoha\yo\foobar\node_modules\office-addin-sso\lib\configure.js:3:12)
    at C:\Users\christian\Dev\Cotoha\yo\foobar\node_modules\office-addin-sso\lib\configure.js:179:95
    at ChildProcess.exithandler (node:child_process:390:7)
    at ChildProcess.emit (node:events:527:28)
    at maybeClose (node:internal/child_process:1092:16)

Steps to Reproduce

Please provide detailed steps for reproducing the issue.

  1. install yo and generator-office
  2. scaffold project with the options above
  3. run npm run configure-soo

Context

I am assuming that this error occurs while checking if I am a tenant admin. It looks to me like an encoding problem. I am not sure what "displayName" the error above refers to, but I assume it is my name from AAD. That name contains Japanese characters (Katakana).

kacomsrd commented 2 years ago

Some more information:

I tried the above in both git bash and powershell (windows terminal).

chcp shows codepage 932

I tried again after executing chcp 65001 but the problem persists.

kacomsrd commented 2 years ago

Update: The trying to parse the output of the following command triggers the error. az rest -m get -u https://graph.microsoft.com/v1.0/directoryRoles/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx/members --headers "Content-Type=application/json" Looking at the output, everything that is supposed to be Japanese is garbled.

kacomsrd commented 2 years ago

Update: It seems to work after setting Control Panel > Region > Administrative > Change system local -> 'Beta: Use Unicode UTF-8 for worldwide language support' but I feel this should not be the solution. The Japanese love their legacy Shift-JIS only apps and I have no idea what side-effects this setting has.

millerds commented 2 years ago

The commands to manipulate azure are run via child_process command line (like you tried directly) and the result is a json string that is being parse with JSON.parse(response). When you change the system setting that changes the result coming from the command line to be readable. If there was a way to know what encoding the response was and then convert it before sending it to the json parser we could take care of this . . . but I'm not sure how to determine the response encoding or to translate it to utf8.

millerds commented 1 year ago

Looks like it might be possible to fix this by using "cmd /c chcp 65001>nul && " as a prefix to the azure command being run on Win32.