xstos commented 3 years ago

Prerequisites

[x] I am running the latest version of Node and the tools
[x] I checked the documentation and found no answer
[x] I checked to make sure that this issue has not already been filed

Expected behavior

The generator demo should work

Current behavior

 yo office

     _-----_     ╭──────────────────────────╮
    |       |    │   Welcome to the Office  │
    |--(o)--|    │   Add-in generator, by   │
   `---------´   │ @OfficeDev! Let's create │
    ( _´U`_ )    │    a project together!   │
    /___A___\   /╰──────────────────────────╯
     |  ~  |
   __'.___.'__
 ´   `  |° ´ Y `

? Choose a project type: Office Add-in Task Pane project supporting single sign-on
? Choose a script type: JavaScript
? What do you want to name your add-in? SSO365
? Which Office client application would you like to support? Word

----------------------------------------------------------------------------------

      Creating SSO365 add-in for Word using JavaScript and Single-sign-on at C:\SSO365

----------------------------------------------------------------------------------

      Congratulations! Your add-in has been created! Your next steps:

      1. Go the directory where your project was created:

         cd C:\SSO365

      2. Configure your SSO taskpane add-in:

         npm run configure-sso

      3. Start the local web server and sideload the add-in:

         npm start

      4. Open the project in VS Code:

         code .

         For more information, visit http://code.visualstudio.com.

      Please visit https://docs.microsoft.com/office/dev/add-ins for more information about Office Add-ins.

Steps to Reproduce

Please provide detailed steps for reproducing the issue.

1) run generator as above 2) run npm run configure-sso

Context

Please provide any relevant information about your setup. This is important in case the issue is not reproducible except for under certain conditions.

Operating System: Windows 10 Pro 10.0.19042 Build 19042
Node version: 14.15.4
Office version: 2101 Build 13628.20380 Click-to-Run (Microsoft 365 Apps for business )
Tool version: yeoman latest

Failure Logs

npm run configure-sso

> office-addin-taskpane-sso-js@0.0.0 configure-sso C:\SSO365
> office-addin-sso configure manifest.xml

(node:17100) UnhandledPromiseRejectionWarning: SyntaxError: Unexpected token  in JSON at position 3324
    at JSON.parse (<anonymous>)
    at C:\SSO365\node_modules\office-addin-sso\lib\configure.js:188:40
    at Generator.next (<anonymous>)
    at C:\SSO365\node_modules\office-addin-sso\lib\configure.js:6:71
    at new Promise (<anonymous>)
    at __awaiter (C:\SSO365\node_modules\office-addin-sso\lib\configure.js:2:12)
    at C:\SSO365\node_modules\office-addin-sso\lib\configure.js:182:95
    at ChildProcess.exithandler (child_process.js:299:7)
    at ChildProcess.emit (events.js:315:20)
    at maybeClose (internal/child_process.js:1048:16)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:17100) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:17100) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

TCourtneyOwen commented 3 years ago

@xstos Thanks for reporting this issue. Let me see if I can repro this issue and get back to you.

-Courtney

TCourtneyOwen commented 3 years ago

@xstos While I don't hit the precise error you are hitting, I do hit another error and it may be related. I suspect there has been a change in the Graph API that we use for configuring the SSO application that has broken our tooling:

(node:26940) UnhandledPromiseRejectionWarning: Error: Unable to determine if user is tenant admin: Bad Request({ "error": { "code": "Request_BadRequest", "message": "Invalid object identifier 'members'.", "innerError": { "date": "2021-02-01T21:03:04", "request-id": "898cee28-1d6d-484b-9263-075dc17598f9", "client-request-id": "898cee28-1d6d-484b-9263-075dc17598f9" } } })

I will see if I can determine the problem and get a fix out for it soon

-Courtney

xstos commented 3 years ago

Cool. I'm here to test if need be :)

TCourtneyOwen commented 3 years ago

It looks like the API for getting the directory members has changed: https://docs.microsoft.com/en-us/graph/api/directoryrole-list-members?view=graph-rest-1.0&tabs=http. It now requires an id, so our current az rest call to ""az rest -m get -u https://graph.microsoft.com/v1.0/directoryRoles/members --headers \"Content-Type=application/json\"" is invalid as we don't specify an id.

TCourtneyOwen commented 3 years ago

Aha! I figured out the real crux of the problem for the issue I am seeing and that others have reported. The directory roles names have changed and they no longer use the naming convention "Company Administrator" for a tenant admin - it's now called "Global Administrator." Hence, when we try to get the id associated with "Company Administrator", we get undefined which cause subsequent configuration of the tenant admin to fail

TCourtneyOwen commented 3 years ago

@xstos You can try apply this very simple change in node_module\offlce-addin-sso\lib\configure.js and see if things work for you: https://github.com/OfficeDev/Office-Addin-Scripts/pull/376

xstos commented 3 years ago

No dice

i stuck a console.log to see what was in the JSON at position 3324 (pasted below).

exports.logoutAzure = logoutAzure;
function promiseExecuteCommand(cmd, returnJson = true, expectError = false) {
    return __awaiter(this, void 0, void 0, function* () {
        return new Promise((resolve, reject) => {
            try {
                childProcess.exec(cmd, { maxBuffer: 1024 * 102400 }, (err, stdout, stderr) => __awaiter(this, void 0, void 0, function* () {
                    if (err && !expectError) {
                        reject(stderr);
                    }
                    let results = stdout;
                    if (results !== '' && returnJson) {
                        console.log(results); //<=== added
                        results = JSON.parse(results);
                    }
                    resolve(results);
                }));
            }
            catch (err) {
                reject(err);
            }
        });
    });
}

It appears to be complaining about this entry

{
        "DisplayName":  "NTFS Undelete 3.0.20.1104",
        "DisplayVersion":  null,
        "Publisher":  "Copyright c 2015 eSupport.com  All Rights Reserved",
        "InstallDate":  "20210111"
    },

The space between eSupport.com and All is suspect. It shows up as two spaces but an online unicode detector showed it as a regular space. Not sure.

xstos commented 3 years ago

In the meantime, instead of maintaining yeoman, y'all could just create a set of empty boilerplate addins on github that people can fork off of...

akrantz commented 3 years ago

The starting templates are in separate github repositories. You can clone them and start with them.

However, be sure to customize the manifest and use a unique id by running: npx office-addin-manifest modify --guid --displayName <name>

Here are a few of them: Basic: https://github.com/OfficeDev/Office-Addin-TaskPane React: https://github.com/OfficeDev/Office-Addin-TaskPane-React SSO: https://github.com/OfficeDev/Office-Addin-TaskPane-SSO

The yo-office branch is what is used when you run yo office. The master branch may have more recent changes which should work. We're constantly working to keep these templates updated.

xstos commented 3 years ago

I'd forgotten about those. My bad.

TCourtneyOwen commented 3 years ago

@xstos What is the the DisplayName specified in the manifest for your SSO project? I am wondering if this might be the source of the problem.

xstos commented 3 years ago

@TCourtneyOwen I don't have one? I just ran yeoman and then npm run configure-sso and it threw the error at the top of the thread.

xstos commented 3 years ago

Oh sorry I guess it's this <DisplayName DefaultValue="SSO365"/>

TCourtneyOwen commented 3 years ago

The project should have a manifest in it at the root of the project - Yo Office always creates a manifest with the project. If there is no manifest, this would most surely be the root of the problem, as configure-sso uses that DisplayName when creating the SSO application in Azure:

const applicationJson: Object = await configure.createNewApplication(manifestInfo.displayName, port.toString(), userJson);

xstos commented 3 years ago

Sorry I had put it at the beginning of the thread and I assumed I'd forgotten something. It's

<DisplayName DefaultValue="SSO365"/>

It's just the default manifest after running yeoman. No extra shenanigans...

TCourtneyOwen commented 3 years ago

Based on the output you pasted above it would seem the issue is happening in createNewApplication, as this is the first phase of the config process, and I don't see any other output logging indicating this phase succeeded. Here's the content of the method:

export async function createNewApplication(ssoAppName: string, port: string, userJson: Object): Promise

OfficeDev / generator-office

UnhandledPromiseRejectionWarning: SyntaxError: Unexpected token in JSON at position 3324 #599

Prerequisites

Expected behavior

Current behavior

Steps to Reproduce

Context

Failure Logs