Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

possible error formatting error message in security module [JIRA: RIAK-1802] #706

Closed
marianoguerra opened this issue Feb 18, 2015 · 8 comments
Assignees

Comments

@marianoguerra
Copy link

hi, I think this is an error, if I get a confirmation I would be more than happy to provide a pull request.

the issue starts when a call like this:

7> riak_core_security:check_permissions({"get", any}, Ctx).     
** exception error: bad argument
     in function  unicode:characters_to_binary/3
    called as unicode:characters_to_binary(any,utf8,utf8)
     in call from riak_core_security:check_permission/2 (src/riak_core_security.erl, line 380)

8> riak_core_security:check_permissions({"get", <<"foo">>}, Ctx).
{false,<<"Permission denied: User 'admin' does not have 'get' on foo">>,
       {context,<<"admin">>,
            [...],
            {1424,292313,620773}}}

bucket2iolist seems to be only called from the line 380 here:

https://github.com/basho/riak_core/blob/develop/src/riak_core_security.erl#L380

and it fails because instead of translating the atom any to something like "*" or "any" it calls unicode:characters_to_binary(Bucket, utf8, utf8) which fails.

should the fix be matching any and returning a string representation of it?

@marianoguerra
Copy link
Author

proposed fix:

note, it doesn't replace Bucket being any in case the parameter is {Type, Bucket}, I haven't seen that case, but adding a case for that is easy

@@ -1325,6 +1325,8 @@ bucket2bin({Type, Bucket}) ->
 bucket2bin(Name) ->
     unicode:characters_to_binary(Name, utf8, utf8).

+bucket2iolist(any) ->
+    <<"*">>;
 bucket2iolist({Type, Bucket}) ->
     [unicode:characters_to_binary(Type, utf8, utf8), "/",
      unicode:characters_to_binary(Bucket, utf8, utf8)];

marianoguerra added a commit to marianoguerra/riak_core that referenced this issue Feb 19, 2015
riak_core_security_tests: security_test_ (Issue 706, error formatted correctly for unauthorized and bucket any)...*failed*
in function unicode:characters_to_binary/3
  called as characters_to_binary(any,utf8,utf8)
in call from riak_core_security:check_permission/2 (src/riak_core_security.erl, line 380)
in call from riak_core_security_tests:'-security_test_/0-fun-141-'/1 (test/riak_core_security_tests.erl, line 251)
in call from riak_core_security_tests:'-security_test_/0-fun-142-'/0 (test/riak_core_security_tests.erl, line 251)
**error:badarg
@marianoguerra
Copy link
Author

here is a failing test case and a proposed fix:

https://github.com/marianoguerra/riak_core/tree/fix-issue-706

comparison here:

https://github.com/marianoguerra/riak_core/compare/fix-issue-706

I can do a pull request if it fits your workflow

@jcapricebasho
Copy link
Contributor

comment for jira

@Basho-JIRA Basho-JIRA changed the title possible error formatting error message in security module possible error formatting error message in security module [JIRA: RIAK-1802] May 13, 2015
@lukebakken
Copy link

@marianoguerra - Did you run into this during normal Riak usage or only from a riak attach session?

@Basho-JIRA
Copy link

Please see this pull request:

basho/riak_kv#1116

_[posted via JIRA by Luke Bakken]_

@Basho-JIRA
Copy link

[~jmeredith], can you get someone to review this and get added to the next Riak release? The "Get Preflist" function will not work with Riak security enabled without this fix. Thanks.

_[posted via JIRA by Derek Somogyi]_

@Basho-JIRA Basho-JIRA assigned jonmeredith and unassigned lukebakken May 14, 2015
@Basho-JIRA
Copy link

This issue is currently holding up the PHP 2.1 release. It appears as though the change/fix will be included in the 2.1.2 release. I added that tag to the Labels field.

_[posted via JIRA by Derek Somogyi]_

@Basho-JIRA
Copy link

Merged into 2.1 on July 7.

_[posted via JIRA by Douglas Rohrer]_

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants