Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

base64 certificate authentication should not use password parameter in m365 cli [BUG] #113

Open
RRosier opened this issue Oct 13, 2021 · 0 comments
Labels
bug Something isn't working

Comments

@RRosier
Copy link

RRosier commented Oct 13, 2021

Describe the bug
When calling doctor with a Base64Encoded certificate, it is calling the m365 cli with an empty --password option.

This is causing an error since m365 does not require a password when passing a Base64Encoded certificate.

m365 cli login
-p, --password [password]
Password for the user or the certificate. Required when authType is set to password, or when authType is set to certificate and the provided certificate requires a password to open

I generated and retrieve my certificate from Azure KeyVault, which is not giving me a password at all.

To Reproduce
Steps to reproduce the behavior:

  1. call doctor publish with a Base64Encoded certificate
    doctor publish --auth certificate --certificateBase64Encoded $(sharepoint-ad-certificate) --appId $(sharepoint-ad-client-id) --tenant $(sharepoint-ad-tenant-id) --url $(sharepoint-docs-site) --disableTracking --debug

  2. returned error (debug mode)
    DEBUG Running with the following options: {"task":"publish","auth":"certificate","overwriteImages":false,"tenant":"","appId":"","certificateBase64Encoded":"","commandName":"localm365","webUrl":"https://.sharepoint.com/sites/","startFolder":"/home/vsts/work/1/s/Docs/src","startFolderRel":"./src","assetLibrary":"Shared Documents","webPartTitle":"documentation-placeholder","skipPrecheck":false,"skipExistingPages":false,"continueOnError":false,"retryWhenFailed":false,"disableTracking":true,"menu":null,"debug":true,"cleanEnd":false,"cleanStart":false,"outputFolder":"","siteDesign":{"chrome":{"headerLayout":"Compact","headerEmphasis":"Darkest","footerEnabled":false}},"markdown":{"allowHtml":true,"theme":"Light","shortcodesFolder":"./shortcodes"},"multilingual":null,"shortcodesFolder":"./shortcodes","skipPages":false,"skipNavigation":false,"skipSiteDesign":false,"cleanQuickLaunch":false,"cleanTopNavigation":false,"pageTemplate":null,"disableComments":true}
    [07:12:12] Authenticate to M365 with certificate [started]
    DEBUG
    DEBUG Command: localm365 login --authType certificate --appId "
    " --tenant "" --certificateBase64Encoded "" --password
    [07:12:13] Authenticate to M365 with certificate [failed]
    [07:12:13] → undefined
    ERROR: Cannot create property 'context' on string 'Command failed: localm365 login --authType certificate --appId "
    " --tenant "" --certificateBase64Encoded "***" --password
    Error: Error: Unable to decrypt PKCS#8 ShroudedKeyBag, wrong password?

Expected behavior
doctor should call m365 like this
localm365 login --authType certificate --appId "*****" --tenant "***" --certificateBase64Encoded "*****"

Additional context
I would love to contribute to the project, unfortunatly I am currently not able to set-up a linux-based dev environment.
When I look at the code, I believe the bug is located in the authenticate.ts file.

else if (auth === "certificate") {
            await execScript(ArgumentsHelper.parse(`login --authType certificate --appId "${appId}" --tenant "${tenant}" --certificateBase64Encoded "${certificateBase64Encoded}" ${password ? `--password ${password}` : `--password`}`), false, false, [certificateBase64Encoded, password]);
          }

Here we probably should just ommit the --password parameter completely when no password is passed to doctor.

else if (auth === "certificate") {
            await execScript(ArgumentsHelper.parse(`login --authType certificate --appId "${appId}" --tenant "${tenant}" --certificateBase64Encoded "${certificateBase64Encoded}" ${password ? `--password ${password}` : ``}`), false, false, [certificateBase64Encoded, password]);
          }
@RRosier RRosier added the bug Something isn't working label Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant