From a007764d277cb96d7da304b53cbdd5ec6b399ef2 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Thu, 14 Jun 2018 18:15:27 +0200 Subject: [PATCH] Fix credo issues Signed-off-by: Thomas Citharel --- .credo.exs | 160 ++++++++++++++++++ lib/eventos/actors/actors.ex | 8 +- lib/eventos/events/comment.ex | 4 + .../controllers/activity_pub_controller.ex | 8 +- .../controllers/event_controller.ex | 2 +- lib/eventos_web/http_signature.ex | 10 +- .../views/activity_pub/actor_view.ex | 12 +- lib/mix/tasks/create_bot.ex | 4 + lib/service/activity_pub/activity_pub.ex | 14 +- lib/service/activity_pub/transmogrifier.ex | 13 +- lib/service/activity_pub/utils.ex | 6 + lib/service/federator.ex | 10 +- .../http_signatures/http_signatures.ex | 22 ++- lib/service/streamer.ex | 17 +- lib/service/web_finger/web_finger.ex | 11 +- lib/service/xml_builder.ex | 6 + test/support/factory.ex | 4 +- 17 files changed, 267 insertions(+), 44 deletions(-) create mode 100644 .credo.exs diff --git a/.credo.exs b/.credo.exs new file mode 100644 index 000000000..f8418d5a4 --- /dev/null +++ b/.credo.exs @@ -0,0 +1,160 @@ +# This file contains the configuration for Credo and you are probably reading +# this after creating it with `mix credo.gen.config`. +# +# If you find anything wrong or unclear in this file, please report an +# issue on GitHub: https://github.com/rrrene/credo/issues +# +%{ + # + # You can have as many configs as you like in the `configs:` field. + configs: [ + %{ + # + # Run any exec using `mix credo -C `. If no exec name is given + # "default" is used. + # + name: "default", + # + # These are the files included in the analysis: + files: %{ + # + # You can give explicit globs or simply directories. + # In the latter case `**/*.{ex,exs}` will be used. + # + included: ["lib/", "src/", "test/", "web/", "apps/"], + excluded: [~r"/_build/", ~r"/deps/"] + }, + # + # If you create your own checks, you must specify the source files for + # them here, so they can be loaded by Credo before running the analysis. + # + requires: [], + # + # If you want to enforce a style guide and need a more traditional linting + # experience, you can change `strict` to `true` below: + # + strict: false, + # + # If you want to use uncolored output by default, you can change `color` + # to `false` below: + # + color: true, + # + # You can customize the parameters of any check by adding a second element + # to the tuple. + # + # To disable a check put `false` as second element: + # + # {Credo.Check.Design.DuplicatedCode, false} + # + checks: [ + # + ## Consistency Checks + # + {Credo.Check.Consistency.ExceptionNames}, + {Credo.Check.Consistency.LineEndings}, + {Credo.Check.Consistency.ParameterPatternMatching}, + {Credo.Check.Consistency.SpaceAroundOperators}, + {Credo.Check.Consistency.SpaceInParentheses}, + {Credo.Check.Consistency.TabsOrSpaces}, + + # + ## Design Checks + # + # You can customize the priority of any check + # Priority values are: `low, normal, high, higher` + # + {Credo.Check.Design.AliasUsage, priority: :low}, + # For some checks, you can also set other parameters + # + # If you don't want the `setup` and `test` macro calls in ExUnit tests + # or the `schema` macro in Ecto schemas to trigger DuplicatedCode, just + # set the `excluded_macros` parameter to `[:schema, :setup, :test]`. + # + {Credo.Check.Design.DuplicatedCode, excluded_macros: []}, + # You can also customize the exit_status of each check. + # If you don't want TODO comments to cause `mix credo` to fail, just + # set this value to 0 (zero). + # + {Credo.Check.Design.TagTODO, exit_status: 0}, + {Credo.Check.Design.TagFIXME}, + + # + ## Readability Checks + # + {Credo.Check.Readability.AliasOrder}, + {Credo.Check.Readability.FunctionNames}, + {Credo.Check.Readability.LargeNumbers}, + {Credo.Check.Readability.MaxLineLength, priority: :low, max_length: 80}, + {Credo.Check.Readability.ModuleAttributeNames}, + {Credo.Check.Readability.ModuleDoc}, + {Credo.Check.Readability.ModuleNames}, + {Credo.Check.Readability.ParenthesesOnZeroArityDefs}, + {Credo.Check.Readability.ParenthesesInCondition}, + {Credo.Check.Readability.PredicateFunctionNames}, + {Credo.Check.Readability.PreferImplicitTry}, + {Credo.Check.Readability.RedundantBlankLines}, + {Credo.Check.Readability.StringSigils}, + {Credo.Check.Readability.TrailingBlankLine}, + {Credo.Check.Readability.TrailingWhiteSpace}, + {Credo.Check.Readability.VariableNames}, + {Credo.Check.Readability.Semicolons}, + {Credo.Check.Readability.SpaceAfterCommas}, + + # + ## Refactoring Opportunities + # + {Credo.Check.Refactor.DoubleBooleanNegation}, + {Credo.Check.Refactor.CondStatements}, + {Credo.Check.Refactor.CyclomaticComplexity}, + {Credo.Check.Refactor.FunctionArity}, + {Credo.Check.Refactor.LongQuoteBlocks}, + {Credo.Check.Refactor.MatchInCondition}, + {Credo.Check.Refactor.NegatedConditionsInUnless}, + {Credo.Check.Refactor.NegatedConditionsWithElse}, + {Credo.Check.Refactor.Nesting, max_nesting: 3}, + {Credo.Check.Refactor.PipeChainStart, + excluded_argument_types: [:atom, :binary, :fn, :keyword], excluded_functions: []}, + {Credo.Check.Refactor.UnlessWithElse}, + + # + ## Warnings + # + {Credo.Check.Warning.BoolOperationOnSameValues}, + {Credo.Check.Warning.ExpensiveEmptyEnumCheck}, + {Credo.Check.Warning.IExPry}, + {Credo.Check.Warning.IoInspect}, + {Credo.Check.Warning.LazyLogging}, + {Credo.Check.Warning.OperationOnSameValues}, + {Credo.Check.Warning.OperationWithConstantResult}, + {Credo.Check.Warning.UnusedEnumOperation}, + {Credo.Check.Warning.UnusedFileOperation}, + {Credo.Check.Warning.UnusedKeywordOperation}, + {Credo.Check.Warning.UnusedListOperation}, + {Credo.Check.Warning.UnusedPathOperation}, + {Credo.Check.Warning.UnusedRegexOperation}, + {Credo.Check.Warning.UnusedStringOperation}, + {Credo.Check.Warning.UnusedTupleOperation}, + {Credo.Check.Warning.RaiseInsideRescue}, + + # + # Controversial and experimental checks (opt-in, just remove `, false`) + # + {Credo.Check.Refactor.ABCSize, false}, + {Credo.Check.Refactor.AppendSingleItem, false}, + {Credo.Check.Refactor.VariableRebinding, false}, + {Credo.Check.Warning.MapGetUnsafePass, false}, + {Credo.Check.Consistency.MultiAliasImportRequireUse, false}, + + # + # Deprecated checks (these will be deleted after a grace period) + # + {Credo.Check.Readability.Specs, false} + + # + # Custom checks can be created using `mix credo.gen.check`. + # + ] + } + ] +} diff --git a/lib/eventos/actors/actors.ex b/lib/eventos/actors/actors.ex index 4916bc8a6..d222ee7a4 100644 --- a/lib/eventos/actors/actors.ex +++ b/lib/eventos/actors/actors.ex @@ -335,9 +335,9 @@ defmodule Eventos.Actors do Register user """ def register(%{email: email, password: password, username: username}) do - key = :public_key.generate_key({:rsa, 2048, 65537}) + key = :public_key.generate_key({:rsa, 2048, 65_537}) entry = :public_key.pem_entry_encode(:RSAPrivateKey, key) - pem = :public_key.pem_encode([entry]) |> String.trim_trailing() + pem = [entry] |> :public_key.pem_encode() |> String.trim_trailing() import Exgravatar @@ -375,9 +375,9 @@ defmodule Eventos.Actors do end def register_bot_account(%{name: name, summary: summary}) do - key = :public_key.generate_key({:rsa, 2048, 65537}) + key = :public_key.generate_key({:rsa, 2048, 65_537}) entry = :public_key.pem_entry_encode(:RSAPrivateKey, key) - pem = :public_key.pem_encode([entry]) |> String.trim_trailing() + pem = [entry] |> :public_key.pem_encode() |> String.trim_trailing() actor = Eventos.Actors.Actor.registration_changeset(%Eventos.Actors.Actor{}, %{ preferred_username: name, diff --git a/lib/eventos/events/comment.ex b/lib/eventos/events/comment.ex index e43e72bc4..10aae2ab1 100644 --- a/lib/eventos/events/comment.ex +++ b/lib/eventos/events/comment.ex @@ -1,4 +1,8 @@ defmodule Eventos.Events.Comment do + @moduledoc """ + An actor comment (for instance on an event or on a group) + """ + use Ecto.Schema import Ecto.Changeset diff --git a/lib/eventos_web/controllers/activity_pub_controller.ex b/lib/eventos_web/controllers/activity_pub_controller.ex index 47b01f927..34ee75378 100644 --- a/lib/eventos_web/controllers/activity_pub_controller.ex +++ b/lib/eventos_web/controllers/activity_pub_controller.ex @@ -83,13 +83,13 @@ defmodule EventosWeb.ActivityPubController do def inbox(conn, params) do headers = Enum.into(conn.req_headers, %{}) - if !String.contains?(headers["signature"] || "", params["actor"]) do - Logger.info("Signature not from author, relayed message, fetching from source") - ActivityPub.fetch_event_from_url(params["object"]["id"]) - else + if String.contains?(headers["signature"] || "", params["actor"]) do Logger.info("Signature error") Logger.info("Could not validate #{params["actor"]}") Logger.info(inspect(conn.req_headers)) + else + Logger.info("Signature not from author, relayed message, fetching from source") + ActivityPub.fetch_event_from_url(params["object"]["id"]) end json(conn, "ok") diff --git a/lib/eventos_web/controllers/event_controller.ex b/lib/eventos_web/controllers/event_controller.ex index e4c43bc95..72bfc776b 100644 --- a/lib/eventos_web/controllers/event_controller.ex +++ b/lib/eventos_web/controllers/event_controller.ex @@ -60,7 +60,7 @@ defmodule EventosWeb.EventController do end def export_to_ics(conn, %{"uuid" => uuid}) do - event = Events.get_event_full_by_uuid(uuid) |> ICalendar.export_event() + event = uuid |> Events.get_event_full_by_uuid() |> ICalendar.export_event() send_resp(conn, 200, event) end diff --git a/lib/eventos_web/http_signature.ex b/lib/eventos_web/http_signature.ex index 7d3b1725b..f5094e19e 100644 --- a/lib/eventos_web/http_signature.ex +++ b/lib/eventos_web/http_signature.ex @@ -1,4 +1,10 @@ defmodule EventosWeb.HTTPSignaturePlug do + @moduledoc """ + # HTTPSignaturePlug + + Plug to check HTTP Signatures on every incoming request + """ + alias Eventos.Service.HTTPSignatures import Plug.Conn require Logger @@ -13,7 +19,9 @@ defmodule EventosWeb.HTTPSignaturePlug do def call(conn, _opts) do user = conn.params["actor"] - Logger.debug("Checking sig for #{user}") + Logger.debug fn -> + "Checking sig for #{user}" + end with [signature | _] <- get_req_header(conn, "signature") do cond do signature && String.contains?(signature, user) -> diff --git a/lib/eventos_web/views/activity_pub/actor_view.ex b/lib/eventos_web/views/activity_pub/actor_view.ex index 517c7d6b6..025764974 100644 --- a/lib/eventos_web/views/activity_pub/actor_view.ex +++ b/lib/eventos_web/views/activity_pub/actor_view.ex @@ -49,9 +49,9 @@ defmodule EventosWeb.ActivityPub.ActorView do end def render("following.json", %{actor: actor, page: page}) do - following = Actor.get_followings(actor) - - collection(following, actor.following_url, page) + actor + |> Actor.get_followings() + |> collection(actor.following_url, page) |> Map.merge(Utils.make_json_ld_header()) end @@ -68,9 +68,9 @@ defmodule EventosWeb.ActivityPub.ActorView do end def render("followers.json", %{actor: actor, page: page}) do - followers = Actor.get_followers(actor) - - collection(followers, actor.followers_url, page) + actor + |> Actor.get_followers() + |> collection(actor.followers_url, page) |> Map.merge(Utils.make_json_ld_header()) end diff --git a/lib/mix/tasks/create_bot.ex b/lib/mix/tasks/create_bot.ex index a2d6aec31..3618084ef 100644 --- a/lib/mix/tasks/create_bot.ex +++ b/lib/mix/tasks/create_bot.ex @@ -1,4 +1,8 @@ defmodule Mix.Tasks.CreateBot do + @moduledoc """ + Creates a bot from a source + """ + use Mix.Task alias Eventos.Actors alias Eventos.Actors.Bot diff --git a/lib/service/activity_pub/activity_pub.ex b/lib/service/activity_pub/activity_pub.ex index 265a4dca5..600a57710 100644 --- a/lib/service/activity_pub/activity_pub.ex +++ b/lib/service/activity_pub/activity_pub.ex @@ -1,4 +1,10 @@ defmodule Eventos.Service.ActivityPub do + @moduledoc """ + # ActivityPub + + Every ActivityPub method + """ + alias Eventos.Events alias Eventos.Events.{Event, Category} alias Eventos.Service.ActivityPub.Transmogrifier @@ -49,8 +55,8 @@ defmodule Eventos.Service.ActivityPub do url, [Accept: "application/activity+json"], follow_redirect: true, - timeout: 10000, - recv_timeout: 20000 + timeout: 10_000, + recv_timeout: 20_000 ), {:ok, data} <- Jason.decode(body), nil <- Events.get_event_by_url!(data["id"]), @@ -285,9 +291,7 @@ defmodule Eventos.Service.ActivityPub do case bot.type do "ics" -> {:ok, %HTTPoison.Response{body: body} = _resp} = HTTPoison.get(bot.source) - ical_events = body - |> ExIcal.parse() - |> ExIcal.by_range(DateTime.utc_now(), DateTime.utc_now() |> Timex.shift(years: 1)) + ical_events = body |> ExIcal.parse() |> ExIcal.by_range(DateTime.utc_now(), DateTime.utc_now() |> Timex.shift(years: 1)) activities = ical_events |> Enum.chunk_every(limit) |> Enum.at(page - 1) diff --git a/lib/service/activity_pub/transmogrifier.ex b/lib/service/activity_pub/transmogrifier.ex index 12c0d382c..430a19799 100644 --- a/lib/service/activity_pub/transmogrifier.ex +++ b/lib/service/activity_pub/transmogrifier.ex @@ -201,10 +201,10 @@ defmodule Eventos.Service.ActivityPub.Transmogrifier do if object = Object.get_by_ap_id(id), do: {:ok, object}, else: nil end - def set_reply_to_uri(%{"inReplyTo" => inReplyTo} = object) do - with false <- String.starts_with?(inReplyTo, "http"), - {:ok, %{data: replied_to_object}} <- get_obj_helper(inReplyTo) do - Map.put(object, "inReplyTo", replied_to_object["external_url"] || inReplyTo) + def set_reply_to_uri(%{"inReplyTo" => in_reply_to} = object) do + with false <- String.starts_with?(in_reply_to, "http"), + {:ok, %{data: replied_to_object}} <- get_obj_helper(in_reply_to) do + Map.put(object, "inReplyTo", replied_to_object["external_url"] || in_reply_to) else _e -> object end @@ -332,10 +332,9 @@ defmodule Eventos.Service.ActivityPub.Transmogrifier do # end # def add_attributed_to(object) do - attributedTo = object["attributedTo"] || object["actor"] + attributed_to = object["attributedTo"] || object["actor"] - object - |> Map.put("attributedTo", attributedTo) + object |> Map.put("attributedTo", attributed_to) end # # def prepare_attachments(object) do diff --git a/lib/service/activity_pub/utils.ex b/lib/service/activity_pub/utils.ex index ecee714d4..ff245538f 100644 --- a/lib/service/activity_pub/utils.ex +++ b/lib/service/activity_pub/utils.ex @@ -1,4 +1,10 @@ defmodule Eventos.Service.ActivityPub.Utils do + @moduledoc """ + # Utils + + Various utils + """ + alias Eventos.Repo alias Eventos.Actors alias Eventos.Actors.Actor diff --git a/lib/service/federator.ex b/lib/service/federator.ex index 2065537af..0ac221229 100644 --- a/lib/service/federator.ex +++ b/lib/service/federator.ex @@ -1,4 +1,8 @@ defmodule Eventos.Service.Federator do + @moduledoc """ + Handle federated activities + """ + use GenServer alias Eventos.Actors alias Eventos.Activity @@ -16,7 +20,7 @@ defmodule Eventos.Service.Federator do spawn(fn -> # 1 minute - Process.sleep(1000 * 60 * 1) + Process.sleep(1000 * 60) end) GenServer.start_link( @@ -101,7 +105,9 @@ defmodule Eventos.Service.Federator do end def handle_cast(m, state) do - IO.inspect("Unknown: #{inspect(m)}, #{inspect(state)}") + Logger.error fn -> + "Unknown: #{inspect(m)}, #{inspect(state)}" + end {:noreply, state} end diff --git a/lib/service/http_signatures/http_signatures.ex b/lib/service/http_signatures/http_signatures.ex index 05e01611b..831fd6462 100644 --- a/lib/service/http_signatures/http_signatures.ex +++ b/lib/service/http_signatures/http_signatures.ex @@ -1,8 +1,14 @@ # https://tools.ietf.org/html/draft-cavage-http-signatures-08 defmodule Eventos.Service.HTTPSignatures do + @moduledoc """ + # HTTP Signatures + + Generates and checks HTTP Signatures + """ + alias Eventos.Actors.Actor alias Eventos.Service.ActivityPub - require Logger + import Logger def split_signature(sig) do default = %{"headers" => "date"} @@ -22,8 +28,12 @@ defmodule Eventos.Service.HTTPSignatures do def validate(headers, signature, public_key) do sigstring = build_signing_string(headers, signature["headers"]) - Logger.debug("Signature: #{signature["signature"]}") - Logger.debug("Sigstring: #{sigstring}") + Logger.debug fn -> + "Signature: #{signature["signature"]}" + end + Logger.debug fn -> + "Sigstring: #{sigstring}" + end {:ok, sig} = Base.decode64(signature["signature"]) :public_key.verify(sigstring, :sha256, sig, public_key) end @@ -74,14 +84,12 @@ defmodule Eventos.Service.HTTPSignatures do with private_key = Actor.get_keys_for_actor(actor) do sigstring = build_signing_string(headers, Map.keys(headers)) - signature = - :public_key.sign(sigstring, :sha256, private_key) - |> Base.encode64() + signature = sigstring |> :public_key.sign(:sha256, private_key) |> Base.encode64() [ keyId: actor.url <> "#main-key", algorithm: "rsa-sha256", - headers: Map.keys(headers) |> Enum.join(" "), + headers: headers |> Map.keys() |> Enum.join(" "), signature: signature ] |> Enum.map(fn {k, v} -> "#{k}=\"#{v}\"" end) diff --git a/lib/service/streamer.ex b/lib/service/streamer.ex index fc9904ca5..5ffda4d0c 100644 --- a/lib/service/streamer.ex +++ b/lib/service/streamer.ex @@ -1,4 +1,10 @@ defmodule Eventos.Service.Streamer do + @moduledoc """ + # Streamer + + Handles streaming activities + """ + use GenServer require Logger alias Eventos.Accounts.Actor @@ -30,7 +36,8 @@ defmodule Eventos.Service.Streamer do end def handle_cast(%{action: :ping}, topics) do - Map.values(topics) + topics + |> Map.values() |> List.flatten() |> Enum.each(fn socket -> Logger.debug("Sending keepalive ping") @@ -51,7 +58,9 @@ defmodule Eventos.Service.Streamer do sockets_for_topic = sockets[topic] || [] sockets_for_topic = Enum.uniq([socket | sockets_for_topic]) sockets = Map.put(sockets, topic, sockets_for_topic) - Logger.debug("Got new conn for #{topic}") + Logger.debug fn -> + "Got new conn for #{topic}" + end {:noreply, sockets} end @@ -60,7 +69,9 @@ defmodule Eventos.Service.Streamer do sockets_for_topic = sockets[topic] || [] sockets_for_topic = List.delete(sockets_for_topic, socket) sockets = Map.put(sockets, topic, sockets_for_topic) - Logger.debug("Removed conn for #{topic}") + Logger.debug fn -> + "Removed conn for #{topic}" + end {:noreply, sockets} end diff --git a/lib/service/web_finger/web_finger.ex b/lib/service/web_finger/web_finger.ex index d518378e1..c61807ad4 100644 --- a/lib/service/web_finger/web_finger.ex +++ b/lib/service/web_finger/web_finger.ex @@ -1,4 +1,9 @@ defmodule Eventos.Service.WebFinger do + @moduledoc """ + # WebFinger + + Performs the WebFinger requests and responses (json only) + """ alias Eventos.Actors alias Eventos.Service.XmlBuilder @@ -59,7 +64,9 @@ defmodule Eventos.Service.WebFinger do {"application/activity+json", "self"} -> Map.put(data, "url", link["href"]) _ -> - Logger.debug("Unhandled type: #{inspect(link["type"])}") + Logger.debug fn -> + "Unhandled type: #{inspect(link["type"])}" + end data end end) @@ -81,7 +88,7 @@ defmodule Eventos.Service.WebFinger do address = "http://#{domain}/.well-known/webfinger?resource=acct:#{actor}" Logger.debug(inspect address) - with {:ok, %HTTPoison.Response{} = response} <- HTTPoison.get(address, [Accept: "application/json, application/activity+json, application/jrd+json"],follow_redirect: true), + with {:ok, %HTTPoison.Response{} = response} <- HTTPoison.get(address, [Accept: "application/json, application/activity+json, application/jrd+json"], follow_redirect: true), %{status_code: status_code, body: body} when status_code in 200..299 <- response do {:ok, doc} = Jason.decode(body) webfinger_from_json(doc) diff --git a/lib/service/xml_builder.ex b/lib/service/xml_builder.ex index 4e884cead..bb172a5ef 100644 --- a/lib/service/xml_builder.ex +++ b/lib/service/xml_builder.ex @@ -1,4 +1,10 @@ defmodule Eventos.Service.XmlBuilder do + @moduledoc """ + XML Builder. + + Do we still need this ? Only for xrd ? + """ + def to_xml({tag, attributes, content}) do open_tag = make_open_tag(tag, attributes) diff --git a/test/support/factory.ex b/test/support/factory.ex index 67eef5e8d..10ee63f44 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -15,9 +15,9 @@ defmodule Eventos.Factory do end def actor_factory do - key = :public_key.generate_key({:rsa, 2048, 65537}) + key = :public_key.generate_key({:rsa, 2048, 65_537}) entry = :public_key.pem_entry_encode(:RSAPrivateKey, key) - pem = :public_key.pem_encode([entry]) |> String.trim_trailing() + pem = [entry] |> :public_key.pem_encode() |> String.trim_trailing() preferred_username = sequence("thomas")