Skip to content

Commit

Permalink
New flow to abstract out recurring path_info -> check_permission sequ…
Browse files Browse the repository at this point in the history
…ence
  • Loading branch information
macintux committed Apr 15, 2014
1 parent 71d4295 commit 87eca59
Showing 1 changed file with 33 additions and 10 deletions.
43 changes: 33 additions & 10 deletions src/yz_wm_search.erl
Expand Up @@ -76,24 +76,47 @@ is_authorized(ReqData, Ctx) ->
"instead.">>, ReqData), Ctx}
end.


%%
%% Written with the idea of using this from multiple modules
check_permissions(RD, Ctx,
_Permission, {_Bucket, undefined}) ->
{true, wrq:append_to_resp_body("Unknown index", RD), Ctx};
check_permissions(RD, Ctx,
_Permission, {_Bucket, N}) when is_integer(N) ->
{true, wrq:append_to_resp_body("Unknown index", RD), Ctx};

This comment has been minimized.

Copy link
@coderoshi

coderoshi Apr 16, 2014

Contributor

Isn't an integer a valid index name? Couldn't you query like /search/13?q=*:*

This comment has been minimized.

Copy link
@macintux

macintux via email Apr 16, 2014

Author Contributor

This comment has been minimized.

Copy link
@coderoshi

coderoshi Apr 16, 2014

Contributor

Since they're both the same, how about merging these two?

check_permissions(RD, Ctx, _Permission, {_Bucket, N}) when is_integer(N); N =:= undefined ->
    {true, wrq:append_to_resp_body("Unknown index", RD), Ctx};

http://www.erlang.org/doc/reference_manual/expressions.html#id81357

This comment has been minimized.

Copy link
@macintux

macintux Apr 17, 2014

Author Contributor

I considered that but preferred the current approach, but I don't feel strongly about it either way. Will change.

check_permissions(RD, Ctx,
Permission, {Bucket, Index}) ->
check_permissions_aux(RD, Ctx, Permission,
{Bucket, mochiweb_util:unquote(Index)});
check_permissions(RD, Ctx, Permission, Bucket) ->
check_permissions_aux(RD, Ctx, Permission, Bucket).

This comment has been minimized.

Copy link
@coderoshi

coderoshi Apr 16, 2014

Contributor

This can't be reached

This comment has been minimized.

Copy link
@macintux

macintux Apr 17, 2014

Author Contributor

It can't yet, but I'm planning on calling check_permissions from other forbidden functions in YZ where there is no index name.


check_permissions_aux(RD, Ctx=#ctx{security=Security},
Permission, Resource) ->
Res = riak_core_security:check_permission(
{Permission, Resource}, Security
),
case Res of
{false, Error, _} ->
{true, wrq:append_to_resp_body(Error, RD), Ctx};
{true, _} ->
{false, RD, Ctx}
end.



%% Uses the riak_kv,secure_referer_check setting rather
%% as opposed to a special yokozuna-specific config
forbidden(RD, Ctx=#ctx{security=undefined}) ->
{riak_kv_wm_utils:is_forbidden(RD), RD, Ctx};
forbidden(RD, Ctx=#ctx{security=Security}) ->
forbidden(RD, Ctx) ->
case riak_kv_wm_utils:is_forbidden(RD) of
true ->
{true, RD, Ctx};
false ->
Index = wrq:path_info(index, RD),
PermAndResource = {?YZ_SECURITY_SEARCH_PERM, {?YZ_SECURITY_INDEX, Index}},
Res = riak_core_security:check_permission(PermAndResource, Security),
case Res of
{false, Error, _} ->
{true, wrq:append_to_resp_body(Error, RD), Ctx};
{true, _} ->
{false, RD, Ctx}
end
check_permissions(RD, Ctx, ?YZ_SECURITY_SEARCH_PERM,
{?YZ_SECURITY_INDEX, wrq:path_info(index, RD)})
end.

%% Treat POST as GET in order to work with existing Solr clients.
Expand Down

0 comments on commit 87eca59

Please sign in to comment.