improvement: better check module validation

docs: add more context to security and policy guides

documentation/topics/policies.md

@@ -2,24 +2,9 @@
 
 Policies determine what actions on a resource are permitted for a given actor.
 
-You can specify an actor using the `actor` option, whenever using the code interface or creating changesets/queries like so:
-
-```elixir
-MyApp.MyApi.read(MyResource, actor: current_user)
-
-MyResource 
-|> Ash.Query.for_read(:read, %{}, actor: current_user)
-
-MyResource 
-|> Ash.Changeset.for_create(:create, %{}, actor: current_user)
-```
-
 ## Important!
 
-Before we jump into the guide, it is critical to understand that the policy code doesn't actually
-_do_ anything in the classic sense. It only builds up a set of policies that are stored for use later.
-The checker that reads those policies and authorizes requests may run all, some of, or none of your checks,
-depending on the details of the request being authorized.
+Read and understand the {{link:ash:guide:Security}} guide before proceeding, which explains actors, how to set them, and other relevant configurations.
 
 ## Guide
 

documentation/topics/security.md

@@ -1,5 +1,9 @@
 # Security
 
+## Important Note!
+
+A great thing to do early on is to be explicit about your security configuration. To that end, once you've read this guide, we highly recommend that you place the configuration found at the bottom of your guide into your api modules, even if you are simply setting them to their default values. Especially the `authorize` option.
+
 ## Authorization
 
 Authorization in Ash is done via authorizers. Generally, you won't need to create your own  authorizer, as the builtin policy authorizer {{link:ash:extension:Policy Authorizer}} should work well for any use case. Authorization is performed with a given actor and a query or changeset.
@@ -33,10 +37,30 @@ Api.read!(User, actor: current_user, authorize?: true)
 Api.read!(User, actor: current_user, authorize?: false)
 ```
 
+#### Where to set the actor
+
+When setting an actor, if you are building a query or changeset, you should do so at the time that you call the various `for_*` functions. This makes the actor available in the context of any change that is run. For example:
+
+```elixir
+# DO THIS
+Resource
+|> Ash.Query.for_read(:read, input, actor: current_user)
+|> Api.read()
+
+# DON'T DO THIS
+Resource
+|> Ash.Query.for_read(:read, input, actor: current_user)
+|> Api.read(actor: current_user)
+```
+
+The second option "works" in most cases, but not all, because some `change`s might need to know the actor
+
 ### Context
 
 Ash can store the actor, query context, or tenant in the process dictionary. This can help simplify things like live views, controllers, or channels where all actions performed share these pieces of context.
 
+This can be useful, but the general recommendation is to be explicit by passing options.
+
 ```elixir
 # in socket connect, liveview mount, or a plug
 Ash.set_actor(current_user)
@@ -58,6 +82,10 @@ Important: `nil` is still a valid actor, so this won't prevent providing `actor:
 
 #### {{link:ash:option:api/authorization/authorize}}
 
+##### Important!
+
+The default value for this is relatively loose, and we intend to change it in the 3.0 release (which is not scheduled for anytime soon). Right now, it is `:when_requested`, but a better default would be `:by_default`, and is what you should choose when starting out. 
+
 When to run authorization for a given request.
 
 - `:always` forces `authorize?: true` on all requests to the Api. 

lib/ash/policy/authorizer.ex

@@ -328,7 +328,11 @@ defmodule Ash.Policy.Authorizer do
 
   @doc false
   def validate_check({module, opts}) when is_atom(module) and is_list(opts) do
-    {:ok, {module, opts}}
+    if Ash.Helpers.implements_behaviour?(module, Ash.Policy.Check) do
+      {:ok, {module, opts}}
+    else
+      {:error, "All checks must implement the Ash.Policy.Check behaviour"}
+    end
   end
 
   def validate_check(module) when is_atom(module) do
@@ -341,12 +345,12 @@ defmodule Ash.Policy.Authorizer do
 
   def validate_condition(conditions) when is_list(conditions) do
     Enum.reduce_while(conditions, {:ok, []}, fn condition, {:ok, conditions} ->
-      {:ok, {condition, opts}} = validate_check(condition)
+      case validate_check(condition) do
+        {:ok, {condition, opts}} ->
+          {:cont, {:ok, [{condition, opts} | conditions]}}
 
-      if Ash.Helpers.implements_behaviour?(condition, Ash.Policy.Check) do
-        {:cont, {:ok, [{condition, opts} | conditions]}}
-      else
-        {:halt, {:error, "Expected all conditions to be valid checks"}}
+        {:error, "All checks must implement the Ash.Policy.Check behaviour"} ->
+          {:halt, {:error, "Expected all conditions to implement the Ash.Policy.Check behaviour"}}
       end
     end)
   end