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

Allow passing csp_nonce_assign_key into router #40

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
20 changes: 20 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,26 @@ defmodule MyPhoenixAppWeb.Router do
end
```

### Content Security Policy

Content security policy nonces can be passed into the router to allow usage of strict content security policies throughout an application.
Copy link
Owner

Choose a reason for hiding this comment

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


This can be achieved by passing in a `csp_nonce_assign_key` to the `FunWithFlags.UI.Router` forward. Values for the nonces should be set in the conn assigns before reaching this router.

The value of this can either be a single nonce assign key, or separate assign keys for script and style tags.

For example:

``` elixir
forward "/", FunWithFlags.UI.Router, namespace: "feature-flags", csp_nonce_assign_key: :my_csp_nonce
```

Or:

``` elixir
forward "/", FunWithFlags.UI.Router, namespace: "feature-flags", csp_nonce_assign_key: %{style: :my_style_nonce, script: :my_script_nonce}
```
Copy link
Owner

Choose a reason for hiding this comment

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

These two examples are for Phoenix. It would be good here to provide examples for Plug.

Copy link
Owner

Choose a reason for hiding this comment

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

Also, I suggest to provide full examples, that also show how the nonce values can be generated and set in the assigns and response headers. In other words, it's important to show how to use this end-to-end, rather than just mention it without clear instructions.

Copy link

Choose a reason for hiding this comment

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

I think the instructions from Oban are pretty clear, would you be open to basically lifting this into the docs?

Copy link
Owner

Choose a reason for hiding this comment

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

The way I understand them, the instructions from Oban show how to configure the names of the assign keys for the nonces. They don't show where the nonces are supposed to come from. I suppose they would come from a different plug, but a full example would be helpful.

Copy link

Choose a reason for hiding this comment

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

Good point! I already had a nonce stored in the conn (originally with put_private/3 and then I added it to the assigns as well for Oban). I can work on a full example.

Copy link

Choose a reason for hiding this comment

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

Funny enough, this just popped up in my feed reader. Our solution is different than that.

Copy link

Choose a reason for hiding this comment

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

Sorry for the delay, but I just wrote 2 examples, 1 simple with a single nonce and another more advanced with nonces specific to style and script tags.


## Caveats

While the base `fun_with_flags` library is quite relaxed in terms of valid flag names, group names and actor identifers, this web dashboard extension applies some more restrictive rules.
Expand Down
15 changes: 14 additions & 1 deletion lib/fun_with_flags/ui/router.ex
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ defmodule FunWithFlags.UI.Router do

@doc false
def call(conn, opts) do
conn = extract_namespace(conn, opts)
conn =
conn
|> extract_namespace(opts)
|> extract_csp_nonce_key(opts)
super(conn, opts)
end

Expand Down Expand Up @@ -286,6 +289,16 @@ defmodule FunWithFlags.UI.Router do
Plug.Conn.assign(conn, :namespace, "/" <> ns)
end

defp extract_csp_nonce_key(conn, opts) do
csp_nonce_assign_key =
case opts[:csp_nonce_assign_key] do
nil -> nil
key when is_atom(key) -> %{style: key, script: key}
%{} = keys -> Map.take(keys, [:style, :script])
end

Plug.Conn.put_private(conn, :csp_nonce_assign_key, csp_nonce_assign_key)
end

defp assign_csrf_token(conn, _opts) do
csrf_token = Plug.CSRFProtection.get_csrf_token()
Expand Down
8 changes: 8 additions & 0 deletions lib/fun_with_flags/ui/templates.ex
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,12 @@ defmodule FunWithFlags.UI.Templates do
|> to_string()
|> URI.encode()
end

def csp_nonce(conn, type) do
assign_key = conn.private[:csp_nonce_assign_key][type]
case conn.assigns[assign_key] do
nil -> ""
nonce -> "nonce=\"#{nonce}\""
end
end
end
4 changes: 2 additions & 2 deletions lib/fun_with_flags/ui/templates/_head.html.eex
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
<meta charset="utf-8">
<title>FunWithFlags - <%= @title %></title>

<link rel="stylesheet" href="<%= path(@conn, "/assets/bootstrap.min.css") %>">
<link rel="stylesheet" href="<%= path(@conn, "/assets/style.css") %>">
<link <%= csp_nonce(@conn, :style) %> rel="stylesheet" href="<%= path(@conn, "/assets/bootstrap.min.css") %>">
<link <%= csp_nonce(@conn, :style) %> rel="stylesheet" href="<%= path(@conn, "/assets/style.css") %>">
</head>
2 changes: 1 addition & 1 deletion lib/fun_with_flags/ui/templates/details.html.eex
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,6 @@
</div>
</div>
</div>
<script type="text/javascript" src="<%= path(@conn, "/assets/details.js") %>"></script>
<script <%= csp_nonce(@conn, :script) %> type="text/javascript" src="<%= path(@conn, "/assets/details.js") %>"></script>
</body>
</html>
33 changes: 33 additions & 0 deletions test/fun_with_flags/ui/router_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,39 @@ defmodule FunWithFlags.UI.RouterTest do
end
end

describe "CSP nonce assign key option" do
test "if csp_nonce_assign_key is set, the CSP nonce is rendered in the script and link tags" do
{:ok, true} = FunWithFlags.enable :coconut

csp_opts = [csp_nonce_assign_key: :pineapple]

conn =
conn(:get, "/flags/coconut")
|> Plug.Conn.assign(:pineapple, "mango")
Copy link
Owner

@tompave tompave Aug 18, 2024

Choose a reason for hiding this comment

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

Let's add a comment to explain what's happening here.

Why this can be confusing: manipulating conn here is basically preparing the request data, not the response, because this happens before calling Router.call(...).

Since this library's "server backend" is self-contained in its Router, I guess that here you expect the nonce "mango" to be generated and set in the assigns by a different plug, executed before this FWF router, in the host-application's router.

That's ok, but let's make it clear with a comment.

|> Router.call(Router.init(csp_opts))

assert 200 = conn.status
assert String.contains?(conn.resp_body, ~s{<script nonce="mango"})
assert String.contains?(conn.resp_body, ~s{<link nonce="mango"})
end

test "if csp_nonce_assign_key is set with differing values, the CSP nonce is rendered in the script and link tags" do
{:ok, true} = FunWithFlags.enable :coconut

csp_opts = [csp_nonce_assign_key: %{script: :lemon, style: :melon}]

conn =
conn(:get, "/flags/coconut")
|> Plug.Conn.assign(:lemon, "peach")
|> Plug.Conn.assign(:melon, "apricot")
|> Router.call(Router.init(csp_opts))

assert 200 = conn.status
assert String.contains?(conn.resp_body, ~s{<script nonce="peach"})
assert String.contains?(conn.resp_body, ~s{<link nonce="apricot"})
end
end


# For GET and DELETE
#
Expand Down
16 changes: 16 additions & 0 deletions test/fun_with_flags/ui/templates_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -186,4 +186,20 @@ defmodule FunWithFlags.UI.TemplatesTest do
assert String.contains?(out, ~s{The flag <strong>watermelon</strong> doesn't exist.})
end
end

describe "CSP nonce" do
test "it includes a CSP nonce if provided", %{conn: conn} do
flag = %Flag{name: :avocado, gates: []}

conn =
conn
|> Plug.Conn.put_private(:csp_nonce_assign_key, %{script: :script_nonce, style: :style_nonce})
|> Plug.Conn.assign(:script_nonce, "honeydew")
|> Plug.Conn.assign(:style_nonce, "watermelon")

out = Templates.details(conn: conn, flag: flag)
assert String.contains?(out, ~s{<script nonce="honeydew"})
assert String.contains?(out, ~s{<link nonce="watermelon"})
end
end
end
Loading