search

OfficeDev / office-js

A repo and NPM package for Office.js, corresponding to a copy of what gets published to the official "evergreen" Office.js CDN, at https://appsforoffice.microsoft.com/lib/1/hosted/office.js.
https://learn.microsoft.com/javascript/api/overview
Other
670 stars 96 forks source link

Excel for Windows: mixed store-type add-in fails attaching #3004

Open wh1t3cAt1k opened 1 year ago

wh1t3cAt1k commented 1 year ago

Your Environment

Steps to reproduce

  1. Open attached file.

bug-addin.xlsx

  1. Observe dialog:

image

  1. Click "Allow and Continue"

Actual result

image

Expected result

Add-in loads properly.

Context

The file's webextension1.xml contents specify two store types:

image

  1. How can this situation be reached? Is it a case when the add-in was installed from the store, but then switched to an admin-based deployment? Or vice versa, first from admin-based deployment, then somehow gets installed from the store?
  2. Since there is an alternate reference to AppSource store type, why doesn't it fall back to using it successfully if the catalogue is not accessible?
  3. What "catalogue" is it trying to connect to?
  4. Is it a normal situation with two different store types in the xml?

See the discussion of a similar problem with side-loaded add-ins https://github.com/OfficeDev/office-js/issues/2878

cc @4ttt

We desperately need a transparent mechanism of automatic/manual selection of how the add-ins are accessed, with intuitive fallback rules. The UI should guide the user into the pit of success or at least have meaningful fallback rules.

ghost commented 1 year ago

Thank you for letting us know about this issue. We will take a look shortly. Thanks.

wh1t3cAt1k commented 1 year ago

We would also appreciate a proper documentation of what different store / storeType values mean.

I would also like to highlight the fact that the only way to fix this issue is to use the Document Inspector, which makes the add-in use all local state, sometimes this includes important settings. 🥴

RuizhiSunMS commented 1 year ago

@jargil Looks it's related with your area?

davidchesnut commented 1 year ago

Hi @wh1t3cAt1k, you can find documentation about the store/storeType values in Automatically open a task pane with a document. It looks like the store="omex" is incorrect and should actually be something like store="en-us". EXCatalog would be for central deployment and it would look for the catalog in the tenant where it was deployed.

Let me see if I can get some attention on this issue, and the other one.

Cheers, David

slabereemsft commented 1 year ago

Hi @wh1t3cAt1k, I'm taking a quick look at your issue while the correct subject matter expert is out.

  1. How can this situation be reached? The WebExtension1.XML that you've provided (inside the XLSX file) feels like its been hand edited or there is a bug that we should figure out how to reproduce. You should be seeing something similar to this:

The way that users would get into the state that I have above is that their IT admin would deploy the add-in to them via Centralized Deployment and they would use the one from their Admin Managed tab (not the one from the store).

  1. Why isn't fallback working as expected? Generally, our stance is towards security so I'd expect that if it can't reach the store then it would conservatively not load the add-in.

  2. The documentation for our catalogs is here - https://learn.microsoft.com/en-us/openspecs/office_standards/ms-owexml/d4081e0b-5711-45de-b708-1dfa1b943ad1

  3. The situation is normal especially in organizations where the store is disabled. The thing that's unusual from what I've seen are the two items bolded above.

Does that help?

wh1t3cAt1k commented 1 year ago

@slabereemsft thank you for the clarification.

Regarding the bug in bold, it was most certainly not edited by hand. We would suggest and be grateful if you could review how different platforms work with store/storeType, I suspect some inconsistency might be at play - I think we have seen such "spoilage" in mixed usage scenarios where the same file would be periodically open in Online, Excel for Mac, and Excel for Windows.

It may or may not also be linked to what is describe in this bug: https://github.com/OfficeDev/office-js/issues/2876#issuecomment-1321500747

====

Generally, our stance is towards security so I'd expect that if it can't reach the store then it would conservatively not load the add-in.

(asking in good conscience, I want to understand) - how does the absence of automatic fallback to the alternative reference contribute to security? I mean, if it doesn't work automatically, what's even the point of having alternateReferences?

In general I understand the security argument. At the same time, I would also argue that in some cases it enters into a conflict with the goal of reasonable UX.

A good example of that (not necessarily relevant to this ticket, just a convenient mention) is a generic prompt "this add-in comes from the office store", and the user doesn't even know which it is, they click "Allow" only to see a cryptic error message about not being able to reach the catalogue. For Excel users, many of whom are non-technical, instances like that are a real jump-scare, and highly confusing!

Overall, I would be glad if you reviewed this policy on your end and enabled automatic fallbacks to alternate reference in case one of them is unreachable. cc @gmichaud

davidchesnut commented 1 year ago

Hi @wh1t3cAt1k, the fallback won't work in your case because it is using a GUID. It needs to use the shorter asset id, such as this one for Script Lab. So we need to figure out what caused that entry to be incorrect. <we:reference id="wa104380862" version="1.1.0.0" store="en-US" storeType="OMEX"/><we:alternateReferences/>

I'd like to try to repo this behavior. What's the name of your add-in on AppSource (or I can look it up by asset id). Do you just insert the add-in from AppSource, open the file, and see the incorrect web extension entries?

Thanks!

wh1t3cAt1k commented 1 year ago

@davidchesnut thank you for quick response!

Yes, I have been talking in general how the fallback should work in our opinion. Can you confirm that it would have worked if not for the incorrect reference entry?

The add-in is called Velixo.

Regarding the replication steps, I am sorry that we don't have clear ones, they are likely not super simple.

What I know for sure is that the file in question is being used by multiple organisation members, all on different platforms - sometimes it's open on the web, sometimes on a Mac, sometimes on Windows Desktop.

The situation is further complicated by the fact that it might have been accidentally opened by developers who use sideloading of a dev manifest. But we generally try to avoid that due to the "poisoning" behaviour, another UX issue that we have reported elsewhere: https://github.com/OfficeDev/office-js/issues/2878 (for that one, we'd like Office ideally to be able to recognize it's the same add-in)

davidchesnut commented 1 year ago

Okay, I'm seeing some different behavior, but still odd. I can insert the add-in in Excel on the web, but not in Excel on Windows. Although the webextension entries look correct. Let me follow up with @slabereemsft and team to look into this more. Thanks!

slabereemsft commented 1 year ago

Regarding security - We have a capability in the store to quickly withdraw add-ins that have turned malicious and so if we have a store add-in and can't talk to the store, we err on the side of caution. Our telemetry hasn't flagged this as a major scenario and hence why we haven't revisited this question.

Automatic fallback - So the alternateReferences tag was originally spec'ed as a fallback mechanism but it was never used in that way (likely for the reason above). Recently, we've been using it for a scenario where we needed to detect that add-ins coming from Centralized Deployment and from the store were in fact the same one. The scenario is that your IT admin deploys the add-in to part of the company but not all and then you share the document with the add-in embedded to someone who doesn't have it.

File Corruption - Thanks for letting me know that this wasn't hand edited. I'm following up if we have had any related issues in the past. If you do discover the repro steps, let us know.

wh1t3cAt1k commented 1 year ago

(cc @gmichaud who was a witness to the new case)

First of all, regarding file corruption, a suspiciously similar issue is https://github.com/OfficeDev/office-js/issues/3076 ... Now, to the more important fundamental pain...

@slabereemsft @davidchesnut do I understand it correctly that automatic fallback, therefore, will start working with alternateReferences at least for the centralised deployment / store issue? Is there any ETA for this to go live?

I wish the user experience was more transparent around this. We literally just had another crazy support request with a very common scenario - please hear out our story since it's one of our top support requests and is a shadow over the reputation of our product.

Initially Velixo was deployed centrally for this user's organisation.

Though the person claims to have accessed the document using his work account, they started seeing an "add-in error" (Excel Desktop for Mac, latest version) right after switching to a new computer.

image

So, as a workaround, they disabled centralised deployment and reinstalled the product from the AppSource store.

All their old documents stopped working 🤡 I think it's because they were still "attached" to the centrally deployed version of the add-in.

We spent an hour (!) with him figuring things out on a call; even then, he couldn't self-resolve the issue, because there is no Document Inspector on a Mac where he could remove the add-in. However, even using Document Inspector cannot be seriously considered a satisfactory workaround, because the add-in loses all its internal add-in scoped state (including the defined connections and user settings that we store there...).

The user is a CIO with years of technical / coding experience, very smart guy, and he could not figure it out himself. Imagine a non-technical Excel user who our product is oriented towards.

This, I think, is another data point that illustrates our opinion that the user experience around "add-in poisoning" and the inability to consistently figure out the add-in's identity regardless of the deployment channel (central, manifest, store) is broken.

TLDR, what we need is consistent add-in's identity that works seamlessly when the document is passed between, say, developers (using a manifest), customers (using central deployment OR store add-in), and our support team (using the store version of the add-in).

If you think about it, the flow between customers and support team is very common.

davidchesnut commented 1 year ago

Assigning to Sean. @wh1t3cAt1k just a quick update. I'm not longer able to repro the problem. I can insert the Velixo add-in in Excel on Windows and it works fine. So something changed.

wh1t3cAt1k commented 1 year ago

Hi @wh1t3cAt1k, the fallback won't work in your case because it is using a GUID. It needs to use the shorter asset id, such as this one for Script Lab.

@davidchesnut apologies that I missed that bit in your response.

Am I correct to understand that you mean the element in our add-in's manifest?

It's currently a GUID indeed: 9354f1a0-2841-46a9-b37a-3cdaefcebd94

If I understood you correctly, what would be the syntax for us to ensure automatic fallback between store/central deployment channels?

If it's something else, please let me know how we can ensure this is enabled.

I looked up here: https://learn.microsoft.com/en-us/javascript/api/manifest/id?view=excel-js-preview and the requirements specify that it needs to be a GUID.

davidchesnut commented 1 year ago

Hi @wh1t3cAt1k, in this case the id is being embedded in the OOXML, and follows the OOXML rules (not manifest xml schema). The full list of possible id values is explained in this document Automatically open a task pane with a document.

My statement on using alternateReferences was incorrect. Sorry about that. Please refer to Sean's guidance posted above.