Skip to content

Loading…

Cleanup for permissions assigned to 'all' #536

Merged
merged 8 commits into from

3 participants

@macintux
Basho Technologies member
  • When granting or revoking, treat 'all' as a group
  • Fix typo that prevented revocation from working for 'all'
  • Update accumulate_grants/2 to pull grants assigned to 'all'
  • Block admins from using 'all' as a user or group

Ancillary change, but useful for verifying above and for admins:

  • Display grants to a group or user separate from inherited

/cc @Vagabond

@Vagabond

So is this intended to be used like:

GRANT riak_kv.get ON SomeType SomeBucket TO all

Rather than

GRANT all ON SomeType SomeBucket TO andrew

@Vagabond

I pushed some WIP eunit tests and specs to this branch, currently getting about 40% coverage on riak_core_security.erl

Will work on it more tomorrow.

macintux and others added some commits
@macintux macintux Cleanup for permissions assigned to 'all':
* When granting or revoking, treat 'all' as a group
* Fix typo that prevented revocation from working for 'all'
* Update accumulate_grants/2 to pull grants assigned to 'all'
* Block admins from using 'all' as a user or group

Ancillary change, but useful for verifying above and for admins:

* Display grants to a group or user separate from inherited
a629bb5
@Vagabond Vagabond Add eunit tests 2fe2eea
@seancribbs seancribbs Port dialyzer warning fixes from #528. daf06af
@seancribbs seancribbs Fix some more dialyzer warnings.
riak_core_console:

* del_source/2 and add_grant/3 always returns 'ok'.
* Make sure that parse_cidr/1 and parse_options() match up with the
  types expected from riak_core_security.

riak_core_security:

* add_grant/3 and add_revoke/3 accept 'any' as a bucket name.
* add_source/4 uses Source as an atom.
5667c5f
@Vagabond

Rebased against develop.

@Vagabond

I still need to add more eunit tests, will try to wrap this up on Monday.

@macintux macintux commented on an outdated diff
test/riak_core_security_tests.erl
((155 lines not shown))
+ ?assertMatch({false, _, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ %% make sure these don't crash, at least
+ ?assertEqual(ok, riak_core_security:print_users()),
+ ?assertEqual(ok, riak_core_security:print_sources()),
+ ?assertEqual(ok, riak_core_security:print_user(<<"user">>)),
+ ?assertEqual(ok, riak_core_security:print_groups()),
+ ?assertEqual({error, {unknown_group, <<"all">>}}, riak_core_security:print_group(<<"all">>)),
+ ok
+ end},
+ { "groups can be members of groups and inherit permissions from them",
+ fun() ->
+ ?assertEqual(ok, riak_core_security:add_group(<<"sysadmin">>, [])),
+ ?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, [{"groups", ["superuser"]}])),
@macintux Basho Technologies member
macintux added a note

There are no useful options as arguments to add_source, so groups=superuser lacks a purpose here.

Update: On second thought, since this test also calls print_sources, it's useful to have an option here to make certain those are handled without throwing an error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Vagabond

Fixed useless arguments to add_source.

@macintux macintux commented on an outdated diff
test/riak_core_security_tests.erl
((52 lines not shown))
+ %% make sure these don't crash, at least
+ ?assertEqual(ok, riak_core_security:print_users()),
+ ?assertEqual(ok, riak_core_security:print_sources()),
+ ?assertEqual(ok, riak_core_security:print_user(<<"user">>)),
+ ok
+ end},
+ { "password auth works",
+ fun() ->
+ ?assertMatch({error, _}, riak_core_security:authenticate(<<"user">>, <<"password">>,
+ [{ip, {127, 0, 0, 1}}])),
+ ?assertEqual(ok, riak_core_security:add_user(<<"user">>,
+ [])),
+ ?assertMatch({error, _}, riak_core_security:authenticate(<<"user">>, <<"password">>,
+ [{ip, {127, 0, 0, 1}}])),
+ ?assertEqual(ok, riak_core_security:add_source(all, {{127, 0, 0, 1}, 32}, password, [])),
+ ?assertMatch({error, _}, riak_core_security:authenticate(<<"user">>, <<"badpassword">>,
@macintux Basho Technologies member
macintux added a note

This line is redundant with the next. There is no password defined yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@macintux macintux commented on an outdated diff
test/riak_core_security_tests.erl
((93 lines not shown))
+ ?assertEqual(ok, riak_core_security:add_revoke(<<"user">>, {<<"default">>, <<"mybucket">>}, ["riak_kv.get"])),
+ ?assertMatch({error, {unknown_permission, _}}, riak_core_security:add_revoke(<<"user">>, {<<"default">>, <<"mybucket">>}, ["riak_kv.upsert"])),
+ ?assertMatch({false, _, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ %% make sure these don't crash, at least
+ ?assertEqual(ok, riak_core_security:print_users()),
+ ?assertEqual(ok, riak_core_security:print_sources()),
+ ?assertEqual(ok, riak_core_security:print_user(<<"user">>)),
+ %% delete the user
+ ?assertMatch(ok, riak_core_security:del_user(<<"user">>)),
+ %% re-add them
+ ?assertEqual(ok, riak_core_security:add_user(<<"user">>,
+ [{"password","password"}])),
+ %% make sure their old grants are gone
+ {ok, Ctx2} = riak_core_security:authenticate(<<"user">>, <<"password">>,
+ [{ip, {127, 0, 0, 1}}]),
+ ?assertMatch({false, _, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx2)),
@macintux Basho Technologies member
macintux added a note

This is a bogus test: riak_kv.get was revoked before the account was deleted. Should add two permissions, revoke one before delete, then delete, and check the other after re-adding the account.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@macintux
Basho Technologies member

+1 to unit tests after latest round of changes

@Vagabond Vagabond merged commit 3c00369 into develop

1 check failed

Details default Build done.
@Vagabond Vagabond deleted the jrd-fix-all-metarole branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 1, 2014
  1. @macintux @Vagabond

    Cleanup for permissions assigned to 'all':

    macintux committed with Vagabond
    * When granting or revoking, treat 'all' as a group
    * Fix typo that prevented revocation from working for 'all'
    * Update accumulate_grants/2 to pull grants assigned to 'all'
    * Block admins from using 'all' as a user or group
    
    Ancillary change, but useful for verifying above and for admins:
    
    * Display grants to a group or user separate from inherited
  2. @Vagabond

    Add eunit tests

    Vagabond committed
  3. @seancribbs @Vagabond
  4. @seancribbs @Vagabond

    Fix some more dialyzer warnings.

    seancribbs committed with Vagabond
    riak_core_console:
    
    * del_source/2 and add_grant/3 always returns 'ok'.
    * Make sure that parse_cidr/1 and parse_options() match up with the
      types expected from riak_core_security.
    
    riak_core_security:
    
    * add_grant/3 and add_revoke/3 accept 'any' as a bucket name.
    * add_source/4 uses Source as an atom.
Commits on Mar 3, 2014
  1. @Vagabond
  2. @Vagabond
  3. @Vagabond

    Fix typo

    Vagabond committed
  4. @Vagabond

    Address review concerns

    Vagabond committed
Showing with 330 additions and 55 deletions.
  1. +4 −15 src/riak_core_console.erl
  2. +101 −40 src/riak_core_security.erl
  3. +225 −0 test/riak_core_security_tests.erl
View
19 src/riak_core_console.erl
@@ -912,13 +912,7 @@ del_source([Users, CIDR]) ->
Other ->
[list_to_binary(O) || O <- Other]
end,
- case riak_core_security:del_source(Unames, parse_cidr(CIDR)) of
- ok ->
- ok;
- Error ->
- io:format("~p~n", [Error]),
- Error
- end.
+ riak_core_security:del_source(Unames, parse_cidr(CIDR)).
grant([Grants, "ON", "ANY", "TO", Users]) ->
Unames = case string:tokens(Users, ",") of
@@ -933,12 +927,7 @@ grant([Grants, "ON", "ANY", "TO", Users]) ->
Other2 ->
Other2
end,
- case riak_core_security:add_grant(Unames, any, Permissions) of
- ok -> ok;
- Error ->
- io:format("~p~n", [Error]),
- Error
- end;
+ riak_core_security:add_grant(Unames, any, Permissions);
grant([Grants, "ON", Type, Bucket, "TO", Users]) ->
grant([Grants, "ON", {list_to_binary(Type), list_to_binary(Bucket)}, "TO",
Users]);
@@ -1067,13 +1056,13 @@ parse_options([], Acc) ->
Acc;
parse_options([H|T], Acc) ->
case re:split(H, "=", [{parts, 2}, {return, list}]) of
- [Key, Value] ->
+ [Key, Value] when is_list(Key), is_list(Value) ->
parse_options(T, [{Key, Value}|Acc]);
_Other ->
throw({error, {invalid_option, H}})
end.
-
+-spec parse_cidr(string()) -> {inet:ip_address(), non_neg_integer()}.
parse_cidr(CIDR) ->
[IP, Mask] = string:tokens(CIDR, "/"),
{ok, Addr} = inet_parse:address(IP),
View
141 src/riak_core_security.erl
@@ -30,7 +30,6 @@
add_grant/3, add_revoke/3, check_permission/2, check_permissions/2,
get_username/1, is_enabled/0, enable/0, disable/0, status/0,
get_ciphers/0, set_ciphers/1, print_ciphers/0]).
-%% TODO add rm_source, API to deactivate/remove users
-define(DEFAULT_CIPHER_LIST,
"ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-GCM-SHA256"
@@ -60,11 +59,22 @@
-define(TOMBSTONE, '$deleted').
+-ifdef(TEST).
+-define(REFRESH_TIME, 1).
+-else.
+-define(REFRESH_TIME, 1000).
+-endif.
+
-record(context,
{username,
grants,
epoch}).
+-type context() :: #context{}.
+-type bucket() :: {binary(), binary()} | binary().
+-type permission() :: {string()} | {string(), bucket()}.
+-type userlist() :: all | string() | [string()].
+
prettyprint_users([all], _) ->
"all";
prettyprint_users(Users0, Width) ->
@@ -163,8 +173,24 @@ print_user(User) ->
end
end ||
{{Username, Bucket}, Permissions} <- Grants, Username /= <<"user/", User/binary>>]),
+ io:format("~nDedicated permissions (~ts)~n~n", [User]),
+ riak_core_console_table:print([{type, 10}, {bucket, 10}, {grants, 40}],
+ [begin
+ case Bucket of
+ any ->
+ ["*", "*",
+ prettyprint_permissions(Permissions, 40)];
+ {T, B} ->
+ [T, B,
+ prettyprint_permissions(Permissions, 40)];
+ T ->
+ [T, "*",
+ prettyprint_permissions(Permissions, 40)]
+ end
+ end ||
+ {{Username, Bucket}, Permissions} <- Grants, Username == <<"user/", User/binary>>]),
GroupedGrants = group_grants(Grants),
- io:format("~nApplied permissions~n~n"),
+ io:format("~nCumulative permissions (~ts)~n~n", [User]),
riak_core_console_table:print([{type, 10}, {bucket, 10}, {grants, 40}],
[begin
case Bucket of
@@ -206,8 +232,24 @@ print_group(Group) ->
end
end ||
{{Groupname, Bucket}, Permissions} <- Grants, Groupname /= Group]),
+ io:format("~nDedicated permissions (~ts)~n~n", [Group]),
+ riak_core_console_table:print([{type, 10}, {bucket, 10}, {grants, 40}],
+ [begin
+ case Bucket of
+ any ->
+ ["*", "*",
+ prettyprint_permissions(Permissions, 40)];
+ {T, B} ->
+ [T, B,
+ prettyprint_permissions(Permissions, 40)];
+ T ->
+ [T, "*",
+ prettyprint_permissions(Permissions, 40)]
+ end
+ end ||
+ {{Groupname, Bucket}, Permissions} <- Grants, chop_name(Groupname) == Group]),
GroupedGrants = group_grants(Grants),
- io:format("~nApplied permissions~n~n"),
+ io:format("~nCumulative permissions (~ts)~n~n", [Group]),
riak_core_console_table:print([{type, 10}, {bucket, 10}, {grants, 40}],
[begin
case Bucket of
@@ -241,6 +283,8 @@ prettyprint_permissions([Permission|Rest], Width, [H|T] =Acc) ->
prettyprint_permissions([Permission|Rest], Width, Acc) ->
prettyprint_permissions(Rest, Width, [[Permission] | Acc]).
+-spec check_permission(Permission :: permission(), Context :: context()) ->
+ {true, context()} | {false, string(), context()}.
check_permission({Permission}, Context0) ->
Context = maybe_refresh_context(Context0),
%% The user needs to have this permission applied *globally*
@@ -248,8 +292,7 @@ check_permission({Permission}, Context0) ->
%% permissions that don't tie to a particular bucket, like 'ping' and
%% 'stats'.
MatchG = match_grant(any, Context#context.grants),
- case MatchG /= undefined andalso
- (lists:member(Permission, MatchG) orelse MatchG == 'all') of
+ case lists:member(Permission, MatchG) of
true ->
{true, Context};
false ->
@@ -261,8 +304,7 @@ check_permission({Permission}, Context0) ->
check_permission({Permission, Bucket}, Context0) ->
Context = maybe_refresh_context(Context0),
MatchG = match_grant(Bucket, Context#context.grants),
- case MatchG /= undefined andalso
- (lists:member(Permission, MatchG) orelse MatchG == 'all') of
+ case lists:member(Permission, MatchG) of
true ->
{true, Context};
false ->
@@ -290,6 +332,8 @@ check_permissions([Permission|Rest], Ctx) ->
get_username(#context{username=Username}) ->
Username.
+-spec authenticate(Username::binary(), Password::binary(), ConnInfo ::
+ [{atom(), any()}]) -> {ok, context()} | {error, term()}.
authenticate(Username, Password, ConnInfo) ->
case user_details(Username) of
undefined ->
@@ -371,6 +415,10 @@ authenticate(Username, Password, ConnInfo) ->
end
end.
+-spec add_user(Username :: binary(), Options :: [{string(), term()}]) ->
+ ok | {error, term()}.
+add_user(<<"all">>, _Options) ->
+ {error, reserved_name};
add_user(Username, Options) ->
case user_exists(Username) of
false ->
@@ -386,6 +434,8 @@ add_user(Username, Options) ->
{error, user_exists}
end.
+add_group(<<"all">>, _Options) ->
+ {error, reserved_name};
add_group(Groupname, Options) ->
case group_exists(Groupname) of
false ->
@@ -401,6 +451,10 @@ add_group(Groupname, Options) ->
{error, group_exists}
end.
+-spec alter_user(Username :: binary(), Options :: [{string(), term()}]) ->
+ ok | {error, term()}.
+alter_user(<<"all">>, _Options) ->
+ {error, reserved_name};
alter_user(Username, Options) ->
case user_details(Username) of
undefined ->
@@ -419,6 +473,8 @@ alter_user(Username, Options) ->
end
end.
+alter_group(<<"all">>, _Options) ->
+ {error, reserved_name};
alter_group(Groupname, Options) ->
case group_details(Groupname) of
undefined ->
@@ -437,6 +493,8 @@ alter_group(Groupname, Options) ->
end
end.
+del_user(<<"all">>) ->
+ {error, reserved_name};
del_user(Username) ->
case user_exists(Username) of
false ->
@@ -459,6 +517,8 @@ del_user(Username) ->
ok
end.
+del_group(<<"all">>) ->
+ {error, reserved_name};
del_group(Groupname) ->
case group_exists(Groupname) of
false ->
@@ -481,11 +541,12 @@ del_group(Groupname) ->
ok
end.
+-spec add_grant(userlist(), bucket() | any, [string()]) -> ok | {error, term()}.
add_grant(all, Bucket, Grants) ->
%% all is always valid
case validate_permissions(Grants) of
ok ->
- add_grant_int([all], Bucket, Grants);
+ add_grant_int([{all, group}], Bucket, Grants);
Error ->
Error
end;
@@ -523,16 +584,12 @@ add_grant(Role, Bucket, Grants) ->
add_grant([Role], Bucket, Grants).
+-spec add_revoke(userlist(), bucket() | any, [string()]) -> ok | {error, term()}.
add_revoke(all, Bucket, Revokes) ->
%% all is always valid
case validate_permissions(Revokes) of
ok ->
- case add_revoke_int([all], Bucket, revokes) of
- ok ->
- ok;
- Error2 ->
- Error2
- end;
+ add_revoke_int([{all, group}], Bucket, Revokes);
Error ->
Error
end;
@@ -570,6 +627,9 @@ add_revoke(User, Bucket, Revokes) ->
add_revoke([User], Bucket, Revokes).
+-spec add_source(Users :: all | binary() | [binary()], 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
@@ -616,7 +676,7 @@ del_source(all, CIDR) ->
ok;
del_source([H|_T]=UserList, CIDR) when is_binary(H) ->
_ = [riak_core_metadata:delete({<<"security">>, <<"sources">>},
- {User, anchor_mask(CIDR)}) || User <- UserList],
+ {User, anchor_mask(CIDR)}) || User <- UserList],
ok;
del_source(User, CIDR) ->
%% single user
@@ -777,7 +837,7 @@ match_grant(Bucket, Grants) ->
maybe_refresh_context(Context) ->
%% TODO replace this with a cluster metadata hash check, or something
Epoch = os:timestamp(),
- case timer:now_diff(Epoch, Context#context.epoch) < 1000 of
+ case timer:now_diff(Epoch, Context#context.epoch) < ?REFRESH_TIME of
false ->
%% context has expired
get_context(Context#context.username);
@@ -799,7 +859,15 @@ get_context(Username) when is_binary(Username) ->
#context{username=Username, grants=Grants, epoch=os:timestamp()}.
accumulate_grants(Role, Type) ->
- {Grants, _Seen} = accumulate_grants([Role], [], [], Type),
+ %% The 'all' grants always apply
+ All = riak_core_metadata:fold(fun({{_R, _Bucket}, [?TOMBSTONE]}, A) ->
+ A;
+ ({{_R, Bucket}, [Permissions]}, A) ->
+ [{{<<"group/all">>, Bucket},
+ Permissions}|A]
+ end, [], metadata_grant_prefix(group),
+ [{match, {all, '_'}}]),
+ {Grants, _Seen} = accumulate_grants([Role], [], All, Type),
lists:flatten(Grants).
accumulate_grants([], Seen, Acc, _Type) ->
@@ -876,12 +944,8 @@ validate_options(Options) ->
undefined ->
validate_groups_option(Options);
Pass ->
- case validate_password_option(Pass, Options) of
- {ok, NewOptions} ->
- validate_groups_option(NewOptions);
- Error ->
- Error
- end
+ {ok, NewOptions} = validate_password_option(Pass, Options),
+ validate_groups_option(NewOptions)
end.
validate_groups_option(Options) ->
@@ -889,8 +953,9 @@ validate_groups_option(Options) ->
undefined ->
{ok, Options};
GroupStr ->
- Groups= [list_to_binary(G) || G <-
- string:tokens(GroupStr, ",")],
+ %% Don't let the admin assign "all" as a container
+ Groups= [list_to_binary(G) ||
+ G <- string:tokens(GroupStr, ","), G /= "all"],
case unknown_roles(Groups, group) of
[] ->
@@ -903,20 +968,16 @@ validate_groups_option(Options) ->
%% Handle 'password' option if given
validate_password_option(Pass, Options) ->
- case riak_core_pw_auth:hash_password(list_to_binary(Pass)) of
- {ok, HashedPass, AuthName, HashFunction, Salt, Iterations} ->
- %% Add to options, replacing plaintext password
- NewOptions = stash("password", {"password",
- [{hash_pass, HashedPass},
- {auth_name, AuthName},
- {hash_func, HashFunction},
- {salt, Salt},
- {iterations, Iterations}]},
- Options),
- {ok, NewOptions};
- {error, Error} ->
- {error, Error}
- end.
+ {ok, HashedPass, AuthName, HashFunction, Salt, Iterations} =
+ riak_core_pw_auth:hash_password(list_to_binary(Pass)),
+ NewOptions = stash("password", {"password",
+ [{hash_pass, HashedPass},
+ {auth_name, AuthName},
+ {hash_func, HashFunction},
+ {salt, Salt},
+ {iterations, Iterations}]},
+ Options),
+ {ok, NewOptions}.
validate_permissions(Perms) ->
@@ -938,7 +999,7 @@ validate_permissions([Perm|T], Known) ->
end
catch
error:badarg ->
- {error, {unknown_permission, Perm}}
+ {error, {unknown_permission, Perm}}
end;
_ ->
{error, {unknown_permission, Perm}}
View
225 test/riak_core_security_tests.erl
@@ -0,0 +1,225 @@
+-module(riak_core_security_tests).
+-compile(export_all).
+
+-ifdef(TEST).
+-include_lib("eunit/include/eunit.hrl").
+
+start_manager(Node) ->
+ Dir = atom_to_list(Node),
+ case length(Dir) of
+ 0 -> exit(no_dir); %% make sure we don't do something stupid below
+ _ -> ok
+ end,
+ os:cmd("mkdir " ++ Dir),
+ os:cmd("rm -rf " ++ Dir ++ "/"),
+ {ok, Mgr} = riak_core_metadata_manager:start_link([{data_dir, Dir},
+ {node_name, Node}]),
+ TreeDir = Dir ++ "/trees",
+ {ok, Tree} = riak_core_metadata_hashtree:start_link(TreeDir),
+ {ok, Bcst} = riak_core_broadcast:start_link([node()], [], [], []),
+ unlink(Bcst),
+ unlink(Tree),
+ unlink(Mgr),
+
+ application:set_env(riak_core, permissions, [{riak_kv,[get,put]}]),
+ {Mgr, Tree, Bcst}.
+
+stop_manager({Mgr, Tree, Bcst}) ->
+ catch exit(Mgr, kill),
+ catch exit(Tree, kill),
+ catch exit(Bcst, kill),
+ ok.
+
+security_test_() ->
+ {foreach,
+ fun() ->
+ start_manager(node())
+ end,
+ fun(S) ->
+ stop_manager(S)
+ end,
+ [ {"trust auth works",
+ fun() ->
+ ?assertMatch({error, _}, riak_core_security:authenticate(<<"user">>, <<"password">>,
+ [{ip, {127, 0, 0, 1}}])),
+ ?assertEqual(ok, riak_core_security:add_user(<<"user">>,
+ [{"password","password"}])),
+ ?assertMatch({error, _}, riak_core_security:authenticate(<<"user">>, <<"password">>,
+ [{ip, {127, 0, 0, 1}}])),
+ ?assertEqual(ok, riak_core_security:add_source(all, {{127, 0, 0, 1}, 32}, trust, [])),
+ ?assertMatch({ok, _}, riak_core_security:authenticate(<<"user">>, <<"password">>,
+ [{ip, {127, 0, 0, 1}}])),
+ %% make sure these don't crash, at least
+ ?assertEqual(ok, riak_core_security:print_users()),
+ ?assertEqual(ok, riak_core_security:print_sources()),
+ ?assertEqual(ok, riak_core_security:print_user(<<"user">>)),
+ ok
+ end},
+ { "password auth works",
+ fun() ->
+ ?assertMatch({error, _}, riak_core_security:authenticate(<<"user">>, <<"password">>,
+ [{ip, {127, 0, 0, 1}}])),
+ ?assertEqual(ok, riak_core_security:add_user(<<"user">>,
+ [])),
+ ?assertMatch({error, _}, riak_core_security:authenticate(<<"user">>, <<"password">>,
+ [{ip, {127, 0, 0, 1}}])),
+ ?assertEqual(ok, riak_core_security:add_source(all, {{127, 0, 0, 1}, 32}, password, [])),
+ ?assertEqual({error, missing_password}, riak_core_security:authenticate(<<"user">>, <<"password">>,
+ [{ip, {127, 0, 0, 1}}])),
+ ?assertEqual(ok, riak_core_security:alter_user(<<"user">>, [{"password", "password"}])),
+ ?assertMatch({error, _}, riak_core_security:authenticate(<<"user">>, <<"badpassword">>,
+ [{ip, {127, 0, 0, 1}}])),
+ ?assertMatch({ok, _}, riak_core_security:authenticate(<<"user">>, <<"password">>,
+ [{ip, {127, 0, 0, 1}}])),
+ %% make sure these don't crash, at least
+ ?assertEqual(ok, riak_core_security:print_users()),
+ ?assertEqual(ok, riak_core_security:print_sources()),
+ ?assertEqual(ok, riak_core_security:print_user(<<"user">>)),
+ ok
+ end},
+ { "user grant/revoke on type/bucket works",
+ fun() ->
+ ?assertEqual(ok, riak_core_security:add_user(<<"user">>,
+ [{"password","password"}])),
+ ?assertEqual(ok, riak_core_security:add_source(all, {{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)),
+ ?assertEqual(ok, riak_core_security:add_grant(<<"user">>, {<<"default">>, <<"mybucket">>}, ["riak_kv.get", "riak_kv.put"])),
+ ?assertMatch({error, {unknown_permission, _}}, riak_core_security:add_grant(<<"user">>, {<<"default">>, <<"mybucket">>}, ["riak_kv.upsert"])),
+ ?assertMatch({true, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ ?assertEqual(ok, riak_core_security:add_revoke(<<"user">>, {<<"default">>, <<"mybucket">>}, ["riak_kv.get"])),
+ ?assertMatch({error, {unknown_permission, _}}, riak_core_security:add_revoke(<<"user">>, {<<"default">>, <<"mybucket">>}, ["riak_kv.upsert"])),
+ ?assertMatch({false, _, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ ?assertMatch({true, _}, riak_core_security:check_permissions({"riak_kv.put", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ %% make sure these don't crash, at least
+ ?assertEqual(ok, riak_core_security:print_users()),
+ ?assertEqual(ok, riak_core_security:print_sources()),
+ ?assertEqual(ok, riak_core_security:print_user(<<"user">>)),
+ %% delete the user
+ ?assertMatch(ok, riak_core_security:del_user(<<"user">>)),
+ %% re-add them
+ ?assertEqual(ok, riak_core_security:add_user(<<"user">>,
+ [{"password","password"}])),
+ %% make sure their old grants are gone
+ {ok, Ctx2} = riak_core_security:authenticate(<<"user">>, <<"password">>,
+ [{ip, {127, 0, 0, 1}}]),
+ ?assertMatch({false, _, _}, riak_core_security:check_permissions({"riak_kv.put", {<<"default">>, <<"mybucket">>}}, Ctx2)),
+ ok
+ end},
+ { "group grant/revoke on type/bucket works",
+ fun() ->
+ ?assertEqual(ok, riak_core_security:add_user(<<"user">>,
+ [{"password","password"}])),
+ ?assertEqual(ok, riak_core_security:add_group(<<"group">>,
+ [])),
+ ?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)),
+ ?assertEqual(ok, riak_core_security:add_grant(<<"group">>, {<<"default">>, <<"mybucket">>}, ["riak_kv.get"])),
+ ?assertMatch({error, {unknown_permission, _}}, riak_core_security:add_grant(<<"group">>, {<<"default">>, <<"mybucket">>}, ["riak_kv.upsert"])),
+ ?assertMatch({false, _, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ ?assertEqual(ok, riak_core_security:alter_user(<<"user">>, [{"groups", ["group"]}])),
+ ?assertMatch({true, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ ?assertEqual(ok, riak_core_security:add_revoke(<<"group">>, {<<"default">>, <<"mybucket">>}, ["riak_kv.get"])),
+ ?assertMatch({error, {unknown_permission, _}}, riak_core_security:add_revoke(<<"group">>, {<<"default">>, <<"mybucket">>}, ["riak_kv.upsert"])),
+ ?assertMatch({false, _, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ ?assertEqual(ok, riak_core_security:add_grant(<<"group">>, {<<"default">>, <<"mybucket">>}, ["riak_kv.get"])),
+ ?assertMatch({true, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ ?assertMatch({error, {unknown_groups, _}}, riak_core_security:alter_user(<<"user">>, [{"groups", ["nogroup"]}])),
+ ?assertEqual(ok, riak_core_security:alter_user(<<"user">>, [{"groups", []}])),
+ ?assertMatch({false, _, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ ?assertEqual(ok, riak_core_security:alter_user(<<"user">>, [{"groups", ["group"]}])),
+ %% make sure these don't crash, at least
+ ?assertEqual(ok, riak_core_security:print_users()),
+ ?assertEqual(ok, riak_core_security:print_sources()),
+ ?assertEqual(ok, riak_core_security:print_user(<<"user">>)),
+ ?assertEqual(ok, riak_core_security:print_groups()),
+ ?assertEqual(ok, riak_core_security:print_group(<<"group">>)),
+ ok
+ end},
+ { "all grant/revoke on type/bucket works",
+ 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, [])),
+ {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)),
+ ?assertEqual(ok, riak_core_security:add_grant(all, {<<"default">>, <<"mybucket">>}, ["riak_kv.get"])),
+ ?assertMatch({error, {unknown_permission, _}}, riak_core_security:add_grant(all, {<<"default">>, <<"mybucket">>}, ["riak_kv.upsert"])),
+ ?assertMatch({true, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ ?assertEqual(ok, riak_core_security:add_revoke(all, {<<"default">>, <<"mybucket">>}, ["riak_kv.get"])),
+ ?assertMatch({false, _, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ %% make sure these don't crash, at least
+ ?assertEqual(ok, riak_core_security:print_users()),
+ ?assertEqual(ok, riak_core_security:print_sources()),
+ ?assertEqual(ok, riak_core_security:print_user(<<"user">>)),
+ ?assertEqual(ok, riak_core_security:print_groups()),
+ ?assertEqual({error, {unknown_group, <<"all">>}}, riak_core_security:print_group(<<"all">>)),
+ ok
+ end},
+ { "groups can be members of groups and inherit permissions from them",
+ fun() ->
+ ?assertEqual(ok, riak_core_security:add_group(<<"sysadmin">>, [])),
+ ?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, [])),
+ %% 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
+ ?assertMatch({error, no_matching_sources}, riak_core_security:authenticate(<<"user">>, <<"password">>,
+ [{ip, {10, 0, 0, 1}}])),
+ {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)),
+ ?assertMatch({false, _, _}, riak_core_security:check_permissions({"riak_kv.put", {<<"default">>, <<"myotherbucket">>}}, Ctx)),
+ ?assertEqual(ok, riak_core_security:alter_user(<<"user">>, [{"groups", ["superuser"]}])),
+ ?assertMatch({true, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ ?assertMatch({true, _}, riak_core_security:check_permissions({"riak_kv.put", {<<"default">>, <<"myotherbucket">>}}, Ctx)),
+ %% make sure these don't crash, at least
+ ?assertEqual(ok, riak_core_security:print_users()),
+ ?assertEqual(ok, riak_core_security:print_sources()),
+ ?assertEqual(ok, riak_core_security:print_user(<<"user">>)),
+ ?assertEqual(ok, riak_core_security:print_groups()),
+ ?assertEqual(ok, riak_core_security:print_group(<<"superuser">>)),
+ ?assertEqual(ok, riak_core_security:print_group(<<"sysadmin">>)),
+ ?assertEqual(ok, riak_core_security:alter_group(<<"superuser">>, [{"groups", []}])),
+ ?assertMatch({false, _, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ ?assertEqual(ok, riak_core_security:alter_group(<<"superuser">>, [{"groups", ["sysadmin"]}])),
+ ?assertMatch({true, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ ?assertMatch(ok, riak_core_security:del_group(<<"sysadmin">>)),
+ ?assertMatch({false, _, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ %% check re-adding the group does not resurrect old permissions or memberships
+ ?assertEqual(ok, riak_core_security:add_group(<<"sysadmin">>, [])),
+ ?assertMatch({false, _, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ ?assertEqual(ok, riak_core_security:add_grant(<<"sysadmin">>, <<"default">>, ["riak_kv.get", "riak_kv.put"])),
+ ?assertMatch({false, _, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ %% re-adding the group membership does restore the permissions, though
+ ?assertEqual(ok, riak_core_security:alter_group(<<"superuser">>, [{"groups", ["sysadmin"]}])),
+ ?assertMatch({true, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ ok
+ end},
+ { "user/group disambiguation",
+ fun() ->
+ ?assertEqual(ok, riak_core_security:add_group(<<"sysadmin">>, [])),
+ ?assertEqual(ok, riak_core_security:add_user(<<"sysadmin">>, [{"password", "password"}])),
+ ?assertEqual(ok, riak_core_security:add_source(all, {{127, 0, 0, 1}, 32}, password, [])),
+ ?assertEqual({error, {duplicate_roles, [<<"sysadmin">>]}}, riak_core_security:add_grant(<<"sysadmin">>, <<"default">>, ["riak_kv.get", "riak_kv.put"])),
+ ?assertEqual(ok, riak_core_security:add_grant(<<"user/sysadmin">>, <<"default">>, ["riak_kv.get", "riak_kv.put"])),
+ ?assertEqual(ok, riak_core_security:add_grant(<<"group/sysadmin">>, any, ["riak_kv.get", "riak_kv.put"])),
+ {ok, Ctx} = riak_core_security:authenticate(<<"sysadmin">>, <<"password">>,
+ [{ip, {127, 0, 0, 1}}]),
+ ?assertMatch({true, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"default">>, <<"mybucket">>}}, Ctx)),
+ ?assertMatch({false, _, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"custom">>, <<"mybucket">>}}, Ctx)),
+ ?assertEqual(ok, riak_core_security:alter_user(<<"sysadmin">>, [{"groups", ["sysadmin"]}])),
+ ?assertMatch({true, _}, riak_core_security:check_permissions({"riak_kv.get", {<<"custom">>, <<"mybucket">>}}, Ctx)),
+ ?assertMatch({true, _}, riak_core_security:check_permissions({"riak_kv.get"}, Ctx)),
+ ok
+ end}
+ ]}.
+
+-endif.
+
Something went wrong with that request. Please try again.