Merge branch 'add-sobelow' into 'master'

Add sobelow

Closes #119

See merge request framasoft/mobilizon!805
This commit is contained in:
Thomas Citharel 2021-01-25 07:45:32 +00:00
commit 8b99ba1cff
22 changed files with 176 additions and 54 deletions

View file

@ -55,8 +55,9 @@ lint-elixir:
- mix deps.get
script:
- export EXITVALUE=0
- mix credo --strict -a || export EXITVALUE=1
- mix format --check-formatted --dry-run || export EXITVALUE=1
- mix credo --strict -a || export EXITVALUE=1
- mix sobelow --config --skip || export EXITVALUE=1
- exit $EXITVALUE
lint-front:

12
.sobelow-conf Normal file
View file

@ -0,0 +1,12 @@
[
verbose: true,
private: false,
skip: true,
router: "lib/web/router.ex",
exit: "low",
format: "txt",
out: "",
threshold: "medium",
ignore: ["Config.HTTPS"],
ignore_files: ["config/dev.1.secret.exs", "config/dev.2.secret.exs", "config/dev.3.secret.exs", "config/dev.secret.exs", "config/e2e.secret.exs", "config/prod.secret.exs", "config/test.secret.exs"]
]

2
.sobelow-skips Normal file
View file

@ -0,0 +1,2 @@
AACA51671C4B3C803ACBCA3FADE84CDE

View file

@ -338,7 +338,7 @@ defmodule Mobilizon.Federation.ActivityPub do
:ok <- maybe_federate(activity) do
{:ok, activity, follower}
else
{:error, err, msg} when err in [:already_following, :suspended] ->
{:error, err, msg} when err in [:already_following, :suspended, :no_person] ->
{:error, msg}
{:different_actors, _} ->

View file

@ -131,7 +131,8 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Actors do
end
end
def follow(%Actor{} = follower_actor, %Actor{} = followed, _local, additional) do
def follow(%Actor{} = follower_actor, %Actor{type: type} = followed, _local, additional)
when type != :Person do
with {:ok, %Follower{} = follower} <-
Mobilizon.Actors.follow(followed, follower_actor, additional["activity_id"], false),
:ok <- FollowMailer.send_notification_to_admins(follower),
@ -140,6 +141,8 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Actors do
end
end
def follow(_, _, _, _), do: {:error, :no_person, "Only group and instances can be followed"}
defp prepare_args_for_actor(args) do
args
|> maybe_sanitize_username()

View file

@ -123,7 +123,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Admin do
# Changes are stored as %{"key" => "value"} so we need to convert them back as struct
defp convert_changes_to_struct(struct, %{"report_id" => _report_id} = changes) do
with data <- for({key, val} <- changes, into: %{}, do: {String.to_atom(key), val}),
with data <- for({key, val} <- changes, into: %{}, do: {String.to_existing_atom(key), val}),
data <- Map.put(data, :report, Mobilizon.Reports.get_report(data.report_id)) do
struct(struct, data)
end
@ -135,7 +135,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Admin do
for(
{key, val} <- changes,
into: %{},
do: {String.to_atom(key), process_eventual_type(changeset, key, val)}
do: {String.to_existing_atom(key), process_eventual_type(changeset, key, val)}
) do
struct(struct, data)
end
@ -144,11 +144,11 @@ defmodule Mobilizon.GraphQL.Resolvers.Admin do
# datetimes are not unserialized as DateTime/NaiveDateTime so we do it manually with changeset data
defp process_eventual_type(changeset, key, val) do
cond do
changeset[String.to_atom(key)] == :utc_datetime and not is_nil(val) ->
changeset[String.to_existing_atom(key)] == :utc_datetime and not is_nil(val) ->
{:ok, datetime, _} = DateTime.from_iso8601(val)
datetime
changeset[String.to_atom(key)] == :naive_datetime and not is_nil(val) ->
changeset[String.to_existing_atom(key)] == :naive_datetime and not is_nil(val) ->
{:ok, datetime} = NaiveDateTime.from_iso8601(val)
datetime

View file

@ -76,10 +76,11 @@ defmodule Mobilizon do
:ok
end
# sobelow_skip ["DOS.StringToAtom"]
@spec cachex_spec(atom, integer, integer, integer, function | nil) :: Supervisor.child_spec()
defp cachex_spec(name, limit, default, interval, fallback \\ nil) do
%{
id: :"cache_#{name}",
id: String.to_atom("cache_#{to_string(name)}"),
start:
{Cachex, :start_link,
[

View file

@ -296,18 +296,6 @@ defmodule Mobilizon.Actors do
end
end
# defp transform_media_file(nil), do: nil
# defp transform_media_file(file) do
# file = for({key, val} <- file, into: %{}, do: {String.to_atom(key), val})
# if is_nil(file) do
# nil
# else
# struct(Mobilizon.Medias.File, file)
# end
# end
@delete_actor_default_options [reserve_username: true, suspension: false]
def delete_actor(%Actor{} = actor, options \\ @delete_actor_default_options) do

View file

@ -58,6 +58,7 @@ defmodule Mobilizon.Discussions do
@doc """
Callback for Absinthe Ecto Dataloader
"""
# sobelow_skip ["SQL.Query"]
@spec data :: Dataloader.Ecto.t()
def data do
Dataloader.Ecto.new(Repo, query: &query/2)

View file

@ -185,6 +185,7 @@ defmodule Mobilizon.Resources do
end)
end
# sobelow_skip ["SQL.Query"]
@spec update_children(Multi.t(), Resource.t(), map()) :: Multi.t()
defp update_children(
%Multi{} = multi,

View file

@ -7,12 +7,21 @@ defmodule Mobilizon.Service.RichMedia.Parsers.MetaTagsParser do
@moduledoc """
Module to parse meta tags data in HTML pages
"""
def parse(html, data, prefix, error_message, key_name, value_name \\ "content") do
def parse(
html,
data,
prefix,
error_message,
key_name,
value_name \\ :content,
allowed_attributes \\ []
) do
meta_data =
html
|> get_elements(key_name, prefix)
|> Enum.reduce(data, fn el, acc ->
attributes = normalize_attributes(el, prefix, key_name, value_name)
attributes = normalize_attributes(el, prefix, key_name, value_name, allowed_attributes)
Map.merge(acc, attributes)
end)
@ -27,18 +36,23 @@ defmodule Mobilizon.Service.RichMedia.Parsers.MetaTagsParser do
end
defp get_elements(html, key_name, prefix) do
html |> Floki.find("meta[#{key_name}^='#{prefix}:']")
html |> Floki.find("meta[#{to_string(key_name)}^='#{prefix}:']")
end
defp normalize_attributes(html_node, prefix, key_name, value_name) do
defp normalize_attributes(html_node, prefix, key_name, value_name, allowed_attributes) do
{_tag, attributes, _children} = html_node
data =
Enum.into(attributes, %{}, fn {name, value} ->
attributes
|> Enum.into(%{}, fn {name, value} ->
{name, String.trim_leading(value, "#{prefix}:")}
end)
%{String.to_atom(data[key_name]) => data[value_name]}
if data[to_string(key_name)] in Enum.map(allowed_attributes, &to_string/1) do
%{String.to_existing_atom(data[to_string(key_name)]) => data[to_string(value_name)]}
else
%{}
end
end
defp maybe_put_title(%{title: _} = meta, _), do: meta

View file

@ -41,10 +41,31 @@ defmodule Mobilizon.Service.RichMedia.Parsers.OEmbed do
{:ok, Enum.into(attributes, %{})["href"]}
end
@oembed_allowed_attributes [
:type,
:version,
:html,
:width,
:height,
:title,
:author_name,
:author_url,
:provider_name,
:provider_url,
:cache_age,
:thumbnail_url,
:thumbnail_width,
:thumbnail_height,
:url
]
defp get_oembed_data(url) do
with {:ok, %{body: json}} <- Tesla.get(url, opts: @http_options),
{:ok, data} <- Jason.decode(json),
data <- data |> Map.new(fn {k, v} -> {String.to_atom(k), v} end) do
data <-
data
|> Map.new(fn {k, v} -> {String.to_existing_atom(k), v} end)
|> Map.take(@oembed_allowed_attributes) do
{:ok, data}
end
end

View file

@ -10,6 +10,26 @@ defmodule Mobilizon.Service.RichMedia.Parsers.OGP do
require Logger
alias Mobilizon.Service.RichMedia.Parsers.MetaTagsParser
@opengraph_properties [
:title,
:type,
:image,
:url,
:audio,
:description,
:determiner,
:locale,
:"locale:alternate",
:site_name,
:video,
:"image:url",
:"image.secure_url",
:"image:type",
:"image:width",
:"image:height",
:"image:alt"
]
def parse(html, data) do
Logger.debug("Using OpenGraph card parser")
@ -19,7 +39,9 @@ defmodule Mobilizon.Service.RichMedia.Parsers.OGP do
data,
"og",
"No OGP metadata found",
"property"
:property,
:content,
@opengraph_properties
) do
data = transform_tags(data)
Logger.debug("Data found with OpenGraph card parser")
@ -29,9 +51,11 @@ defmodule Mobilizon.Service.RichMedia.Parsers.OGP do
defp transform_tags(data) do
data
|> Map.put(:image_remote_url, Map.get(data, :image))
|> Map.put(:width, get_integer_value(data, :"image:width"))
|> Map.put(:height, get_integer_value(data, :"image:height"))
|> Enum.reject(fn {_, v} -> is_nil(v) end)
|> Map.new()
|> Map.update(:image_remote_url, Map.get(data, :image), & &1)
|> Map.update(:width, get_integer_value(data, :"image:width"), & &1)
|> Map.update(:height, get_integer_value(data, :"image:height"), & &1)
end
@spec get_integer_value(map(), atom()) :: integer() | nil

View file

@ -10,25 +10,49 @@ defmodule Mobilizon.Service.RichMedia.Parsers.TwitterCard do
alias Mobilizon.Service.RichMedia.Parsers.MetaTagsParser
require Logger
@twitter_card_properties [
:card,
:site,
:creator,
:title,
:description,
:image,
:"image:alt"
]
@spec parse(String.t(), map()) :: {:ok, map()} | {:error, String.t()}
def parse(html, data) do
Logger.debug("Using Twitter card parser")
res =
with {:ok, data} <- parse_name_attrs(data, html),
{:ok, data} <- parse_property_attrs(data, html),
data <- transform_tags(data) do
Logger.debug("Data found with Twitter card parser")
Logger.debug(inspect(data))
data
|> parse_name_attrs(html)
|> parse_property_attrs(html)
Logger.debug("Data found with Twitter card parser")
Logger.debug(inspect(res))
res
end
end
defp parse_name_attrs(data, html) do
MetaTagsParser.parse(html, data, "twitter", %{}, "name")
MetaTagsParser.parse(html, data, "twitter", %{}, :name, :content, [:"twitter:card"])
end
defp parse_property_attrs({_, data}, html) do
MetaTagsParser.parse(html, data, "twitter", "No twitter card metadata found", "property")
MetaTagsParser.parse(
html,
data,
"twitter",
"No twitter card metadata found",
:property,
:content,
@twitter_card_properties
)
end
defp transform_tags(data) do
data
|> Enum.reject(fn {_, v} -> is_nil(v) end)
|> Map.new()
|> Map.update(:image_remote_url, Map.get(data, :image), & &1)
end
end

View file

@ -4,6 +4,7 @@ defmodule Mobilizon.Web.Auth.ErrorHandler do
"""
import Plug.Conn
# sobelow_skip ["XSS.SendResp"]
def auth_error(conn, {type, _reason}, _opts) do
body = Jason.encode!(%{message: to_string(type)})
send_resp(conn, 401, body)

View file

@ -145,6 +145,7 @@ defmodule Mobilizon.Web.ReverseProxy do
end
end
# sobelow_skip ["XSS.SendResp"]
def call(conn, _, _) do
conn
|> send_resp(400, Conn.Status.reason_phrase(400))
@ -223,6 +224,7 @@ defmodule Mobilizon.Web.ReverseProxy do
|> send_resp(code, "")
end
# sobelow_skip ["XSS.SendResp"]
defp error_or_redirect(conn, url, code, body, opts) do
if Keyword.get(opts, :redirect_on_failure, false) do
conn

View file

@ -4,6 +4,12 @@ defmodule Mobilizon.Web.Router do
"""
use Mobilizon.Web, :router
@csp if Application.fetch_env!(:mobilizon, :env) != :dev,
do: "default-src 'self';",
else:
"default-src 'self'; script-src 'self' 'unsafe-eval' 'unsafe-inline'; style-src 'self' 'unsafe-inline'"
@headers %{"content-security-policy" => @csp}
pipeline :graphql do
# plug(:accepts, ["json"])
plug(Mobilizon.Web.Auth.Pipeline)
@ -30,6 +36,7 @@ defmodule Mobilizon.Web.Router do
pipeline :activity_pub_and_html do
plug(:accepts, ["html", "activity-json"])
plug(:put_secure_browser_headers, @headers)
plug(Cldr.Plug.AcceptLanguage,
cldr_backend: Mobilizon.Cldr
@ -37,6 +44,7 @@ defmodule Mobilizon.Web.Router do
end
pipeline :atom_and_ical do
plug(:put_secure_browser_headers, @headers)
plug(:accepts, ["atom", "ics", "html"])
end
@ -48,10 +56,7 @@ defmodule Mobilizon.Web.Router do
)
plug(:accepts, ["html"])
plug(:fetch_session)
plug(:fetch_flash)
plug(:protect_from_forgery)
plug(:put_secure_browser_headers)
plug(:put_secure_browser_headers, @headers)
end
pipeline :remote_media do
@ -158,6 +163,8 @@ defmodule Mobilizon.Web.Router do
get("/interact", PageController, :interact)
get("/auth/:provider", AuthController, :request)
# Have a look at https://github.com/ueberauth/ueberauth/issues/125 some day
# Also possible CSRF issue
get("/auth/:provider/callback", AuthController, :callback)
post("/auth/:provider/callback", AuthController, :callback)
end

View file

@ -5,6 +5,7 @@ defmodule Mobilizon.Web.Views.Utils do
alias Mobilizon.Service.Metadata.Utils, as: MetadataUtils
# sobelow_skip ["Traversal.FileModule"]
@spec inject_tags(Enum.t(), String.t()) :: {:safe, String.t()}
def inject_tags(tags, locale \\ "en") do
with {:ok, index_content} <- File.read(index_file_path()) do

View file

@ -154,7 +154,8 @@ defmodule Mobilizon.Mixfile do
{:mock, "~> 0.3.4", only: :test},
{:elixir_feed_parser, "~> 2.1.0", only: :test},
{:mox, "~> 1.0", only: :test},
{:junit_formatter, "~> 3.1", only: [:test]}
{:junit_formatter, "~> 3.1", only: [:test]},
{:sobelow, "~> 0.8", only: [:dev, :test]}
] ++ oauth_deps()
end

View file

@ -116,6 +116,7 @@
"sitemapper": {:hex, :sitemapper, "0.5.0", "23b0bb7b3888f03d4e4e5bedb7034e6d2979e169366372d960d6f433112b9bdf", [:mix], [{:ex_aws_s3, "~> 2.0", [hex: :ex_aws_s3, repo: "hexpm", optional: true]}, {:xml_builder, "~> 2.1.1", [hex: :xml_builder, repo: "hexpm", optional: false]}], "hexpm", "be7acff8d0245aa7ca125b9c4d0751009bbbca26ef866d888fef4fdf98670e41"},
"sleeplocks": {:hex, :sleeplocks, "1.1.1", "3d462a0639a6ef36cc75d6038b7393ae537ab394641beb59830a1b8271faeed3", [:rebar3], [], "hexpm", "84ee37aeff4d0d92b290fff986d6a95ac5eedf9b383fadfd1d88e9b84a1c02e1"},
"slugger": {:hex, :slugger, "0.3.0", "efc667ab99eee19a48913ccf3d038b1fb9f165fa4fbf093be898b8099e61b6ed", [:mix], [], "hexpm", "20d0ded0e712605d1eae6c5b4889581c3460d92623a930ddda91e0e609b5afba"},
"sobelow": {:hex, :sobelow, "0.11.0", "cdc17e3a9f1ea78dc55dbe0a03121cb6767fef737c6d9f1e62ee7e78730abccc", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "c57807bfe6f231338b657781f89ef0320b66a0dbe779aa911d6ed27cfa14ae6e"},
"ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.6", "cf344f5692c82d2cd7554f5ec8fd961548d4fd09e7d22f5b62482e5aeaebd4b0", [:make, :mix, :rebar3], [], "hexpm", "bdb0d2471f453c88ff3908e7686f86f9be327d065cc1ec16fa4540197ea04680"},
"telemetry": {:hex, :telemetry, "0.4.2", "2808c992455e08d6177322f14d3bdb6b625fbcfd233a73505870d8738a2f4599", [:rebar3], [], "hexpm", "2d1419bd9dda6a206d7b5852179511722e2b18812310d304620c7bd92a13fcef"},
"tesla": {:hex, :tesla, "1.4.0", "1081bef0124b8bdec1c3d330bbe91956648fb008cf0d3950a369cda466a31a87", [:mix], [{:castore, "~> 0.1", [hex: :castore, repo: "hexpm", optional: true]}, {:exjsx, ">= 3.0.0", [hex: :exjsx, repo: "hexpm", optional: true]}, {:finch, "~> 0.3", [hex: :finch, repo: "hexpm", optional: true]}, {:fuse, "~> 2.4", [hex: :fuse, repo: "hexpm", optional: true]}, {:gun, "~> 1.3", [hex: :gun, repo: "hexpm", optional: true]}, {:hackney, "~> 1.6", [hex: :hackney, repo: "hexpm", optional: true]}, {:ibrowse, "~> 4.4.0", [hex: :ibrowse, repo: "hexpm", optional: true]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: true]}, {:mime, "~> 1.0", [hex: :mime, repo: "hexpm", optional: false]}, {:mint, "~> 1.0", [hex: :mint, repo: "hexpm", optional: true]}, {:poison, ">= 1.0.0", [hex: :poison, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4", [hex: :telemetry, repo: "hexpm", optional: true]}], "hexpm", "bf1374a5569f5fca8e641363b63f7347d680d91388880979a33bc12a6eb3e0aa"},

View file

@ -1,6 +1,7 @@
defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.FollowTest do
use Mobilizon.DataCase
import ExUnit.CaptureLog
import Mobilizon.Factory
alias Mobilizon.Actors
alias Mobilizon.Actors.Follower
@ -8,9 +9,25 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.FollowTest do
alias Mobilizon.Federation.ActivityPub.{Activity, Transmogrifier}
describe "handle incoming follow requests" do
test "it works for incoming follow requests" do
test "it works only for groups" do
actor = insert(:actor)
data =
File.read!("test/fixtures/mastodon-follow-activity.json")
|> Jason.decode!()
|> Map.put("object", actor.url)
assert capture_log(fn ->
:error = Transmogrifier.handle_incoming(data)
end) =~ "Only group and instances can be followed"
actor = Actors.get_actor_with_preload(actor.id)
refute Actors.is_following(Actors.get_actor_by_url!(data["actor"], true), actor)
end
test "it works for incoming follow requests" do
actor = insert(:group)
data =
File.read!("test/fixtures/mastodon-follow-activity.json")
|> Jason.decode!()
@ -27,7 +44,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.FollowTest do
end
test "it rejects activities without a valid ID" do
actor = insert(:actor)
actor = insert(:group)
data =
File.read!("test/fixtures/mastodon-follow-activity.json")
@ -59,7 +76,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.FollowTest do
describe "handle incoming follow accept activities" do
test "it works for incoming accepts" do
follower = insert(:actor)
followed = insert(:actor, manually_approves_followers: false)
followed = insert(:group, manually_approves_followers: false)
refute Actors.is_following(follower, followed)
@ -91,7 +108,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.FollowTest do
test "it works for incoming accepts which were pre-accepted" do
follower = insert(:actor)
followed = insert(:actor, manually_approves_followers: true)
followed = insert(:group, manually_approves_followers: true)
refute Actors.is_following(follower, followed)
@ -126,7 +143,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.FollowTest do
test "it works for incoming accepts which are referenced by IRI only" do
follower = insert(:actor)
followed = insert(:actor, manually_approves_followers: true)
followed = insert(:group, manually_approves_followers: true)
{:ok, follow_activity, _} = ActivityPub.follow(follower, followed)
@ -148,7 +165,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.FollowTest do
test "it fails for incoming accepts which cannot be correlated" do
follower = insert(:actor)
followed = insert(:actor)
followed = insert(:group)
accept_data =
File.read!("test/fixtures/mastodon-accept-activity.json")
@ -169,7 +186,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.FollowTest do
describe "handle incoming follow reject activities" do
test "it fails for incoming rejects which cannot be correlated" do
follower = insert(:actor)
followed = insert(:actor)
followed = insert(:group)
accept_data =
File.read!("test/fixtures/mastodon-reject-activity.json")
@ -188,7 +205,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.FollowTest do
test "it works for incoming rejects which are referenced by IRI only" do
follower = insert(:actor)
followed = insert(:actor)
followed = insert(:group)
{:ok, follow_activity, _} = ActivityPub.follow(follower, followed)

View file

@ -46,7 +46,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.UndoTest do
end
test "it works for incomming unfollows with an existing follow" do
actor = insert(:actor)
actor = insert(:group)
follow_data =
File.read!("test/fixtures/mastodon-follow-activity.json")