Never pass the #user{} record into auth backends, they should only see their own stuff. Get rid of authN_backend, it has little reason for existing. Flatten case of authZ_backend.

7 files changed, 87 insertions, 62 deletions

diff --git a/include/rabbit.hrl b/include/rabbit.hrl
index 627d0479..86c30fc5 100644
--- a/include/rabbit.hrl
+++ b/include/rabbit.hrl
@@ -14,12 +14,17 @@
 %% Copyright (c) 2007-2014 GoPivotal, Inc.  All rights reserved.
 %%
 
+%% Passed around most places
 -record(user, {username,
                tags,
-               authN_backend, %% Authentication module this user came from
-               authZ_backends %% List of authorization modules
-              }).
+               authz_backends}). %% List of {Module, AuthUser} pairs
 
+%% Passed to auth backends
+-record(auth_user, {username,
+                    tags,
+                    impl}).
+
+%% Implementation for the internal auth backend
 -record(internal_user, {username, password_hash, tags}).
 -record(permission, {configure, write, read}).
 -record(user_vhost, {username, virtual_host}).
diff --git a/src/rabbit_access_control.erl b/src/rabbit_access_control.erl
index a6d3cd6c..0ebd2fcf 100644
--- a/src/rabbit_access_control.erl
+++ b/src/rabbit_access_control.erl
@@ -55,55 +55,64 @@ check_user_pass_login(Username, Password) ->
 check_user_login(Username, AuthProps) ->
     {ok, Modules} = application:get_env(rabbit, auth_backends),
     R = lists:foldl(
-          fun ({ModN, ModZ}, {refused, _, _}) ->
+          fun ({ModN, ModZs0}, {refused, _, _}) ->
+                  ModZs = case ModZs0 of
+                              A when is_atom(A) -> [A];
+                              L when is_list(L) -> L
+                          end,
                   %% Different modules for authN vs authZ. So authenticate
                   %% with authN module, then if that succeeds do
                   %% passwordless (i.e pre-authenticated) login with authZ.
                   case try_authenticate(ModN, Username, AuthProps) of
-                      {ok, User, _AuthZ} -> try_authorize(ModZ, User, []);
-                      Else               -> Else
+                      {ok, ModNUser = #auth_user{username = Username2}} ->
+                          user(ModNUser, try_authorize(ModZs, Username2));
+                      Else ->
+                          Else
                   end;
               (Mod, {refused, _, _}) ->
                   %% Same module for authN and authZ. Just take the result
                   %% it gives us
-                  try_authenticate(Mod, Username, AuthProps);
-              (_, {ok, User, AuthZ}) ->
+                  case try_authenticate(Mod, Username, AuthProps) of
+                      {ok, ModNUser} -> user(ModNUser, {ok, [{Mod, ModNUser}]});
+                      Else           -> Else
+                  end;
+              (_, {ok, User}) ->
                   %% We've successfully authenticated. Skip to the end...
-                  {ok, User, AuthZ}
+                  {ok, User}
           end, {refused, "No modules checked '~s'", [Username]}, Modules),
-    case R of
-        {ok, RUser, RAuthZ} ->
-            rabbit_event:notify(user_authentication_success, [{name,Username}]),
-            %% Store the list of authorization backends
-            {ok, RUser#user{authZ_backends=RAuthZ}};
-        _ ->
-            rabbit_event:notify(user_authentication_failure, [{name,Username}]),
-            R
-    end.
+    rabbit_event:notify(case R of
+                            {ok, _User} -> user_authentication_success;
+                            _           -> user_authentication_failure
+                        end, [{name, Username}]),
+    R.
 
 try_authenticate(Module, Username, AuthProps) ->
     case Module:check_user_login(Username, AuthProps) of
-        {ok, User, AuthZ} -> {ok, User, [{Module, AuthZ}]};
-        {error, E}        -> {refused, "~s failed authenticating ~s: ~p~n",
-                              [Module, Username, E]};
-        {refused, F, A}   -> {refused, F, A}
+        {ok, AuthUser}  -> {ok, AuthUser};
+        {error, E}      -> {refused, "~s failed authenticating ~s: ~p~n",
+                            [Module, Username, E]};
+        {refused, F, A} -> {refused, F, A}
     end.
 
-try_authorize(Modules, User, AuthZList) when is_list(Modules) ->
+try_authorize(Modules, Username) ->
     lists:foldr(
-      fun (Module, {ok, _User, AuthZList2}) ->
-              try_authorize(Module, User, AuthZList2);
+      fun (Module, {ok, AUsers}) ->
+              case Module:check_user_login(Username, []) of
+                  {ok, AUser}     -> {ok, [{Module, AUser} | AUsers]};
+                  {error, E}      -> {refused, "~s failed authorizing ~s: ~p~n",
+                                        [Module, Username, E]};
+                  {refused, F, A} -> {refused, F, A}
+              end;
           (_,      {refused, _, _} = Error) ->
               Error
-      end, {ok, User, AuthZList}, Modules);
-
-try_authorize(Module, User = #user{username = Username}, AuthZList) ->
-    case Module:check_user_login(Username, []) of
-        {ok, _User, AuthZ} -> {ok, User, [{Module, AuthZ}|AuthZList]};
-        {error, E} -> {refused, "~s failed authorizing ~s: ~p~n",
-                       [Module, Username, E]};
-        Else       -> Else
-    end.
+      end, {ok, []}, Modules).
+
+user(#auth_user{username = Username, tags = Tags}, {ok, ModZUsers}) ->
+    {ok, #user{username       = Username,
+               tags           = Tags,
+               authz_backends = ModZUsers}};
+user(_AuthUser, Error) ->
+    Error.
 
 check_user_loopback(Username, SockOrAddr) ->
     {ok, Users} = application:get_env(rabbit, loopback_users),
@@ -113,14 +122,14 @@ check_user_loopback(Username, SockOrAddr) ->
         false -> not_allowed
     end.
 
-check_vhost_access(User = #user{username       = Username,
-                                authZ_backends = Modules}, VHostPath, Sock) ->
+check_vhost_access(#user{username       = Username,
+                         authz_backends = Modules}, VHostPath, Sock) ->
     lists:foldl(
-      fun({Mod, Impl}, ok) ->
+      fun({Mod, AUser}, ok) ->
               check_access(
                 fun() ->
                         rabbit_vhost:exists(VHostPath) andalso
-                            Mod:check_vhost_access(User, Impl, VHostPath, Sock)
+                            Mod:check_vhost_access(AUser, VHostPath, Sock)
                 end,
                 Mod, "access to vhost '~s' refused for user '~s'",
                 [VHostPath, Username]);
@@ -132,14 +141,14 @@ check_resource_access(User, R = #resource{kind = exchange, name = <<"">>},
                       Permission) ->
     check_resource_access(User, R#resource{name = <<"amq.default">>},
                           Permission);
-check_resource_access(User = #user{username       = Username,
-                                   authZ_backends = Modules},
+check_resource_access(#user{username       = Username,
+                            authz_backends = Modules},
                       Resource, Permission) ->
     lists:foldl(
-      fun({Module, Impl}, ok) ->
+      fun({Module, AUser}, ok) ->
               check_access(
                 fun() -> Module:check_resource_access(
-                           User, Impl, Resource, Permission) end,
+                           AUser, Resource, Permission) end,
                 Module, "access to ~s refused for user '~s'",
                 [rabbit_misc:rs(Resource), Username]);
          (_, Else) -> Else
diff --git a/src/rabbit_auth_backend.erl b/src/rabbit_auth_backend.erl
index 99f291f1..315d5719 100644
--- a/src/rabbit_auth_backend.erl
+++ b/src/rabbit_auth_backend.erl
@@ -16,8 +16,17 @@
 
 -module(rabbit_auth_backend).
 
+-include("rabbit.hrl").
+
 -ifdef(use_specs).
 
+-export_type([auth_user/0]).
+
+-type(auth_user() ::
+        #auth_user{username :: rabbit_types:username(),
+                   tags     :: [atom()],
+                   impl     :: any()}).
+
 %% A description proplist as with auth mechanisms,
 %% exchanges. Currently unused.
 -callback description() -> [proplists:property()].
@@ -33,7 +42,7 @@
 %% {refused, Msg, Args}
 %%     Client failed authentication. Log and die.
 -callback check_user_login(rabbit_types:username(), [term()]) ->
-    {'ok', rabbit_types:user(), any()} |
+    {'ok', auth_user()} |
     {'refused', string(), [any()]} |
     {'error', any()}.
 
@@ -43,11 +52,10 @@
 %% false
 %% {error, Error}
 %%     Something went wrong. Log and die.
--callback check_vhost_access(rabbit_types:user(), any(),
+-callback check_vhost_access(auth_user(),
                              rabbit_types:vhost(), rabbit_net:socket()) ->
     boolean() | {'error', any()}.
 
-
 %% Given #user, resource and permission, can a user access a resource?
 %%
 %% Possible responses:
@@ -55,7 +63,7 @@
 %% false
 %% {error, Error}
 %%     Something went wrong. Log and die.
--callback check_resource_access(rabbit_types:user(), any(),
+-callback check_resource_access(auth_user(),
                                 rabbit_types:r(atom()),
                                 rabbit_access_control:permission_atom()) ->
     boolean() | {'error', any()}.
@@ -65,8 +73,8 @@
 -export([behaviour_info/1]).
 
 behaviour_info(callbacks) ->
-    [{description, 0}, {check_user_login, 2}, {check_vhost_access, 4},
-     {check_resource_access, 4}];
+    [{description, 0}, {check_user_login, 2}, {check_vhost_access, 3},
+     {check_resource_access, 3}];
 behaviour_info(_Other) ->
     undefined.
 
diff --git a/src/rabbit_auth_backend_dummy.erl b/src/rabbit_auth_backend_dummy.erl
index 8e2d8269..e279e4bf 100644
--- a/src/rabbit_auth_backend_dummy.erl
+++ b/src/rabbit_auth_backend_dummy.erl
@@ -21,7 +21,7 @@
 
 -export([description/0]).
 -export([user/0]).
--export([check_user_login/2, check_vhost_access/4, check_resource_access/4]).
+-export([check_user_login/2, check_vhost_access/3, check_resource_access/3]).
 
 -ifdef(use_specs).
 
@@ -33,8 +33,11 @@
 %% not needed. This user can do anything AMQPish.
 user() -> #user{username       = <<"none">>,
                 tags           = [],
-                authN_backend  = ?MODULE,
-                authZ_backends = []}.
+                authz_backends = [{?MODULE, buser()}]}.
+
+buser() -> #auth_user{username = <<"none">>,
+                      tags     = [],
+                      impl     = none}.
 
 %% Implementation of rabbit_auth_backend
 
@@ -45,5 +48,5 @@ description() ->
 check_user_login(_, _) ->
     {refused, "cannot log in conventionally as dummy user", []}.
 
-check_vhost_access(#user{}, _Impl, _VHostPath, _Sock) -> true.
-check_resource_access(#user{}, _Impl, #resource{}, _Permission) -> true.
+check_vhost_access(#auth_user{}, _VHostPath, _Sock) -> true.
+check_resource_access(#auth_user{}, #resource{}, _Permission) -> true.
diff --git a/src/rabbit_auth_backend_internal.erl b/src/rabbit_auth_backend_internal.erl
index 5cdff985..c8f09be9 100644
--- a/src/rabbit_auth_backend_internal.erl
+++ b/src/rabbit_auth_backend_internal.erl
@@ -20,7 +20,7 @@
 -behaviour(rabbit_auth_backend).
 
 -export([description/0]).
--export([check_user_login/2, check_vhost_access/4, check_resource_access/4]).
+-export([check_user_login/2, check_vhost_access/3, check_resource_access/3]).
 
 -export([add_user/2, delete_user/1, lookup_user/1,
          change_password/2, clear_password/1,
@@ -98,16 +98,16 @@ internal_check_user_login(Username, Fun) ->
     case lookup_user(Username) of
         {ok, User = #internal_user{tags = Tags}} ->
             case Fun(User) of
-                true -> {ok, #user{username      = Username,
-                                   tags          = Tags,
-                                   authN_backend = ?MODULE}, User};
+                true -> {ok, #auth_user{username = Username,
+                                        tags     = Tags,
+                                        impl     = none}};
                 _    -> Refused
             end;
         {error, not_found} ->
             Refused
     end.
 
-check_vhost_access(#user{username = Username}, _Impl, VHostPath, _Sock) ->
+check_vhost_access(#auth_user{username = Username}, VHostPath, _Sock) ->
     case mnesia:dirty_read({rabbit_user_permission,
                             #user_vhost{username     = Username,
                                         virtual_host = VHostPath}}) of
@@ -115,7 +115,7 @@ check_vhost_access(#user{username = Username}, _Impl, VHostPath, _Sock) ->
         [_R] -> true
     end.
 
-check_resource_access(#user{username = Username}, _Impl,
+check_resource_access(#auth_user{username = Username},
                       #resource{virtual_host = VHostPath, name = Name},
                       Permission) ->
     case mnesia:dirty_read({rabbit_user_permission,
diff --git a/src/rabbit_channel.erl b/src/rabbit_channel.erl
index 2f7e234d..13cc925c 100644
--- a/src/rabbit_channel.erl
+++ b/src/rabbit_channel.erl
@@ -581,7 +581,8 @@ check_user_id_header(#'P_basic'{user_id = Username},
                      #ch{user = #user{username = Username}}) ->
     ok;
 check_user_id_header(
-  #'P_basic'{}, #ch{user = #user{authN_backend = rabbit_auth_backend_dummy}}) ->
+  #'P_basic'{}, #ch{user = #user{authz_backends =
+                                     [{rabbit_auth_backend_dummy, _}]}}) ->
     ok;
 check_user_id_header(#'P_basic'{user_id = Claimed},
                      #ch{user = #user{username = Actual,
diff --git a/src/rabbit_types.erl b/src/rabbit_types.erl
index b3158cc8..27fbae88 100644
--- a/src/rabbit_types.erl
+++ b/src/rabbit_types.erl
@@ -134,8 +134,7 @@
 -type(user() ::
         #user{username       :: username(),
               tags           :: [atom()],
-              authN_backend  :: atom(),
-              authZ_backends :: [{atom(), any()}]}).
+              authz_backends :: [{atom(), any()}]}).
 
 -type(internal_user() ::
         #internal_user{username      :: username(),