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

sendToDevice in StopGapWidgetDriver doesn't handle encrypted events properly #24470

Open
Fox32 opened this issue Feb 8, 2023 · 2 comments
Open
Labels
A-Widgets O-Occasional Affects or can be seen by some users regularly or most users rarely S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect

Comments

@Fox32
Copy link
Contributor

Fox32 commented Feb 8, 2023

Steps to reproduce

  1. Create a widget that uses sendToDevice
  2. Send a to device message without encryption:
    widgetApi.sendToDevice( 'com.example.message',
        false,
        { '@user-id':
          { '*':
            { hello: 'world' } 
          }
       }
    )
  3. Send a device message with encryption
    widgetApi.sendToDevice( 'com.example.message',
        true,
        { '@user-id':
          { '*':
            { hello: 'world' } 
          }
       }
    )

Outcome

What did you expect?

Both messages can be received by another device.

What happened instead?

Only the unencrypted event is received.

I already analyzed it a bit more:
The issue is that the code flow for encrypted and unencrypted devices is different. In StopGapWidgetDriver, for unencrypted messages the content parameter of sendToDevice is forwarded as-is together with the event type, but the event type is not forwarded at all for encrypted messages. I could verify that on the receiver side, the message is received without any event type, neither is the content of the message correct, because the payload is not under the content property, but applied to the top level.

I think the correct way would be to construct a new payload object inside StopGapWidgetDriver that has a top level type and all content is placed in content. I could verify that doing it manually works:

widgetApi.sendToDevice( 'com.example.message',
    true,
    { '@user-id':
      { '*':
        { type: 'com.example.message', content: {hello: 'world'} } 
      }
   }
)

Fixing this issue though might be a breaking change for existing widgets that use this API. Especially for Element Call. Element Call itself already takes care of this behavior and would break if the issue is fixed.

Operating system

macOS

Browser information

Chrome

URL for webapp

develop.element.io

Application version

Element version: a26be26-react-2da98a6024ff-js-16672b3d0cad Olm version: 3.2.12

Homeserver

matrix.org

Will you send logs?

No

@dbkr
Copy link
Member

dbkr commented Feb 13, 2023

matrix-org/matrix-spec-proposals#2762 is the spec for this which doesn't explicitly state either way what should happen with encrypted events (although the sending part would suggest they should be forwarded after decryption if the widget is to be unaware of encryption).

@dbkr dbkr added S-Major Severely degrades major functionality or product features, with no satisfactory workaround O-Occasional Affects or can be seen by some users regularly or most users rarely A-Widgets labels Feb 13, 2023
@Fox32
Copy link
Contributor Author

Fox32 commented Feb 13, 2023

@dbkr the related MSC is matrix-org/matrix-spec-proposals#3819, I already added a comment there that this behavior needs to be clarified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Widgets O-Occasional Affects or can be seen by some users regularly or most users rarely S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect
Projects
None yet
Development

No branches or pull requests

2 participants