Expands the secure_rel utility to handle elastic domains #1565

joelgriffith · 2019-02-14T19:07:12Z

Summary

Hey friends 👋, this is a fun one to fulfill the requirements of elastic/kibana#30846. In short, we want to see where we're getting traffic from in Elastic products so we can better cater to those users. To that end, I've altered get_secure_rel_for_target to allow it to be passed an href param. It will check if the domain (elastic.co) is part of the href and omit the noreferrer attribute.

I searched endlessly for a util to get just the parent domain name to no avail. They either didn't work, were clunky, or had some interesting side-effects. Thus, I've added a small utility in the services are for doing checks. It has a good amount of tests, including edgecases, for seeing if a string is a URL with the domain of elastic.co, including multiple subs.

Please let me know if I'm missing anything, this is my first EUI PR!

Checklist

~~[ ] This was checked in mobile~~
This was checked in IE11
~~[ ] This was checked in dark mode~~
~~[ ] Any props added have proper autodocs~~
~~[ ] Documentation examples were added~~
A changelog entry exists and is marked appropriately
This was checked for breaking changes and labeled appropriately
Jest tests were updated or added to match the most common scenarios
~~[ ] This was checked against keyboard-only and screenreader scenarios~~
~~[ ] This required updates to Framer X components~~

joelgriffith · 2019-02-14T19:08:54Z

src/services/security/get_secure_rel_for_target.ts

@@ -2,31 +2,37 @@
 * Secures outbound links. For more info:
 * https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vulnerability-ever/
 */


Interface is expanded here. Uses a single object argument versus trying to add another argument since that felt weird given the optionality of those args. I also tried to consolidate the algo a bit to make it readable, but let me know if that's not the case!

cchaos · 2019-02-14T19:44:48Z

I'd love to review, but have no clue what I'm looking at ;) Tagged @chandlerprall in

chandlerprall · 2019-02-14T22:11:00Z

src/services/url.ts

@@ -0,0 +1,16 @@
+const isElasticDomain = /(https?:\/\/(.+?\.)?elastic\.co(\/[A-Za-z0-9\-\._~:\/\?#\[\]@!$&'\(\)\*\+,;\=]*)?)/g;


I confirmed that this positively matches against all URLs in the elastic.co sitemaps. A bit hesitant that it doesn't match against extended character sets (unicode), but we don't use those in our urls at this time.

chandlerprall

Needs an update to CHANGELOG.md describing the modification.

chandlerprall · 2019-02-14T22:11:41Z

src/services/url.ts

@@ -0,0 +1,16 @@
+const isElasticDomain = /(https?:\/\/(.+?\.)?elastic\.co(\/[A-Za-z0-9\-\._~:\/\?#\[\]@!$&'\(\)\*\+,;\=]*)?)/g;


This does not match if there is a querystring with no path, e.g. https://elastic.co?a=b

Good catch, addressed!

…-on-elastic-domains

chandlerprall

Changes LGTM, tested the new function locally

* Expands the getSecureRelForTarget utility to let referrer's pass-through to elastic.co domains.

This PR changes the behavior of the EUILink component to apply rel="noreferrer" universally for all external links, as it's considered a best practice. Prior to this PR, there was an exception for elastic.co domains, added in #1565. As this behavior is no longer required, we're opting to remove the exception.

* [EuiToolTip] Add `repositionOnScroll` prop (#6515) * Add `repositionOnScroll` prop and event listener * [Docs] Add fixed tooltip example + add auto-focus behavior for keyboard users * changelog * Updated changelog. * 72.2.0 * Updated documentation. * [EuiDataGrid] Fix nested interactive controls axe error (#6517) * Fix nested interactive controls error in column visibility control * Restore mouse experience by making switch "label" draggable - this (almost) restores previous draggable functionality - the column name and handle are both mouse draggable, while the switch is not as it is its own interactive thing * Fix nested interactive controls error in column sorting control - the X button and sort button group should not be draggable as they are interactive * Improve mouse experience by making column name draggable - The x button and sort button group are interactive and cannot be draggable, but the column name/token can be + tweak padding to belong to the column name, for extra grabbable area * [opinionated] Fix multiple CSS styles to actually work correctly - whether or not these style changes are wanted will have to be approved by a designer * changelog * Improve Kibana upgrade docs (#6518) - add table of contents - order steps more generally in chronological order so it's easier to follow vs jumping around the page - update recommendations and general context * [Docs] Update `EuiConfirmModal` examples (#6519) * [Docs] Update `EuiConfirmModal` examples * Adding a width * Update src-docs/src/views/modal/confirm_modal.tsx Co-authored-by: florent-leborgne <florent.leborgne@elastic.co> Co-authored-by: florent-leborgne <florent.leborgne@elastic.co> * [EuiToolTip] Enforce only one visible tooltip at a time (#6520) * [misc cleanup] Group relative imports by general concept - keep parent services together, keep tooltip-specific imports together * Add tooltip manager that hides all other tooltips when a new tooltip is shown * Write Cypress E2E tests for multiple tooltip behavior * Changelog * Unit tests for control of internal tooltip visibility state via `ref` (#6522) * [Docs] Fix code snippet (#6526) * Added a11y specs for EuiNotificationEvent, EuiPageHeader, EuiPortal (#6524) * Added a11y specs for EuiNotificationEvent, EuiPageHeader, EuiPortal * Added a11y and keyboard specs for EuiNotificationEvent. * Added page header a11y tests. * Added a11y and keyboard specs for EuiPortal. * Simplifying Portal test, adding breadcrumb and tabs to Page Header. * Simplified two components, added a state check to EuiNotification. * Further reducing the EuiPortal specs to test just that one component. * update i18ntokens * Updated changelog. * 73.0.0 * Updated documentation. * Check that the `upstream` remote is correctly set before proceeding in release script (#6528) * [EuiModalHeaderTitle] Remove automatic detection of title contents in favor of always wrapping a `h1`, configurable via new `component` prop (#6530) * Use a `component` prop for EuiModalHeaderTitle tag wrapper instead of trying to detect child types - this is because Kibana's `<FormattedMessage>` component (which only outputs a string) is incorrectly not getting styles applied to it * Add prop unit tests + convert tests to RTL while we're here * Allow `EuiConfirmModal`s to override title component tag as well if necessary * Changelog * tweak changelog copy a bit more * [Docs EuiTable] Replace all the demos using `data_store` with `faker-js` (#6521) * Replacing `data_store` with `faker-js` * Converted `actions.js` to `tsx` * Converted `auto.js` to `.tsx` * Converted `expanding_rows.js` to `*.tsx` * Converted `footer.js` to `*.tsx` * Converted `mobile.js` to `*.tsx` * Converted `selection.js` to `*.tsx` * Converted `sorting.js` to `*.tsx` * Fix typing/`any` usages - provide correct EuiBasicTable typescript usages/definitions * Clean up divs and Fragments * Simplify & clean up delete selected items code + remove it entirely from `footer` and `expanding_rows` examples - those demos really don't need that behavior at all * Remove unnecessary `renderStatus` abstraction - not being used in multiple places, so no need to DRY it out * Fix incorrectly rendering mobile names + DRY out `renderStatus` - not totally clear to me why those examples have such complex mobile examples + add a `mobileOptions.only` example in the `mobile` demo to make up for removed examples * Remove unused `sortable` - there's no sorting being passed to the table, so it does nothing * Clean up table layout example - remove need for randomly generated group IDs - simplify var names - use value passed directly from `onChange` instead of using a `.find()` - use flex group for button toggles instead of `&emsp;`s * Clean up emoji flag logic - move `getEmojiFlag` util to bottom of the file - it's not relevant to anything table-related and shouldn't be the first thing a dev has to read/parse - remove unnecessary extra flag/countryCode() call * Remove unnecessary data length abstraction/logic - if `userLength` isn't being used anywhere else, just hard code it in - no need for a var - remove unnecessary `filteredUsers` logic + remove unnecessary `RIGHT_ALIGNMENT` import * Organize table/component logic by concept - move `columns` out to static init where logical - add headings to sort out manual pagination/sorting logic where it isn't necessarily relevant to the demo (but aids in visual output) - add comment to `findUsers` to more clearly explain what it's doing - remove unnecessary abstractions for longer files * Removed `getEmojiFlag` * Remove lodash `uniqBy` dependency - replace one instance with a `Set` example for uniqueness * Tweak new location column to truncate - this column is now fairly long compared to before - let's restore the previous table visual apperances by truncating * Standardize inconsistent link behavior - it doesn't really make sense to have a last name be a link as opposed to a username - some demos were doing this, some weren't, so standardize the rendering - remove actual external links to github - most of the newly random usernames aren't valid in any case Co-authored-by: Constance Chen <constance.chen.3@gmail.com> * Removed domain check from EuiLink (#6535) This PR changes the behavior of the EUILink component to apply rel="noreferrer" universally for all external links, as it's considered a best practice. Prior to this PR, there was an exception for elastic.co domains, added in #1565. As this behavior is no longer required, we're opting to remove the exception. * [EuiBasicTable] Fix row heights jumping when actions are disabled (#6538) * [EuiBasicTable] Fix row heights jumping when actions are disabled * Update snapshots * changelog * Adding EuiModal callout for H1 rendering. (#6497) * Adding EuiModal callout for H1 rendering. * Updating text to be more concise and clear. * Updating documentation for default H1 wrapper on EuiModalHeaderTitle. * Removing example callout, added explanation to component type definition. * Removed extra H1 tags from Docs examples. * Removed H1 from EUIModal component and a11y specs. * Update checklist template to recommend `@default` jsdoc usage (#6541) * [Emotion] Convert EuiBasicTable (#6539) * [tech debt] convert `useEuiTheme` tests to RTL `renderHook` - which is generally a nicer API than the one I yolo'd * [tech debt] Add more missing unit tests for `useEuiTheme` * [tech debt] write basic unit test for `withEuiTheme` * Add new `RenderWithEuiTheme` render prop util * Convert `tbody` loading styles to Emotion - I opted not to create a top-level component for this due to the very limited styles being applied, and due to HOC/theme access shenanigans * Fix error/empty states not rendering loading styles - by only rendering one `<tbody>`, not multiple * Write basic `loading` test + switch `render` to RTL * [extra] Massive clean up of EuiBasicTable unit tests - switch to RTL totally (shallow was not handling the new render prop well) - DRY out various repeated props - stop use snapshots for every single test - use specific assertions instead. For visual rendering for various prop combos, we should use Storybook - leave snapshots in for two specific render tests - barebones & kitchen sink props * Delete scss files * Add `shouldRenderCustomStyles` test * changelog * Add affordance for reduced motion media query - this matches how EuiProgress behaves + clean up animation shorthand * Add CSS workaround/fix for visual Safari bug - apparently `position: relative` on the parent and not on the `tbody` was a cross-browser fix :( * [EuiResizablePanel] Added tabindex prop to EuiResizablePanel for keyboard accessibility. (#6534) * Added tabindex prop to EuiResizablePanel for keyboard accessibility. * Added unit test and corrected tabIndex type. * Renaming tabindex to tabIndex for consistency in docs. * Renaming test description. Removing changelog. Co-authored-by: Jason Stoltzfus <jastoltz24@gmail.com> Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co> Co-authored-by: florent-leborgne <florent.leborgne@elastic.co> Co-authored-by: Trevor Pierce <1Copenut@users.noreply.github.com> Co-authored-by: Bree Hall <briannajdhall@gmail.com>

Expands the secure_rel utility to handle elastic domains

3552092

joelgriffith commented Feb 14, 2019

View reviewed changes

joelgriffith mentioned this pull request Feb 14, 2019

Remove noreferrer rel value from links to official Elastic docs elastic/kibana#30846

Closed

joelgriffith requested review from pugnascotia and cchaos February 14, 2019 19:39

cchaos requested review from chandlerprall and removed request for cchaos February 14, 2019 19:44

chandlerprall reviewed Feb 14, 2019

View reviewed changes

chandlerprall suggested changes Feb 14, 2019

View reviewed changes

joelgriffith added 3 commits February 14, 2019 14:15

Adjusting regex when no / but a ? is followed

cf475f9

Merge remote-tracking branch 'origin/master' into chore-no-noreferrer…

afb5de1

…-on-elastic-domains

CHANGELOG.md

23bfe92

chandlerprall approved these changes Feb 14, 2019

View reviewed changes

joelgriffith merged commit 99b52fd into master Feb 14, 2019

This was referenced Feb 15, 2019

Upgrade EUI to 6.10.2 elastic/kibana#31272

Merged

Upgrade EUI to 7.1.0 elastic/kibana#31282

Merged

Upgrade to EUI 7.1.0 elastic/kibana#31304

Merged

nreese mentioned this pull request Feb 15, 2019

Chore secure rel in markdown elastic/kibana#31314

Merged

snide deleted the chore-no-noreferrer-on-elastic-domains branch March 5, 2019 21:05

Shigawire pushed a commit to Shigawire/eui that referenced this pull request May 10, 2019

Expands the secure_rel utility to handle elastic domains (elastic#1565)

6d46358

* Expands the getSecureRelForTarget utility to let referrer's pass-through to elastic.co domains.

kobelb mentioned this pull request Dec 11, 2019

Harden default Kibana security headers elastic/kibana#52809

Closed

JasonStoltz mentioned this pull request Jan 18, 2023

Removed domain check from EuiLink #6535

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expands the secure_rel utility to handle elastic domains #1565

Expands the secure_rel utility to handle elastic domains #1565

joelgriffith commented Feb 14, 2019

joelgriffith Feb 14, 2019 •

edited

Loading

cchaos commented Feb 14, 2019

chandlerprall Feb 14, 2019 •

edited

Loading

chandlerprall left a comment

chandlerprall Feb 14, 2019

joelgriffith Feb 14, 2019

chandlerprall left a comment

		@@ -0,0 +1,16 @@
		const isElasticDomain = /(https?:\/\/(.+?\.)?elastic\.co(\/[A-Za-z0-9\-\._~:\/\?#\[\]@!$&'\(\)\\+,;\=])?)/g;

Expands the secure_rel utility to handle elastic domains #1565

Expands the secure_rel utility to handle elastic domains #1565

Conversation

joelgriffith commented Feb 14, 2019

Summary

Checklist

joelgriffith Feb 14, 2019 • edited Loading

Choose a reason for hiding this comment

cchaos commented Feb 14, 2019

chandlerprall Feb 14, 2019 • edited Loading

Choose a reason for hiding this comment

chandlerprall left a comment

Choose a reason for hiding this comment

chandlerprall Feb 14, 2019

Choose a reason for hiding this comment

joelgriffith Feb 14, 2019

Choose a reason for hiding this comment

chandlerprall left a comment

Choose a reason for hiding this comment

joelgriffith Feb 14, 2019 •

edited

Loading

chandlerprall Feb 14, 2019 •

edited

Loading