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

Add toNonceUnstyled functions for CSP nonce support #570

Merged
merged 1 commit into from
Apr 6, 2022

Conversation

jfmengels
Copy link
Contributor

Fixes #569

Happy to make any changes, and to take suggestions for better documentation of the function 🙂

@jonathanmoregard
Copy link

Anything we can do to help get this in @rtfeldman ? :)

@JohanWinther
Copy link

@jfmengels For future-proofing the "API", do you think having a toUnstyledWith : Options -> Html msg -> VirtualDom.Node msg would be a good idea?

@jfmengels
Copy link
Contributor Author

@JohanWinther That depends on what Options is. If it's a record, you'd call it like

Html.toUnstyledWith
  { nonce = Just "nonce id"
  ,  -- ...
  }
  (view model)

I think the API looks okay in that case, but it is not future-proof, because any addition/change you make to that record is a breaking change.

To make it future-proof, you can use the builder pattern:

Html.toUnstyledWith
  (Html.defaultOptions
    |> Html.withNonce "nonce id"
  )
  (model view)

The ergonomics become quite different, and not necessarily idiomatic to the rest of the package. In addition, the Html.Styled module would have a few additional out-of-place functions, or you move the options to a separate module.

The Html List pattern, with toUnstyledWith : List Option -> Html msg -> VirtualDom.Node msg, probably makes a lot more sense here:

Html.toUnstyledWith
  [ Html.nonce "nonce id"
  ]
  (model view)

But then the question becomes: What do we need to future proof for? I haven't followed feature requests for this repo enough, but this seems to be the first option since last the few years that elm-css has been released. So my gut feeling is that this is premature configuration and that we should wait until such a use-case comes around (or that someone mentions an existing one in thread). At that point, we can duplicate the toUnstyled functions for all possible permutations, or add one with options. A breaking change is also possibleand not necessarily out of question.

Anyway. If there is no use-case for a list of options, I don't think it's worth adding. But happy to add it if maintainers wish for it.

@JohanWinther
Copy link

@jfmengels I think you are on to something about not needing to future proof. I was looking at the way elm-ui handles it where a new option for the nonce was the way it was solved there, but of course, like you said, this is the first option in this package.

I'm looking forward to this change, mainly because I'm working at the same company (but different team) as @jonathanmoregard and my team will probably have the same problem to solve in the near future.

Copy link
Owner

@rtfeldman rtfeldman left a comment

Choose a reason for hiding this comment

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

Lovely, thanks @jfmengels! 🎉

@rtfeldman rtfeldman merged commit 85a9516 into rtfeldman:master Apr 6, 2022
@choonkeat
Copy link

choonkeat commented Jun 18, 2022

I think I'm missing something while trying to use Html.Styled.toNonceUnstyled. I'm still getting

Content Security Policy: The page’s settings blocked the loading of a resource at inline (“style-src”)

and traced it down by modifying the elm output js

 	for (var key in attrs)
 	{
 		var value = attrs[key];
+		if (key === 'style') {
+			console.log(value !== 'undefined' ? 'setAttribute' : 'removeAttribute', key, value, domNode);
+			continue;
+		}
 		typeof value !== 'undefined'
 			? domNode.setAttribute(key, value)
 			: domNode.removeAttribute(key);

to see this single line logged

Screenshot 2022-06-18 at 4 45 34 PM

which is from #559

elm-css/src/Css/Global.elm

Lines 108 to 111 in c5ffa8c

|> VirtualDom.node "span"
[ VirtualDom.attribute "style" "display: none;"
, VirtualDom.attribute "class" "elm-css-style-wrapper"
]
VirtualDom.node "span"
[ VirtualDom.attribute "style" "display: none;"
, VirtualDom.attribute "class" "elm-css-style-wrapper"
]

How do I avoid that?

NOTE: I have .elm-css-style-wrapper { display: none; } in my css file, so would not need the inline style attribute

@mostlyobvious
Copy link

According to https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/style-src#violation_cases, inline style attributes (like in <span style="display: none;" class="elm-css-style-wrapper">...</span>) also violate style-src policy which does not allow unsafe-inline.

Current Html.Styled.toNonceUnstyled implementation is not enough as @choonkeat pointed out.

mostlyobvious added a commit to RailsEventStore/rails_event_store that referenced this pull request Jun 22, 2022
Typed CSS is nice and also not needing to think about CSS output was
nice. However it's a dead end with Content-Security-Policy. Our aim with
Browser is to be as compatible with various apps as possible so it was
limiting Browser adoption too much.

rtfeldman/elm-css#569 (comment)
rtfeldman/elm-css#570 (comment)
https://arkency.slack.com/files/U025BA51D/F03LJ1UPZQV/audio_clip__2022-06-22_01_11_39_.m4a

Previous idea with nonce that did not work out:
https://arkency.slack.com/archives/C7B95EW3V/p1655828630714969

Related:
https: //github.com//issues/1346

Co-authored-by: Szymon Fiedler <szymon.fiedler@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CSP nonce support (to be able to protect against XSS attacks)
6 participants