OfficeDev / generator-office

Yeoman generator for building Microsoft Office related projects.
https://www.npmjs.com/package/generator-office
MIT License
835 stars 209 forks source link

configure-sso script fails when tenant name has a space #518

Closed corruptmem closed 4 years ago

corruptmem commented 4 years ago

Yeoman configuration:

Node version v12.16.0, yo package 3.1.1, generator-office 1.6.1.

When I run npm run configure-sso, I get the following.

> npm run configure-sso
> office-addin-taskpane-sso@0.0.0 configure-sso <redacted>\SSOTest
> 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
Setting signin audience
Granting admin consent

ERROR: Bad Request({
  "error": {
    "code": "Request_BadRequest",
    "message": "Invalid value specified for property 'replyUrls' of resource 'ServicePrincipal'.",
    "innerError": {
      "request-id": "<redacted>",
      "date": "2020-02-15T12:15:13"
    },
    "details": [
      {
        "target": "replyUrls",
        "code": "InvalidValue"
      }
    ]
  }
})

    at Object.<anonymous> (<redacted>\SSOTest\node_modules\office-addin-sso\lib\configure.js:335:19)
    at Generator.throw (<anonymous>)
    at rejected (<redacted>\SSOTest\node_modules\office-addin-sso\lib\configure.js:4:65)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
(node:7352) 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:7352) [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.

As far as I can tell, the issue appears to be because my tenant name has a space in it.

configure.ts in node package office-addin-so has the following code:

        let azRestCommand: string = fs.readFileSync(defaults.azRestGetOrganizationDetailsCommandPath, 'utf8');
        const tenantDetails: any = await promiseExecuteCommand(azRestCommand);
        const tenantName: string = tenantDetails.value[0].displayName
        const oneDriveReplyUrl: string = `https://${tenantName}-my.sharepoint.com/_forms/singlesignon.aspx`;
        const sharePointReplyUrl: string = `https://${tenantName}.sharepoint.com/_forms/singlesignon.aspx`;

azRestGetOrganizationDetailsCommandPath contains az rest -m get -u https://graph.microsoft.com/v1.0/organization --headers "Content-Type=application/json", which when executed on any tenant I have access to, returns an object with displayName set to a value with a space in it.

It then tries to interpolate that value into the domains in the lines underneath, which causes it to fail.

If I manually override the tenantName to be the value that appears before the .onmicrosoft.com on the initial domain, it all works fine.

TCourtneyOwen commented 4 years ago

Sorry you are having problems. I thought I had submitted a fix for this with this commit: https://github.com/OfficeDev/Office-Addin-Scripts/commit/8e0bd8dc96f19b9f0bd5e965e29e266d8d27537a

Are you on Windows or Mac?

TCourtneyOwen commented 4 years ago

I should be able to get a quick fix in for this on Tuesday when I am back in the office - we are off Monday for a national holiday. Thanks again for bringing this to our attention.

-Courtney

corruptmem commented 4 years ago

@TCourtneyOwen Thanks for the quick response! I'm on Windows 10. I did look at #244 and it doesn't look like it's the same problem as is affecting me. I hacked the script to force the tenantName to be correct and it all works fine once I do that.

TCourtneyOwen commented 4 years ago

@corruptmem I have been swamped with some other work items and am just getting around to look into this. Would you happen to have the hacked script so I can try it out? I am most interested in how you escaped the space in the tenant name in your hacked script.

Thanks,

Courtney

TCourtneyOwen commented 4 years ago

@corruptmemPR with fix for this issue at https://github.com/OfficeDev/Office-Addin-Scripts/pull/260

TCourtneyOwen commented 4 years ago

@corruptmem this issue has been fixed in the SSO project templates. Thanks for bringing this to our attention.

-Courtney