Skip to content

Conversation

@mayagbarnes
Copy link
Collaborator

@mayagbarnes mayagbarnes commented Jun 6, 2023

Describe your changes

Adding Cypress end to end tests for host <-> guest communication, based on the recently refactored HostCommunicationManager.
Previously, we used hostframe.html to manually test sending messages from the host frame and their processing. This PR allows our automated Github Action workflow for Cypress e2e tests to test this functionality by:

  • whitelisting localhost as an allowed origin - ONLY in development mode
  • adds a script/spec to trigger messages from a host frame & check for their results

@mayagbarnes mayagbarnes changed the title [DRAFT] Add e2e tests for host communication [DRAFT] Add host communication e2e tests Jun 6, 2023
@mayagbarnes mayagbarnes added impact:internal PR changes only affect internal code change:chore PR contains maintenance or housekeeping change labels Jun 6, 2023
@mayagbarnes mayagbarnes force-pushed the host-auto-tests branch 2 times, most recently from ab9ee47 to 648ef96 Compare June 22, 2023 02:26
@mayagbarnes mayagbarnes changed the title [DRAFT] Add host communication e2e tests Add host communication e2e tests Jun 22, 2023
@mayagbarnes mayagbarnes marked this pull request as ready for review June 23, 2023 19:35
@mayagbarnes
Copy link
Collaborator Author

Hi @sfc-gh-hpathak 😃
We are looking to add automated testing for our Host <-> Guest Communication messaging (ex: Streamlit as the guest, Community Cloud / SIS as the host), but want to confirm that whitelisting localhost as an allowedOrigin is okay from a security perspective?

@mayagbarnes mayagbarnes force-pushed the host-auto-tests branch 2 times, most recently from 4a74c90 to 33fafdb Compare June 27, 2023 05:44
Copy link
Collaborator

@vdonato vdonato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM aside from one small tweak that needs to be made

I'm also not sure if this warrants a bit more security process. I think it's worth filing an RA to record this change, but it's unlikely that we need to do more than that.

Copy link
Collaborator

@vdonato vdonato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small nits, but aside from those LGTM!

Will have the RA reviewed by EOD today

@mayagbarnes mayagbarnes added the security-assessment-completed Security assessment has been completed for PR label Jul 7, 2023
@mayagbarnes
Copy link
Collaborator Author

For documentation, applicable RA link here

@mayagbarnes mayagbarnes merged commit 503bde1 into develop Jul 11, 2023
@mayagbarnes mayagbarnes deleted the host-auto-tests branch July 11, 2023 22:54
tconkling added a commit to tconkling/streamlit that referenced this pull request Jul 12, 2023
* develop:
  Feature: st.toast (streamlit#6783)
  Bump semver from 5.7.1 to 5.7.2 in /frontend (streamlit#6982)
  Add host communication e2e tests  (streamlit#6806)
  Re-add homepage to package.json (streamlit#6987)
  Remove unnecessary code and Add rm commands to make clean (streamlit#6980)
  Allow setting placeholder for st.selectbox (streamlit#6913)
  Allow setting placeholder for st.multiselect (streamlit#6901)
  Also use bottom padding in embedded mode for chat input (streamlit#6979)
  Feature/st lib (streamlit#6692)
  Remove unused import (streamlit#6977)
  Release 1.24.1 (streamlit#6965)
  Update st.audio/st.video docstrings (streamlit#6964)
  Slightly simplify bug report template (streamlit#6972)
  Fix baseweb warnings by using longhand properties (streamlit#6976)
asmeralt pushed a commit to asmeralt/streamlit that referenced this pull request Sep 29, 2025
Adding Cypress end to end tests for host <-> guest communication, based on the recently refactored HostCommunicationManager.
Previously, we used hostframe.html to manually test sending messages from the host frame and their processing. This PR allows our automated Github Action workflow for Cypress e2e tests to test this functionality by: whitelisting localhost as an allowed origin - ONLY in development mode & adding a script/spec to trigger messages from a host frame & check for their results
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:chore PR contains maintenance or housekeeping change impact:internal PR changes only affect internal code security-assessment-completed Security assessment has been completed for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants