* Remove unsupported "all" meta-permission from riak_core_console

* Correct recent "succesfully" typo * riak_core_security:del_source/2 cannot return errors, so don't watch for them in console * Remove last function clause from both add_source/4 and del_source/2 to avoid infinite loop problem previously identified with add_grant/add_revoke * Update type specs: user lists are binary(), not string(), and update add_source to reflect loss of the last clause * Update unit tests to reflect update to add_source/4 clause
basho · Mar 5, 2014 · 8613da2 · 8613da2
1 parent afbab36
commit 8613da2
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 55 deletions.
diff --git a/src/riak_core_console.erl b/src/riak_core_console.erl
@@ -896,7 +896,7 @@ add_group([Groupname|Options]) ->
 add_role(Name, Options, Fun) ->
     try Fun(list_to_binary(Name), parse_options(Options)) of
         ok ->
-            io:format("Succesfully added ~s~n", [Name]),
+            io:format("Successfully added ~s~n", [Name]),
             ok;
         Error ->
             io:format(security_error_xlate(Error)),
@@ -940,7 +940,7 @@ del_group([Groupname]) ->
 del_role(Name, Fun) ->
     case Fun(list_to_binary(Name)) of
         ok ->
-            io:format("Succesfully deleted ~s~n", [Name]),
+            io:format("Successfully deleted ~s~n", [Name]),
             ok;
         Error ->
             io:format(security_error_xlate(Error)),
@@ -959,7 +959,7 @@ add_source([Users, CIDR, Source | Options]) ->
                                   list_to_atom(Source),
                                   parse_options(Options)) of
         ok ->
-            io:format("Succesfully added source~n"),
+            io:format("Successfully added source~n"),
             ok;
         Error ->
             io:format(security_error_xlate(Error)),
@@ -979,15 +979,8 @@ del_source([Users, CIDR]) ->
         Other ->
             [list_to_binary(O) || O <- Other]
     end,
-    case riak_core_security:del_source(Unames, parse_cidr(CIDR)) of
-        ok ->
-            io:format("Succesfully deleted source~n"),
-            ok;
-        Error ->
-            io:format(security_error_xlate(Error)),
-            io:format("~n"),
-            Error
-    end.
+    riak_core_security:del_source(Unames, parse_cidr(CIDR)),
+    io:format("Deleted source~n").
 
 grant([Grants, "ON", "ANY", "TO", Users]) ->
     Unames = case string:tokens(Users, ",") of
@@ -996,15 +989,10 @@ grant([Grants, "ON", "ANY", "TO", Users]) ->
         Other ->
             [list_to_binary(O) || O <- Other]
     end,
-    Permissions = case string:tokens(Grants, ",") of
-        ["all"] ->
-            all;
-        Other2 ->
-            Other2
-    end,
+    Permissions = string:tokens(Grants, ","),
     case riak_core_security:add_grant(Unames, any, Permissions) of
         ok ->
-            io:format("Succesfully granted~n"),
+            io:format("Successfully granted~n"),
             ok;
         Error ->
             io:format(security_error_xlate(Error)),
@@ -1023,15 +1011,10 @@ grant([Grants, "ON", Bucket, "TO", Users]) ->
         Other ->
             [list_to_binary(O) || O <- Other]
     end,
-    Permissions = case string:tokens(Grants, ",") of
-        ["all"] ->
-            all;
-        Other2 ->
-            Other2
-    end,
+    Permissions = string:tokens(Grants, ","),
     case riak_core_security:add_grant(Unames, Bucket, Permissions) of
         ok ->
-            io:format("Succesfully granted~n"),
+            io:format("Successfully granted~n"),
             ok;
         Error ->
             io:format(security_error_xlate(Error)),
@@ -1049,15 +1032,10 @@ revoke([Grants, "ON", "ANY", "FROM", Users]) ->
         Other ->
             [list_to_binary(O) || O <- Other]
     end,
-    Permissions = case string:tokens(Grants, ",") of
-        ["all"] ->
-            all;
-        Other2 ->
-            Other2
-    end,
+    Permissions = string:tokens(Grants, ","),
     case riak_core_security:add_revoke(Unames, any, Permissions) of
         ok ->
-            io:format("Succesfully revoked~n"),
+            io:format("Successfully revoked~n"),
             ok;
         Error ->
             io:format(security_error_xlate(Error)),
@@ -1077,15 +1055,10 @@ revoke([Grants, "ON", Bucket, "FROM", Users]) ->
         Other ->
             [list_to_binary(O) || O <- Other]
     end,
-    Permissions = case string:tokens(Grants, ",") of
-        ["all"] ->
-            all;
-        Other2 ->
-            Other2
-    end,
+    Permissions = string:tokens(Grants, ","),
     case riak_core_security:add_revoke(Unames, Bucket, Permissions) of
         ok ->
-            io:format("Succesfully revoked~n"),
+            io:format("Successfully revoked~n"),
             ok;
         Error ->
             io:format(security_error_xlate(Error)),

diff --git a/src/riak_core_security.erl b/src/riak_core_security.erl
@@ -73,7 +73,7 @@
 -type context() :: #context{}.
 -type bucket() :: {binary(), binary()} | binary().
 -type permission() :: {string()} | {string(), bucket()}.
--type userlist() :: all | string() | [string()].
+-type userlist() :: all | [binary()].
 
 prettyprint_users([all], _) ->
     "all";
@@ -619,8 +619,8 @@ add_revoke([H|_T]=RoleList, Bucket, Revokes) when is_binary(H) ->
             Error
     end.
 
--spec add_source(Users :: all | binary() | [binary()], CIDR ::
-                 {inet:ip_address(), non_neg_integer()}, Source :: atom(),
+-spec add_source(userlist(), CIDR :: {inet:ip_address(), non_neg_integer()},
+                 Source :: atom(),
                  Options :: [{string(), term()}]) -> ok | {error, term()}.
 add_source(all, CIDR, Source, Options) ->
     %% all is always valid
@@ -656,10 +656,7 @@ add_source([H|_T]=UserList, CIDR, Source, Options) when is_binary(H) ->
             ok;
         Error ->
             Error
-    end;
-add_source(User, CIDR, Source, Options) ->
-    %% single user
-    add_source([User], CIDR, Source, Options).
+    end.
 
 del_source(all, CIDR) ->
     %% all is always valid
@@ -669,11 +666,7 @@ del_source(all, CIDR) ->
 del_source([H|_T]=UserList, CIDR) when is_binary(H) ->
     _ = [riak_core_metadata:delete({<<"security">>, <<"sources">>},
                                    {User, anchor_mask(CIDR)}) || User <- UserList],
-    ok;
-del_source(User, CIDR) ->
-    %% single user
-    del_source([User], CIDR).
-
+    ok.
 
 is_enabled() ->
     try riak_core_capability:get({riak_core, security}) of

diff --git a/test/riak_core_security_tests.erl b/test/riak_core_security_tests.erl
@@ -113,7 +113,7 @@ security_test_() ->
                                             [{"password","password"}])),
                 ?assertEqual(ok, riak_core_security:add_group(<<"group">>,
                                             [])),
-                ?assertEqual(ok, riak_core_security:add_source(<<"user">>, {{127, 0, 0, 1}, 32}, password, [])),
+                ?assertEqual(ok, riak_core_security:add_source([<<"user">>], {{127, 0, 0, 1}, 32}, password, [])),
                 {ok, Ctx} = riak_core_security:authenticate(<<"user">>, <<"password">>,
                                                 [{ip, {127, 0, 0, 1}}]),
                 ?assertMatch({false, _,  _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
@@ -143,7 +143,7 @@ security_test_() ->
         fun() ->
                 ?assertEqual(ok, riak_core_security:add_user(<<"user">>,
                                             [{"password","password"}])),
-                ?assertEqual(ok, riak_core_security:add_source(<<"user">>, {{127, 0, 0, 1}, 32}, password, [])),
+                ?assertEqual(ok, riak_core_security:add_source([<<"user">>], {{127, 0, 0, 1}, 32}, password, [])),
                 {ok, Ctx} = riak_core_security:authenticate(<<"user">>, <<"password">>,
                                                 [{ip, {127, 0, 0, 1}}]),
                 ?assertMatch({false, _,  _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
@@ -166,7 +166,7 @@ security_test_() ->
                 ?assertEqual(ok, riak_core_security:add_group(<<"superuser">>, [{"groups", ["sysadmin"]}])),
                 ?assertEqual(ok, riak_core_security:add_user(<<"user">>,
                                             [{"password","password"}])),
-                ?assertEqual(ok, riak_core_security:add_source(<<"user">>, {{127, 0, 0, 1}, 32}, password, [])),
+                ?assertEqual(ok, riak_core_security:add_source([<<"user">>], {{127, 0, 0, 1}, 32}, password, [])),
                 %% sysadmins can get/put on any key in a default bucket
                 ?assertEqual(ok, riak_core_security:add_grant([<<"sysadmin">>], <<"default">>, ["riak_kv.get", "riak_kv.put"])),
                 %% authenticating from the wrong IP