Skip to content

[MOB-8515]: allow-popups-to-escape-sandbox #380

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

Merged
merged 8 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@

# Toggle this to true if you would need to hit our EU APIs.
# IS_EU_ITERABLE_SERVICE=false

# DANGEROUSLY_ALLOW_JS_POPUP_EXECUTION=false
3 changes: 3 additions & 0 deletions react-example/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@

# Convenience variable to automatically set the login email during testing.
# [email protected]

# IS_EU_ITERABLE_SERVICE=false
# DANGEROUSLY_ALLOW_JS_POPUP_EXECUTION=false
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this at the sample app level?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the package is installed via package manager, devs don't have access to modify the .env file of the web-sdk so they need to be able to pass this value in from their own .env. This just serves as an example of how to do that.

3 changes: 3 additions & 0 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ export const RETRY_USER_ATTEMPTS = 0;
const IS_EU_ITERABLE_SERVICE =
process.env.IS_EU_ITERABLE_SERVICE === 'true' ? true : false;

export const dangerouslyAllowJsPopupExecution =
process.env.DANGEROUSLY_ALLOW_JS_POPUP_EXECUTION === 'true' ? true : false;

const US_ITERABLE_DOMAIN = 'api.iterable.com';

const EU_ITERABLE_DOMAIN = 'api.eu.iterable.com';
Expand Down
5 changes: 4 additions & 1 deletion src/inapp/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { by } from '@pabra/sortby';
import {
ANIMATION_DURATION,
dangerouslyAllowJsPopupExecution,
DEFAULT_CLOSE_BUTTON_OFFSET_PERCENTAGE
} from 'src/constants';
import { WebInAppDisplaySettings } from 'src/inapp';
Expand Down Expand Up @@ -286,7 +287,9 @@ const generateSecuredIFrame = () => {
// event handlers on elements in it preventing our custom link handling
iframe.setAttribute(
'sandbox',
'allow-same-origin allow-popups allow-top-navigation'
`allow-same-origin allow-popups allow-top-navigation ${
dangerouslyAllowJsPopupExecution ? 'allow-popups-to-escape-sandbox' : ''
}`
);
/*
_display: none_ would remove the ability to set event handlers on elements
Expand Down
2 changes: 2 additions & 0 deletions webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ function getParsedEnv() {
...env.parsed,
VERSION: version,
IS_EU_ITERABLE_SERVICE: process.env.IS_EU_ITERABLE_SERVICE || false,
DANGEROUSLY_ALLOW_JS_POPUP_EXECUTION:
process.env.DANGEROUSLY_ALLOW_JS_POPUP_EXECUTION || false
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we do a strict check against true given the effect of the changes this value controls?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the webpack config to allow the env variable to be passed in. made the check a const and made it stricter here

};
}

Expand Down
2 changes: 2 additions & 0 deletions webpack.dev.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ function getParsedEnv() {
...env.parsed,
VERSION: version,
IS_EU_ITERABLE_SERVICE: process.env.IS_EU_ITERABLE_SERVICE || false,
DANGEROUSLY_ALLOW_JS_POPUP_EXECUTION:
process.env.DANGEROUSLY_ALLOW_JS_POPUP_EXECUTION || false
};
}

Expand Down
2 changes: 2 additions & 0 deletions webpack.node.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ function getParsedEnv() {
...env.parsed,
VERSION: version,
IS_EU_ITERABLE_SERVICE: process.env.IS_EU_ITERABLE_SERVICE || false,
DANGEROUSLY_ALLOW_JS_POPUP_EXECUTION:
process.env.DANGEROUSLY_ALLOW_JS_POPUP_EXECUTION || false
};
}

Expand Down