Skip to content

Commit

Permalink
feat(theming) add custom component style rendering (#5812)
Browse files Browse the repository at this point in the history

Co-authored-by: Caleb Pollman <[email protected]>
  • Loading branch information
dbanksdesign and calebpollman authored Sep 19, 2024
1 parent 557c08e commit 0ddeea9
Show file tree
Hide file tree
Showing 15 changed files with 374 additions and 102 deletions.
29 changes: 29 additions & 0 deletions .changeset/few-houses-grow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
"@aws-amplify/ui-react": minor
"@aws-amplify/ui": minor
---

feat(theming) add custom component style rendering

```jsx
const customComponentTheme = defineComponentTheme({
name: 'custom-component',
theme(tokens) {
return {
color: tokens.colors.red[10]
}
}
});

export function CustomComponent() {
return (
<>
<View className={customComponentTheme.className()}>
</View>
// This will create a style tag with only the styles in the component theme
// the styles are scoped to the global theme
<ComponentStyle theme={theme} componentThemes=[customComponentTheme] />
</>
)
}
```
1 change: 1 addition & 0 deletions packages/react/__tests__/__snapshots__/exports.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ exports[`@aws-amplify/ui-react/internal exports should match snapshot 1`] = `

exports[`@aws-amplify/ui-react/server exports should match snapshot 1`] = `
[
"ComponentStyle",
"ThemeStyle",
"createComponentClasses",
"createTheme",
Expand Down
53 changes: 53 additions & 0 deletions packages/react/src/components/ThemeProvider/ComponentStyle.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import * as React from 'react';
import { WebTheme, createComponentCSS } from '@aws-amplify/ui';
import {
BaseComponentProps,
ElementType,
ForwardRefPrimitive,
Primitive,
PrimitiveProps,
} from '../../primitives/types';
import { primitiveWithForwardRef } from '../../primitives/utils/primitiveWithForwardRef';
import { BaseComponentTheme } from '@aws-amplify/ui';
import { Style } from './Style';

interface BaseComponentStyleProps extends BaseComponentProps {
/**
* Provide a server generated nonce which matches your CSP `style-src` rule.
* This will be attached to the generated <style> tag.
* @see: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/style-src
*/
nonce?: string;
theme: Pick<WebTheme, 'name' | 'breakpoints' | 'tokens'>;
componentThemes: BaseComponentTheme[];
}

export type ComponentStyleProps<Element extends ElementType = 'style'> =
PrimitiveProps<BaseComponentStyleProps, Element>;

const ComponentStylePrimitive: Primitive<ComponentStyleProps, 'style'> = (
{ theme, componentThemes = [], ...rest },
ref
) => {
if (!theme || !componentThemes.length) {
return null;
}

const cssText = createComponentCSS({
theme,
components: componentThemes,
});

return <Style {...rest} ref={ref} cssText={cssText} />;
};

/**
* @experimental
* [📖 Docs](https://ui.docs.amplify.aws/react/components/theme)
*/
export const ComponentStyle: ForwardRefPrimitive<
BaseComponentStyleProps,
'style'
> = primitiveWithForwardRef(ComponentStylePrimitive);

ComponentStyle.displayName = 'ComponentStyle';
90 changes: 90 additions & 0 deletions packages/react/src/components/ThemeProvider/Style.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import * as React from 'react';
import {
BaseComponentProps,
ElementType,
ForwardRefPrimitive,
Primitive,
PrimitiveProps,
} from '../../primitives/types';
import { primitiveWithForwardRef } from '../../primitives/utils/primitiveWithForwardRef';

interface BaseStyleProps extends BaseComponentProps {
cssText?: string;
}

export type StyleProps<Element extends ElementType = 'style'> = PrimitiveProps<
BaseStyleProps,
Element
>;

const StylePrimitive: Primitive<StyleProps, 'style'> = (
{ cssText, ...rest },
ref
) => {
/*
Only inject theme CSS variables if given a theme.
The CSS file users import already has the default theme variables in it.
This will allow users to use the provider and theme with CSS variables
without having to worry about specificity issues because this stylesheet
will likely come after a user's defined CSS.
Q: Why are we using dangerouslySetInnerHTML?
A: We need to directly inject the theme's CSS string into the <style> tag without typical HTML escaping.
For example, JSX would escape characters meaningful in CSS such as ', ", < and >, thus breaking the CSS.
Q: Why not use a sanitization library such as DOMPurify?
A: For our use case, we specifically want to purify CSS text, *not* HTML.
DOMPurify, as well as any other HTML sanitization library, would escape/encode meaningful CSS characters
and break our CSS in the same way that JSX would.
Q: Are there any security risks in this particular use case?
A: Anything set inside of a <style> tag is always interpreted as CSS text, *not* HTML.
Reference: “Restrictions on the content of raw text elements” https://html.spec.whatwg.org/dev/syntax.html#cdata-rcdata-restrictions
And in our case, we are using dangerouslySetInnerHTML to set CSS text inside of a <style> tag.
Thus, it really comes down to the question: Could a malicious user escape from the context of the <style> tag?
For example, when inserting HTML into the DOM, could someone prematurely close the </style> tag and add a <script> tag?
e.g., </style><script>alert('hello')</script>
The answer depends on whether the code is rendered on the client or server side.
Client side
- To prevent XSS inside of the <style> tag, we need to make sure it's not closed prematurely.
- This is prevented by React because React creates a style DOM node (e.g., React.createElement(‘style’, ...)), and directly sets innerHTML as a string.
- Even if the string contains a closing </style> tag, it will still be interpreted as CSS text by the browser.
- Therefore, there is not an XSS vulnerability on the client side.
Server side
- When React code is rendered on the server side (e.g., NextJS), the code is sent to the browser as HTML text.
- Therefore, it *IS* possible to insert a closing </style> tag and escape the CSS context, which opens an XSS vulnerability.
Q: How are we mitigating the potential attack vector?
A: To fix this potential attack vector on the server side, we need to filter out any closing </style> tags,
as this the only way to escape from the context of the browser interpreting the text as CSS.
We also need to catch cases where there is any kind of whitespace character </style[HERE]>, such as tabs, carriage returns, etc:
</style
>
Therefore, by only rendering CSS text which does not include a closing '</style>' tag,
we ensure that the browser will correctly interpret all the text as CSS.
*/
if (cssText === undefined || /<\/style/i.test(cssText)) {
return null;
}

return (
<style
{...rest}
ref={ref}
// eslint-disable-next-line react/no-danger
dangerouslySetInnerHTML={{ __html: cssText }}
/>
);
};

/**
* @experimental
* [📖 Docs](https://ui.docs.amplify.aws/react/components/theme)
*/
export const Style: ForwardRefPrimitive<BaseStyleProps, 'style'> =
primitiveWithForwardRef(StylePrimitive);

Style.displayName = 'Style';
69 changes: 10 additions & 59 deletions packages/react/src/components/ThemeProvider/ThemeStyle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
PrimitiveProps,
} from '../../primitives/types';
import { primitiveWithForwardRef } from '../../primitives/utils/primitiveWithForwardRef';
import { Style } from './Style';

interface BaseStyleThemeProps extends BaseComponentProps {
/**
Expand All @@ -29,65 +30,15 @@ const ThemeStylePrimitive: Primitive<ThemeStyleProps, 'style'> = (
if (!theme) return null;

const { name, cssText } = theme;
/*
Only inject theme CSS variables if given a theme.
The CSS file users import already has the default theme variables in it.
This will allow users to use the provider and theme with CSS variables
without having to worry about specificity issues because this stylesheet
will likely come after a user's defined CSS.
Q: Why are we using dangerouslySetInnerHTML?
A: We need to directly inject the theme's CSS string into the <style> tag without typical HTML escaping.
For example, JSX would escape characters meaningful in CSS such as ', ", < and >, thus breaking the CSS.
Q: Why not use a sanitization library such as DOMPurify?
A: For our use case, we specifically want to purify CSS text, *not* HTML.
DOMPurify, as well as any other HTML sanitization library, would escape/encode meaningful CSS characters
and break our CSS in the same way that JSX would.
Q: Are there any security risks in this particular use case?
A: Anything set inside of a <style> tag is always interpreted as CSS text, *not* HTML.
Reference: “Restrictions on the content of raw text elements” https://html.spec.whatwg.org/dev/syntax.html#cdata-rcdata-restrictions
And in our case, we are using dangerouslySetInnerHTML to set CSS text inside of a <style> tag.
Thus, it really comes down to the question: Could a malicious user escape from the context of the <style> tag?
For example, when inserting HTML into the DOM, could someone prematurely close the </style> tag and add a <script> tag?
e.g., </style><script>alert('hello')</script>
The answer depends on whether the code is rendered on the client or server side.
Client side
- To prevent XSS inside of the <style> tag, we need to make sure it's not closed prematurely.
- This is prevented by React because React creates a style DOM node (e.g., React.createElement(‘style’, ...)), and directly sets innerHTML as a string.
- Even if the string contains a closing </style> tag, it will still be interpreted as CSS text by the browser.
- Therefore, there is not an XSS vulnerability on the client side.
Server side
- When React code is rendered on the server side (e.g., NextJS), the code is sent to the browser as HTML text.
- Therefore, it *IS* possible to insert a closing </style> tag and escape the CSS context, which opens an XSS vulnerability.
Q: How are we mitigating the potential attack vector?
A: To fix this potential attack vector on the server side, we need to filter out any closing </style> tags,
as this the only way to escape from the context of the browser interpreting the text as CSS.
We also need to catch cases where there is any kind of whitespace character </style[HERE]>, such as tabs, carriage returns, etc:
</style
>
Therefore, by only rendering CSS text which does not include a closing '</style>' tag,
we ensure that the browser will correctly interpret all the text as CSS.
*/
if (/<\/style/i.test(cssText)) {
return null;
} else {
return (
<style
{...rest}
ref={ref}
id={`amplify-theme-${name}`}
// eslint-disable-next-line react/no-danger
dangerouslySetInnerHTML={{ __html: cssText }}
nonce={nonce}
/>
);
}
return (
<Style
{...rest}
ref={ref}
cssText={cssText}
nonce={nonce}
id={`amplify-theme-${name}`}
/>
);
};

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { render } from '@testing-library/react';
import * as React from 'react';

import { ComponentStyle } from '../ComponentStyle';
import { createTheme, defineComponentTheme } from '@aws-amplify/ui';

describe('ComponentStyle', () => {
it('does not render anything if no theme is passed', async () => {
// @ts-expect-error - missing props
const { container } = render(<ComponentStyle />);

const styleTag = container.querySelector(`style`);
expect(styleTag).toBe(null);
});

it('does not render anything if no component themes are passed', async () => {
// @ts-expect-error - missing props
const { container } = render(<ComponentStyle theme={createTheme()} />);

const styleTag = container.querySelector(`style`);
expect(styleTag).toBe(null);
});

it('renders a style tag if theme and component themes are passed', async () => {
const testComponentTheme = defineComponentTheme({
name: 'test',
theme(tokens) {
return {
color: tokens.colors.red[100],
};
},
});
const { container } = render(
<ComponentStyle
theme={createTheme()}
componentThemes={[testComponentTheme]}
/>
);

const styleTag = container.querySelector(`style`);
expect(styleTag).toBeInTheDocument();
});
});
1 change: 1 addition & 0 deletions packages/react/src/server.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export { ThemeStyle } from './components/ThemeProvider/ThemeStyle';
export { ComponentStyle } from './components/ThemeProvider/ComponentStyle';
export {
createTheme,
defineComponentTheme,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`@aws-amplify/ui defineComponentTheme should return a cssText function 1`] = `
"[data-amplify-theme="default-theme"] .amplify-test { background-color:pink; border-radius:var(--amplify-radii-small); }
[data-amplify-theme="default-theme"] .amplify-test--small { border-radius:0; }
"
`;

exports[`@aws-amplify/ui defineComponentTheme should return a cssText function that works with custom tokens 1`] = `
"[data-amplify-theme="test"] .amplify-test { background-color:var(--amplify-colors-hot-pink-10); }
"
`;
Loading

0 comments on commit 0ddeea9

Please sign in to comment.