Tweak the APIs again, so that authz plugins aren't expected to create a #auth_user record in the first place, just whatever impl they want. Which necessitates seperate login functions for authz and authn, and if we're going to do that we might as well split the behaviours so that we have the possibility of making an authz-only plugin.bug26469

8 files changed, 120 insertions, 71 deletions

diff --git a/include/rabbit.hrl b/include/rabbit.hrl
index 86c30fc5..9cbd978e 100644
--- a/include/rabbit.hrl
+++ b/include/rabbit.hrl
@@ -17,7 +17,7 @@
 %% Passed around most places
 -record(user, {username,
                tags,
-               authz_backends}). %% List of {Module, AuthUser} pairs
+               authz_backends}). %% List of {Module, AuthUserImpl} pairs
 
 %% Passed to auth backends
 -record(auth_user, {username,
diff --git a/src/rabbit_access_control.erl b/src/rabbit_access_control.erl
index 0ebd2fcf..d1577432 100644
--- a/src/rabbit_access_control.erl
+++ b/src/rabbit_access_control.erl
@@ -73,8 +73,10 @@ check_user_login(Username, AuthProps) ->
                   %% Same module for authN and authZ. Just take the result
                   %% it gives us
                   case try_authenticate(Mod, Username, AuthProps) of
-                      {ok, ModNUser} -> user(ModNUser, {ok, [{Mod, ModNUser}]});
-                      Else           -> Else
+                      {ok, ModNUser = #auth_user{impl = Impl}} ->
+                          user(ModNUser, {ok, [{Mod, Impl}]});
+                      Else ->
+                          Else
                   end;
               (_, {ok, User}) ->
                   %% We've successfully authenticated. Skip to the end...
@@ -87,7 +89,7 @@ check_user_login(Username, AuthProps) ->
     R.
 
 try_authenticate(Module, Username, AuthProps) ->
-    case Module:check_user_login(Username, AuthProps) of
+    case Module:user_login_authentication(Username, AuthProps) of
         {ok, AuthUser}  -> {ok, AuthUser};
         {error, E}      -> {refused, "~s failed authenticating ~s: ~p~n",
                             [Module, Username, E]};
@@ -96,9 +98,9 @@ try_authenticate(Module, Username, AuthProps) ->
 
 try_authorize(Modules, Username) ->
     lists:foldr(
-      fun (Module, {ok, AUsers}) ->
-              case Module:check_user_login(Username, []) of
-                  {ok, AUser}     -> {ok, [{Module, AUser} | AUsers]};
+      fun (Module, {ok, ModsImpls}) ->
+              case Module:user_login_authorization(Username) of
+                  {ok, Impl}      -> {ok, [{Module, Impl} | ModsImpls]};
                   {error, E}      -> {refused, "~s failed authorizing ~s: ~p~n",
                                         [Module, Username, E]};
                   {refused, F, A} -> {refused, F, A}
@@ -107,13 +109,18 @@ try_authorize(Modules, Username) ->
               Error
       end, {ok, []}, Modules).
 
-user(#auth_user{username = Username, tags = Tags}, {ok, ModZUsers}) ->
+user(#auth_user{username = Username, tags = Tags}, {ok, ModZImpls}) ->
     {ok, #user{username       = Username,
                tags           = Tags,
-               authz_backends = ModZUsers}};
+               authz_backends = ModZImpls}};
 user(_AuthUser, Error) ->
     Error.
 
+auth_user(#user{username = Username, tags = Tags}, Impl) ->
+    #auth_user{username = Username,
+               tags     = Tags,
+               impl     = Impl}.
+
 check_user_loopback(Username, SockOrAddr) ->
     {ok, Users} = application:get_env(rabbit, loopback_users),
     case rabbit_net:is_loopback(SockOrAddr)
@@ -122,14 +129,15 @@ check_user_loopback(Username, SockOrAddr) ->
         false -> not_allowed
     end.
 
-check_vhost_access(#user{username       = Username,
-                         authz_backends = Modules}, VHostPath, Sock) ->
+check_vhost_access(User = #user{username       = Username,
+                                authz_backends = Modules}, VHostPath, Sock) ->
     lists:foldl(
-      fun({Mod, AUser}, ok) ->
+      fun({Mod, Impl}, ok) ->
               check_access(
                 fun() ->
                         rabbit_vhost:exists(VHostPath) andalso
-                            Mod:check_vhost_access(AUser, VHostPath, Sock)
+                            Mod:check_vhost_access(
+                              auth_user(User, Impl), VHostPath, Sock)
                 end,
                 Mod, "access to vhost '~s' refused for user '~s'",
                 [VHostPath, Username]);
@@ -141,14 +149,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{username       = Username,
-                            authz_backends = Modules},
+check_resource_access(User = #user{username       = Username,
+                                   authz_backends = Modules},
                       Resource, Permission) ->
     lists:foldl(
-      fun({Module, AUser}, ok) ->
+      fun({Module, Impl}, ok) ->
               check_access(
                 fun() -> Module:check_resource_access(
-                           AUser, Resource, Permission) end,
+                           auth_user(User, Impl), 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_dummy.erl b/src/rabbit_auth_backend_dummy.erl
index d2905334..d2f07c1d 100644
--- a/src/rabbit_auth_backend_dummy.erl
+++ b/src/rabbit_auth_backend_dummy.erl
@@ -17,11 +17,12 @@
 -module(rabbit_auth_backend_dummy).
 -include("rabbit.hrl").
 
--behaviour(rabbit_auth_backend).
+-behaviour(rabbit_authn_backend).
+-behaviour(rabbit_authz_backend).
 
--export([description/0]).
 -export([user/0]).
--export([check_user_login/2, check_vhost_access/3, check_resource_access/3]).
+-export([user_login_authentication/2, user_login_authorization/1,
+         check_vhost_access/3, check_resource_access/3]).
 
 -ifdef(use_specs).
 
@@ -33,19 +34,14 @@
 %% not needed. This user can do anything AMQPish.
 user() -> #user{username       = <<"none">>,
                 tags           = [],
-                authz_backends = [{?MODULE, auser()}]}.
-
-auser() -> #auth_user{username = <<"none">>,
-                      tags     = [],
-                      impl     = none}.
+                authz_backends = [{?MODULE, none}]}.
 
 %% Implementation of rabbit_auth_backend
 
-description() ->
-    [{name, <<"Dummy">>},
-     {description, <<"Database for the dummy user">>}].
+user_login_authentication(_, _) ->
+    {refused, "cannot log in conventionally as dummy user", []}.
 
-check_user_login(_, _) ->
+user_login_authorization(_) ->
     {refused, "cannot log in conventionally as dummy user", []}.
 
 check_vhost_access(#auth_user{}, _VHostPath, _Sock) -> true.
diff --git a/src/rabbit_auth_backend_internal.erl b/src/rabbit_auth_backend_internal.erl
index c8f09be9..20a5766d 100644
--- a/src/rabbit_auth_backend_internal.erl
+++ b/src/rabbit_auth_backend_internal.erl
@@ -17,10 +17,11 @@
 -module(rabbit_auth_backend_internal).
 -include("rabbit.hrl").
 
--behaviour(rabbit_auth_backend).
+-behaviour(rabbit_authn_backend).
+-behaviour(rabbit_authz_backend).
 
--export([description/0]).
--export([check_user_login/2, check_vhost_access/3, check_resource_access/3]).
+-export([user_login_authentication/2, user_login_authorization/1,
+         check_vhost_access/3, check_resource_access/3]).
 
 -export([add_user/2, delete_user/1, lookup_user/1,
          change_password/2, clear_password/1,
@@ -76,13 +77,9 @@
 %%----------------------------------------------------------------------------
 %% Implementation of rabbit_auth_backend
 
-description() ->
-    [{name, <<"Internal">>},
-     {description, <<"Internal user / password database">>}].
-
-check_user_login(Username, []) ->
+user_login_authentication(Username, []) ->
     internal_check_user_login(Username, fun(_) -> true end);
-check_user_login(Username, [{password, Cleartext}]) ->
+user_login_authentication(Username, [{password, Cleartext}]) ->
     internal_check_user_login(
       Username,
       fun (#internal_user{password_hash = <<Salt:4/binary, Hash/binary>>}) ->
@@ -90,9 +87,15 @@ check_user_login(Username, [{password, Cleartext}]) ->
           (#internal_user{}) ->
               false
       end);
-check_user_login(Username, AuthProps) ->
+user_login_authentication(Username, AuthProps) ->
     exit({unknown_auth_props, Username, AuthProps}).
 
+user_login_authorization(Username) ->
+    case user_login_authentication(Username, []) of
+        {ok, #auth_user{impl = Impl}} -> {ok, Impl};
+        Else                          -> Else
+    end.
+
 internal_check_user_login(Username, Fun) ->
     Refused = {refused, "user '~s' - invalid credentials", [Username]},
     case lookup_user(Username) of
diff --git a/src/rabbit_authn_backend.erl b/src/rabbit_authn_backend.erl
new file mode 100644
index 00000000..cfc3f5db
--- /dev/null
+++ b/src/rabbit_authn_backend.erl
@@ -0,0 +1,49 @@
+%% The contents of this file are subject to the Mozilla Public License
+%% Version 1.1 (the "License"); you may not use this file except in
+%% compliance with the License. You may obtain a copy of the License
+%% at http://www.mozilla.org/MPL/
+%%
+%% Software distributed under the License is distributed on an "AS IS"
+%% basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See
+%% the License for the specific language governing rights and
+%% limitations under the License.
+%%
+%% The Original Code is RabbitMQ.
+%%
+%% The Initial Developer of the Original Code is GoPivotal, Inc.
+%% Copyright (c) 2007-2014 GoPivotal, Inc.  All rights reserved.
+%%
+
+-module(rabbit_authn_backend).
+
+-include("rabbit.hrl").
+
+-ifdef(use_specs).
+
+%% Check a user can log in, given a username and a proplist of
+%% authentication information (e.g. [{password, Password}]). If your
+%% backend is not to be used for authentication, this should always
+%% refuse access.
+%%
+%% Possible responses:
+%% {ok, User}
+%%     Authentication succeeded, and here's the user record.
+%% {error, Error}
+%%     Something went wrong. Log and die.
+%% {refused, Msg, Args}
+%%     Client failed authentication. Log and die.
+-callback user_login_authentication(rabbit_types:username(), [term()]) ->
+    {'ok', rabbit_types:auth_user()} |
+    {'refused', string(), [any()]} |
+    {'error', any()}.
+
+-else.
+
+-export([behaviour_info/1]).
+
+behaviour_info(callbacks) ->
+    [{user_login_authentication, 2}];
+behaviour_info(_Other) ->
+    undefined.
+
+-endif.
diff --git a/src/rabbit_auth_backend.erl b/src/rabbit_authz_backend.erl
index 315d5719..ff5f014e 100644
--- a/src/rabbit_auth_backend.erl
+++ b/src/rabbit_authz_backend.erl
@@ -14,56 +14,49 @@
 %% Copyright (c) 2007-2014 GoPivotal, Inc.  All rights reserved.
 %%
 
--module(rabbit_auth_backend).
+-module(rabbit_authz_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()].
-
-%% Check a user can log in, given a username and a proplist of
-%% authentication information (e.g. [{password, Password}]).
+%% Check a user can log in, when this backend is being used for
+%% authorisation only. Authentication has already taken place
+%% successfully, but we need to check that the user exists in this
+%% backend, and initialise any impl field we will want to have passed
+%% back in future calls to check_vhost_access/3 and
+%% check_resource_access/3.
 %%
 %% Possible responses:
-%% {ok, User}
-%%     Authentication succeeded, and here's the user record.
+%% {ok, Impl}
+%%     User authorisation succeeded, and here's the impl field.
 %% {error, Error}
 %%     Something went wrong. Log and die.
 %% {refused, Msg, Args}
-%%     Client failed authentication. Log and die.
--callback check_user_login(rabbit_types:username(), [term()]) ->
-    {'ok', auth_user()} |
+%%     User authorisation failed. Log and die.
+-callback user_login_authorization(rabbit_types:username()) ->
+    {'ok', any()} |
     {'refused', string(), [any()]} |
     {'error', any()}.
 
-%% Given #user and vhost, can a user log in to a vhost?
+%% Given #auth_user and vhost, can a user log in to a vhost?
 %% Possible responses:
 %% true
 %% false
 %% {error, Error}
 %%     Something went wrong. Log and die.
--callback check_vhost_access(auth_user(),
+-callback check_vhost_access(rabbit_types:auth_user(),
                              rabbit_types:vhost(), rabbit_net:socket()) ->
     boolean() | {'error', any()}.
 
-%% Given #user, resource and permission, can a user access a resource?
+%% Given #auth_user, resource and permission, can a user access a resource?
 %%
 %% Possible responses:
 %% true
 %% false
 %% {error, Error}
 %%     Something went wrong. Log and die.
--callback check_resource_access(auth_user(),
+-callback check_resource_access(rabbit_types:auth_user(),
                                 rabbit_types:r(atom()),
                                 rabbit_access_control:permission_atom()) ->
     boolean() | {'error', any()}.
@@ -73,8 +66,8 @@
 -export([behaviour_info/1]).
 
 behaviour_info(callbacks) ->
-    [{description, 0}, {check_user_login, 2}, {check_vhost_access, 3},
-     {check_resource_access, 3}];
+    [{user_login_authorization, 1},
+     {check_vhost_access, 3}, {check_resource_access, 3}];
 behaviour_info(_Other) ->
     undefined.
 
diff --git a/src/rabbit_types.erl b/src/rabbit_types.erl
index 27fbae88..039568df 100644
--- a/src/rabbit_types.erl
+++ b/src/rabbit_types.erl
@@ -27,7 +27,7 @@
               vhost/0, ctag/0, amqp_error/0, r/1, r2/2, r3/3, listener/0,
               binding/0, binding_source/0, binding_destination/0,
               amqqueue/0, exchange/0,
-              connection/0, protocol/0, user/0, internal_user/0,
+              connection/0, protocol/0, auth_user/0, user/0, internal_user/0,
               username/0, password/0, password_hash/0,
               ok/1, error/1, ok_or_error/1, ok_or_error2/2, ok_pid_or_error/0,
               channel_exit/0, connection_exit/0, mfargs/0, proc_name/0,
@@ -131,6 +131,11 @@
 
 -type(protocol() :: rabbit_framing:protocol()).
 
+-type(auth_user() ::
+        #auth_user{username :: username(),
+                   tags     :: [atom()],
+                   impl     :: any()}).
+
 -type(user() ::
         #user{username       :: username(),
               tags           :: [atom()],
diff --git a/test/src/rabbit_tests.erl b/test/src/rabbit_tests.erl
index e614bfd7..dcbec8f6 100644
--- a/test/src/rabbit_tests.erl
+++ b/test/src/rabbit_tests.erl
@@ -1294,12 +1294,7 @@ test_spawn_remote() ->
 user(Username) ->
     #user{username       = Username,
           tags           = [administrator],
-          authz_backends = [{rabbit_auth_backend_internal, auser(Username)}]}.
-
-auser(Username) ->
-    #auth_user{username = Username,
-               tags     = [administrator],
-               impl     = none}.
+          authz_backends = [{rabbit_auth_backend_internal, none}]}.
 
 test_confirms() ->
     {_Writer, Ch} = test_spawn(),