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

fix: AuthPluginParent wasn't working when embedded in an iframe #1383

Merged
merged 2 commits into from
Jun 21, 2023

Conversation

mofojed
Copy link
Member

@mofojed mofojed commented Jun 21, 2023

To test:

  1. Start up deephaven-core with PSK set to hello: ./gradlew server-jetty-app:run -Ppsk=hello
  2. Start up Web UI: npm start
  3. Create a table named t in the Web UI at https://localhost:4000:
from deephaven import empty_table
t = empty_table(50).update(["x=i%5", "y=(i*i)%5"])
  1. Open the HTML file in the provided zip.
    iframe-auth-provider-local.zip
  2. Ensure the table appears correctly in the iframe. Click the "Open in new tab" link, and ensure the table opens there correctly as well.

Also tested that going to localhost:4010/?name=t&authProvider=parent directly did not try and use the parent plugin, because it wasn't opened by another window.

- Check for window.opener _or_ window.parent when using AuthPluginParent
- Reorder plugin priority to use AuthPluginParent first if specified
- Tested using an example html page that both embeds and pops open a new tab
@mofojed mofojed requested a review from vbabich June 21, 2023 16:27
@mofojed mofojed self-assigned this Jun 21, 2023
@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

Merging #1383 (7d41761) into main (004ac6c) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1383      +/-   ##
==========================================
+ Coverage   45.58%   45.59%   +0.01%     
==========================================
  Files         506      506              
  Lines       34968    34975       +7     
  Branches     8735     8737       +2     
==========================================
+ Hits        15940    15947       +7     
  Misses      18977    18977              
  Partials       51       51              
Flag Coverage Δ
unit 45.59% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ackages/app-utils/src/components/AuthBootstrap.tsx 93.33% <ø> (ø)
packages/auth-plugins/src/AuthPluginParent.tsx 93.75% <100.00%> (ø)
packages/jsapi-utils/src/MessageUtils.ts 77.77% <100.00%> (+3.30%) ⬆️

vbabich
vbabich previously approved these changes Jun 21, 2023
packages/jsapi-utils/src/MessageUtils.test.ts Outdated Show resolved Hide resolved
- Now it doesn't duplicate the same code
@mofojed mofojed merged commit e23695d into deephaven:main Jun 21, 2023
@mofojed mofojed deleted the 1373-authpluginparent branch June 21, 2023 18:18
@github-actions github-actions bot locked and limited conversation to collaborators Jun 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AuthPluginParent should work when embed is opened in an iframe
2 participants