You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by Chris Anderson <jc...@apache.org> on 2010/11/27 07:17:19 UTC

Re: svn commit: r1039619 - in /couchdb/trunk: share/www/dialog/ share/www/script/ share/www/script/test/ share/www/spec/ src/couchdb/

On Fri, Nov 26, 2010 at 8:41 PM,  <jc...@apache.org> wrote:
> Author: jchris
> Date: Sat Nov 27 04:41:20 2010
> New Revision: 1039619
>
> URL: http://svn.apache.org/viewvc?rev=1039619&view=rev
> Log:
> rename "readers" to "members" in _security object, keep backwards compatibility with old security objects"


I've updated the _security object API to use "members" instead of
"readers" because readers is misleading. Please review this commit.
The only tricky part is the backwards compatibility so that existing
databases and security objects are still functional. We can remove the
backwards compatibility code for 2.0 if we want.

Chris

>
> Modified:
>    couchdb/trunk/share/www/dialog/_database_security.html
>    couchdb/trunk/share/www/script/futon.browse.js
>    couchdb/trunk/share/www/script/test/reader_acl.js
>    couchdb/trunk/share/www/script/test/replication.js
>    couchdb/trunk/share/www/spec/couch_js_instance_methods_3_spec.js
>    couchdb/trunk/src/couchdb/couch_db.erl
>
> Modified: couchdb/trunk/share/www/dialog/_database_security.html
> URL: http://svn.apache.org/viewvc/couchdb/trunk/share/www/dialog/_database_security.html?rev=1039619&r1=1039618&r2=1039619&view=diff
> ==============================================================================
> --- couchdb/trunk/share/www/dialog/_database_security.html (original)
> +++ couchdb/trunk/share/www/dialog/_database_security.html Sat Nov 27 04:41:20 2010
> @@ -16,12 +16,12 @@ specific language governing permissions
>   <h2>Security</h2>
>   <fieldset>
>     <p class="help">
> -      Each database contains lists of admins and readers.
> -      Admins and readers are each defined by <tt>names</tt> and <tt>roles</tt>, which are lists of strings.
> +      Each database contains lists of admins and members.
> +      Admins and members are each defined by <tt>names</tt> and <tt>roles</tt>, which are lists of strings.
>     </p>
>
>     <h3>Admins</h3>
> -    <p class="help">Database admins can update design documents and edit the readers list.</p>
> +    <p class="help">Database admins can update design documents and edit the admin and member lists.</p>
>     <table summary=""><tbody><tr>
>       <th><label>Names:</label></th>
>       <td><input type="text" name="admin_names" size="40"></td>
> @@ -31,14 +31,14 @@ specific language governing permissions
>     </tr>
>     </tbody></table>
>
> -    <h3>Readers</h3>
> -    <p class="help">Database readers can access the database. If no readers are defined, the database is public.</p>
> +    <h3>Members</h3>
> +    <p class="help">Database members can access the database. If no members are defined, the database is public.</p>
>     <table summary=""><tbody><tr>
>       <th><label>Names:</label></th>
> -      <td><input type="text" name="reader_names" size="40"></td>
> +      <td><input type="text" name="member_names" size="40"></td>
>     </tr><tr>
>       <th><label>Roles:</label></th>
> -      <td><input type="text" name="reader_roles" size="40"></td>
> +      <td><input type="text" name="member_roles" size="40"></td>
>     </tr>
>     </tbody></table>
>
>
> Modified: couchdb/trunk/share/www/script/futon.browse.js
> URL: http://svn.apache.org/viewvc/couchdb/trunk/share/www/script/futon.browse.js?rev=1039619&r1=1039618&r2=1039619&view=diff
> ==============================================================================
> --- couchdb/trunk/share/www/script/futon.browse.js [utf-8] (original)
> +++ couchdb/trunk/share/www/script/futon.browse.js [utf-8] Sat Nov 27 04:41:20 2010
> @@ -189,26 +189,34 @@
>       }
>
>       this.databaseSecurity = function() {
> +        function namesAndRoles(r, key) {
> +          var names = [];
> +          var roles = [];
> +          if (r && typeof r[key + "s"] === "object") {
> +            if ($.isArray(r[key + "s"]["names"])) {
> +              names = r[key + "s"]["names"];
> +            }
> +            if ($.isArray(r[key + "s"]["roles"])) {
> +              roles = r[key + "s"]["roles"];
> +            }
> +          }
> +          return {names : names, roles: roles};
> +        };
> +
>         $.showDialog("dialog/_database_security.html", {
>           load : function(d) {
>             db.getDbProperty("_security", {
>               success: function(r) {
> -                ["admin", "reader"].forEach(function(key) {
> -                  var names = [];
> -                  var roles = [];
> -
> -                  if (r && typeof r[key + "s"] === "object") {
> -                    if ($.isArray(r[key + "s"]["names"])) {
> -                      names = r[key + "s"]["names"];
> -                    }
> -                    if ($.isArray(r[key + "s"]["roles"])) {
> -                      roles = r[key + "s"]["roles"];
> -                    }
> -                  }
> -
> -                  $("input[name=" + key + "_names]", d).val(JSON.stringify(names));
> -                  $("input[name=" + key + "_roles]", d).val(JSON.stringify(roles));
> -                });
> +                var admins = namesAndRoles(r, "admin")
> +                  , members = namesAndRoles(r, "member");
> +                if (members.names.length + members.roles.length == 0) {
> +                  // backwards compatibility with readers for 1.x
> +                  members = namesAndRoles(r, "reader");
> +                }
> +                $("input[name=admin_names]", d).val(JSON.stringify(admins.names));
> +                $("input[name=admin_roles]", d).val(JSON.stringify(admins.roles));
> +                $("input[name=member_names]", d).val(JSON.stringify(members.names));
> +                $("input[name=member_roles]", d).val(JSON.stringify(members.roles));
>               }
>             });
>           },
> @@ -220,13 +228,13 @@
>                 names: [],
>                 roles: []
>               },
> -              readers: {
> +              members: {
>                 names: [],
>                 roles: []
>               }
>             };
>
> -            ["admin", "reader"].forEach(function(key) {
> +            ["admin", "member"].forEach(function(key) {
>               var names, roles;
>
>               try {
>
> Modified: couchdb/trunk/share/www/script/test/reader_acl.js
> URL: http://svn.apache.org/viewvc/couchdb/trunk/share/www/script/test/reader_acl.js?rev=1039619&r1=1039618&r2=1039619&view=diff
> ==============================================================================
> --- couchdb/trunk/share/www/script/test/reader_acl.js (original)
> +++ couchdb/trunk/share/www/script/test/reader_acl.js Sat Nov 27 04:41:20 2010
> @@ -37,7 +37,7 @@ couchTests.reader_acl = function(debug)
>       T(secretDb.open("baz").foo == "bar");
>
>       T(secretDb.setSecObj({
> -        "readers" : {
> +        "members" : {
>           roles : ["super-secret-club"],
>           names : ["joe","barb"]
>         }
> @@ -64,13 +64,13 @@ couchTests.reader_acl = function(debug)
>       CouchDB.logout();
>
>       // make anyone with the top-secret role an admin
> -      // db admins are automatically readers
> +      // db admins are automatically members
>       T(secretDb.setSecObj({
>         "admins" : {
>           roles : ["top-secret"],
>           names : []
>         },
> -        "readers" : {
> +        "members" : {
>           roles : ["super-secret-club"],
>           names : ["joe","barb"]
>         }
> @@ -90,14 +90,14 @@ couchTests.reader_acl = function(debug)
>       CouchDB.logout();
>       T(CouchDB.session().userCtx.roles.indexOf("_admin") != -1);
>
> -      // admin now adds the top-secret role to the db's readers
> +      // admin now adds the top-secret role to the db's members
>       // and removes db-admins
>       T(secretDb.setSecObj({
>         "admins" : {
>           roles : [],
>           names : []
>         },
> -        "readers" : {
> +        "members" : {
>           roles : ["super-secret-club", "top-secret"],
>           names : ["joe","barb"]
>         }
> @@ -124,10 +124,10 @@ couchTests.reader_acl = function(debug)
>       T(CouchDB.login("jchris@apache.org", "funnybone").ok);
>       T(CouchDB.session().userCtx.roles.indexOf("_admin") == -1);
>       T(secretDb.open("baz").foo == "bar");
> -      // readers can query stored views
> +      // members can query stored views
>       T(secretDb.view("foo/bar").total_rows == 1);
>
> -      // readers can't do temp views
> +      // members can't do temp views
>       try {
>         var results = secretDb.query(function(doc) {
>           emit(null, null);
> @@ -137,13 +137,28 @@ couchTests.reader_acl = function(debug)
>         T(true && "temp view is admin only");
>       }
>
> -
>       CouchDB.logout();
>
> +      // works with readers (backwards compat with 1.0)
> +      T(secretDb.setSecObj({
> +        "admins" : {
> +          roles : [],
> +          names : []
> +        },
> +        "readers" : {
> +          roles : ["super-secret-club", "top-secret"],
> +          names : ["joe","barb"]
> +        }
> +      }).ok);
> +
> +      T(CouchDB.login("jchris@apache.org", "funnybone").ok);
> +      T(CouchDB.session().userCtx.roles.indexOf("_admin") == -1);
> +      T(secretDb.open("baz").foo == "bar");
> +
>       // can't set non string reader names or roles
>       try {
>         secretDb.setSecObj({
> -          "readers" : {
> +          "members" : {
>             roles : ["super-secret-club", {"top-secret":"awesome"}],
>             names : ["joe","barb"]
>           }
> @@ -153,7 +168,7 @@ couchTests.reader_acl = function(debug)
>
>       try {
>         secretDb.setSecObj({
> -          "readers" : {
> +          "members" : {
>             roles : ["super-secret-club", {"top-secret":"awesome"}],
>             names : ["joe",22]
>           }
> @@ -163,7 +178,7 @@ couchTests.reader_acl = function(debug)
>
>       try {
>         secretDb.setSecObj({
> -          "readers" : {
> +          "members" : {
>             roles : ["super-secret-club", {"top-secret":"awesome"}],
>             names : "joe"
>           }
>
> Modified: couchdb/trunk/share/www/script/test/replication.js
> URL: http://svn.apache.org/viewvc/couchdb/trunk/share/www/script/test/replication.js?rev=1039619&r1=1039618&r2=1039619&view=diff
> ==============================================================================
> --- couchdb/trunk/share/www/script/test/replication.js (original)
> +++ couchdb/trunk/share/www/script/test/replication.js Sat Nov 27 04:41:20 2010
> @@ -623,7 +623,7 @@ couchTests.replication = function(debug)
>       names: [],
>       roles: ["admin"]
>     },
> -    readers: {
> +    members: {
>       names: [],
>       roles: ["reader"]
>     }
> @@ -687,7 +687,7 @@ couchTests.replication = function(debug)
>         names: [],
>         roles: ["bar"]
>       },
> -      readers: {
> +      members: {
>         names: [],
>         roles: ["foo"]
>       }
>
> Modified: couchdb/trunk/share/www/spec/couch_js_instance_methods_3_spec.js
> URL: http://svn.apache.org/viewvc/couchdb/trunk/share/www/spec/couch_js_instance_methods_3_spec.js?rev=1039619&r1=1039618&r2=1039619&view=diff
> ==============================================================================
> --- couchdb/trunk/share/www/spec/couch_js_instance_methods_3_spec.js (original)
> +++ couchdb/trunk/share/www/spec/couch_js_instance_methods_3_spec.js Sat Nov 27 04:41:20 2010
> @@ -184,15 +184,15 @@ describe 'CouchDB instance'
>
>   describe '.setSecObj'
>     it 'should return ok true'
> -      db.setSecObj({"readers":{"names":["laura"],"roles":["president"]}}).ok.should.be_true
> +      db.setSecObj({"members":{"names":["laura"],"roles":["president"]}}).ok.should.be_true
>     end
>
>     it 'should save a well formed object into the _security object '
> -      db.should.receive("request", "once").with_args("PUT", "/spec_db/_security", {body: '{"readers":{"names":["laura"],"roles":["president"]}}'})
> -      db.setSecObj({"readers": {"names" : ["laura"], "roles" : ["president"]}})
> +      db.should.receive("request", "once").with_args("PUT", "/spec_db/_security", {body: '{"members":{"names":["laura"],"roles":["president"]}}'})
> +      db.setSecObj({"members": {"names" : ["laura"], "roles" : ["president"]}})
>     end
>
> -    it 'should throw an error when the readers or admins object is malformed'
> +    it 'should throw an error when the members or admins object is malformed'
>       -{ db.setSecObj({"admins":["cylon"]}) }.should.throw_error
>     end
>
>
> Modified: couchdb/trunk/src/couchdb/couch_db.erl
> URL: http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_db.erl?rev=1039619&r1=1039618&r2=1039619&view=diff
> ==============================================================================
> --- couchdb/trunk/src/couchdb/couch_db.erl (original)
> +++ couchdb/trunk/src/couchdb/couch_db.erl Sat Nov 27 04:41:20 2010
> @@ -26,7 +26,7 @@
>  -export([set_security/2,get_security/1]).
>  -export([init/1,terminate/2,handle_call/3,handle_cast/2,code_change/3,handle_info/2]).
>  -export([changes_since/5,changes_since/6,read_doc/2,new_revid/1]).
> --export([check_is_admin/1, check_is_reader/1]).
> +-export([check_is_admin/1, check_is_member/1]).
>  -export([reopen/1]).
>
>  -include("couch_db.hrl").
> @@ -77,7 +77,7 @@ open(DbName, Options) ->
>     case couch_server:open(DbName, Options) of
>         {ok, Db} ->
>             try
> -                check_is_reader(Db),
> +                check_is_member(Db),
>                 {ok, Db}
>             catch
>                 throw:Error ->
> @@ -297,14 +297,14 @@ check_is_admin(#db{user_ctx=#user_ctx{na
>         ok
>     end.
>
> -check_is_reader(#db{user_ctx=#user_ctx{name=Name,roles=Roles}=UserCtx}=Db) ->
> +check_is_member(#db{user_ctx=#user_ctx{name=Name,roles=Roles}=UserCtx}=Db) ->
>     case (catch check_is_admin(Db)) of
>     ok -> ok;
>     _ ->
> -        {Readers} = get_readers(Db),
> -        ReaderRoles = couch_util:get_value(<<"roles">>, Readers,[]),
> +        {Members} = get_members(Db),
> +        ReaderRoles = couch_util:get_value(<<"roles">>, Members,[]),
>         WithAdminRoles = [<<"_admin">> | ReaderRoles],
> -        ReaderNames = couch_util:get_value(<<"names">>, Readers,[]),
> +        ReaderNames = couch_util:get_value(<<"names">>, Members,[]),
>         case ReaderRoles ++ ReaderNames of
>         [] -> ok; % no readers == public access
>         _Else ->
> @@ -326,8 +326,10 @@ check_is_reader(#db{user_ctx=#user_ctx{n
>  get_admins(#db{security=SecProps}) ->
>     couch_util:get_value(<<"admins">>, SecProps, {[]}).
>
> -get_readers(#db{security=SecProps}) ->
> -    couch_util:get_value(<<"readers">>, SecProps, {[]}).
> +get_members(#db{security=SecProps}) ->
> +    % we fallback to readers here for backwards compatibility
> +    couch_util:get_value(<<"members">>, SecProps,
> +        couch_util:get_value(<<"readers">>, SecProps, {[]})).
>
>  get_security(#db{security=SecProps}) ->
>     {SecProps}.
> @@ -343,9 +345,11 @@ set_security(_, _) ->
>
>  validate_security_object(SecProps) ->
>     Admins = couch_util:get_value(<<"admins">>, SecProps, {[]}),
> -    Readers = couch_util:get_value(<<"readers">>, SecProps, {[]}),
> +    % we fallback to readers here for backwards compatibility
> +    Members = couch_util:get_value(<<"members">>, SecProps,
> +        couch_util:get_value(<<"readers">>, SecProps, {[]})),
>     ok = validate_names_and_roles(Admins),
> -    ok = validate_names_and_roles(Readers),
> +    ok = validate_names_and_roles(Members),
>     ok.
>
>  % validate user input
>
>
>



-- 
Chris Anderson
http://jchrisa.net
http://couch.io

Re: svn commit: r1039619 - in /couchdb/trunk: share/www/dialog/ share/www/script/ share/www/script/test/ share/www/spec/ src/couchdb/

Posted by Jan Lehnardt <ja...@apache.org>.
All yours :)

On 27 Nov 2010, at 15:53, Chris Anderson wrote:

> Jan,
> 
> I like all those suggestions. Care to make the changes or should I?
> 
> Chris
> 
> On Sat, Nov 27, 2010 at 6:29 AM, Jan Lehnardt <ja...@apache.org> wrote:
>> 
>> On 27 Nov 2010, at 15:27, Jan Lehnardt wrote:
>> 
>>> Hi Chris,
>>> 
>>> Good change, comments inline.
>>> 
>>> On 27 Nov 2010, at 07:17, Chris Anderson wrote:
>>> 
>>>> On Fri, Nov 26, 2010 at 8:41 PM,  <jc...@apache.org> wrote:
>>>>> Author: jchris
>>>>> Date: Sat Nov 27 04:41:20 2010
>>>>> New Revision: 1039619
>>>>> 
>>>>> URL: http://svn.apache.org/viewvc?rev=1039619&view=rev
>>>>> Log:
>>>>> rename "readers" to "members" in _security object, keep backwards compatibility with old security objects"
>>>> 
>>>> 
>>>> I've updated the _security object API to use "members" instead of
>>>> "readers" because readers is misleading. Please review this commit.
>>>> The only tricky part is the backwards compatibility so that existing
>>>> databases and security objects are still functional. We can remove the
>>>> backwards compatibility code for 2.0 if we want.
>>>> 
>>>> Chris
>>>>> […]
>>>>> Modified: couchdb/trunk/src/couchdb/couch_db.erl
>>>>> URL: http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_db.erl?rev=1039619&r1=1039618&r2=1039619&view=diff
>>>>> ==============================================================================
>>>>> --- couchdb/trunk/src/couchdb/couch_db.erl (original)
>>>>> +++ couchdb/trunk/src/couchdb/couch_db.erl Sat Nov 27 04:41:20 2010
>>>>> @@ -26,7 +26,7 @@
>>>>> -export([set_security/2,get_security/1]).
>>>>> -export([init/1,terminate/2,handle_call/3,handle_cast/2,code_change/3,handle_info/2]).
>>>>> -export([changes_since/5,changes_since/6,read_doc/2,new_revid/1]).
>>>>> --export([check_is_admin/1, check_is_reader/1]).
>>>>> +-export([check_is_admin/1, check_is_member/1]).
>>>>> -export([reopen/1]).
>>>>> 
>>>>> -include("couch_db.hrl").
>>>>> @@ -77,7 +77,7 @@ open(DbName, Options) ->
>>>>>    case couch_server:open(DbName, Options) of
>>>>>        {ok, Db} ->
>>>>>            try
>>>>> -                check_is_reader(Db),
>>>>> +                check_is_member(Db),
>>>>>                {ok, Db}
>>>>>            catch
>>>>>                throw:Error ->
>>>>> @@ -297,14 +297,14 @@ check_is_admin(#db{user_ctx=#user_ctx{na
>>>>>        ok
>>>>>    end.
>>>>> 
>>>>> -check_is_reader(#db{user_ctx=#user_ctx{name=Name,roles=Roles}=UserCtx}=Db) ->
>>>>> +check_is_member(#db{user_ctx=#user_ctx{name=Name,roles=Roles}=UserCtx}=Db) ->
>>>>>    case (catch check_is_admin(Db)) of
>>>>>    ok -> ok;
>>>>>    _ ->
>>>>> -        {Readers} = get_readers(Db),
>>>>> -        ReaderRoles = couch_util:get_value(<<"roles">>, Readers,[]),
>>>>> +        {Members} = get_members(Db),
>>>>> +        ReaderRoles = couch_util:get_value(<<"roles">>, Members,[]),
>>>>>        WithAdminRoles = [<<"_admin">> | ReaderRoles],
>>>>> -        ReaderNames = couch_util:get_value(<<"names">>, Readers,[]),
>>>>> +        ReaderNames = couch_util:get_value(<<"names">>, Members,[]),
>>>>>        case ReaderRoles ++ ReaderNames of
>>> 
>>> I'd s/ReaderRoles/MemberRoles for naming consistency.
>>> 
>>> 
>>>>>        [] -> ok; % no readers == public access
>>>>>        _Else ->
>>>>> @@ -326,8 +326,10 @@ check_is_reader(#db{user_ctx=#user_ctx{n
>>>>> get_admins(#db{security=SecProps}) ->
>>>>>    couch_util:get_value(<<"admins">>, SecProps, {[]}).
>>>>> 
>>>>> -get_readers(#db{security=SecProps}) ->
>>>>> -    couch_util:get_value(<<"readers">>, SecProps, {[]}).
>>>>> +get_members(#db{security=SecProps}) ->
>>>>> +    % we fallback to readers here for backwards compatibility
>>>>> +    couch_util:get_value(<<"members">>, SecProps,
>>>>> +        couch_util:get_value(<<"readers">>, SecProps, {[]})).
>>>>> 
>>>>> get_security(#db{security=SecProps}) ->
>>>>>    {SecProps}.
>>>>> @@ -343,9 +345,11 @@ set_security(_, _) ->
>>>>> 
>>>>> validate_security_object(SecProps) ->
>>>>>    Admins = couch_util:get_value(<<"admins">>, SecProps, {[]}),
>>>>> -    Readers = couch_util:get_value(<<"readers">>, SecProps, {[]}),
>>>>> +    % we fallback to readers here for backwards compatibility
>>>>> +    Members = couch_util:get_value(<<"members">>, SecProps,
>>>>> +        couch_util:get_value(<<"readers">>, SecProps, {[]})),
>>> 
>>> How about making this
>>> 
>>>  Members = get_members(SecProps);
>>> 
>>> by doing:
>>> 
>>> get_members(#db{security=SecProps}) ->
>>>    get_members_int(SecProps);
>>> get_members(SecProps) ->
>>>    get_members_int(SecProps),
>>    get_members_int(SecProps).
>> 
>> 
>>> 
>>> get_members_int(SecProps) ->
>>>    % we fallback to readers here for backwards compatibility
>>>    couch_util:get_value(<<"members">>, SecProps,
>>>        couch_util:get_value(<<"readers">>, SecProps, {[]})).
>>> 
>>> 
>>>>>    ok = validate_names_and_roles(Admins),
>>>>> -    ok = validate_names_and_roles(Readers),
>>>>> +    ok = validate_names_and_roles(Members),
>>>>>    ok.
>>>>> 
>>>>> % validate user input
>>>>> 
>>>>> 
>>>>> 
>>> 
>>> Cheers
>>> Jan
>>> --
>>> 
>> 
>> 
> 
> 
> 
> -- 
> Chris Anderson
> http://jchrisa.net
> http://couch.io


Re: svn commit: r1039619 - in /couchdb/trunk: share/www/dialog/ share/www/script/ share/www/script/test/ share/www/spec/ src/couchdb/

Posted by Chris Anderson <jc...@apache.org>.
Jan,

I like all those suggestions. Care to make the changes or should I?

Chris

On Sat, Nov 27, 2010 at 6:29 AM, Jan Lehnardt <ja...@apache.org> wrote:
>
> On 27 Nov 2010, at 15:27, Jan Lehnardt wrote:
>
>> Hi Chris,
>>
>> Good change, comments inline.
>>
>> On 27 Nov 2010, at 07:17, Chris Anderson wrote:
>>
>>> On Fri, Nov 26, 2010 at 8:41 PM,  <jc...@apache.org> wrote:
>>>> Author: jchris
>>>> Date: Sat Nov 27 04:41:20 2010
>>>> New Revision: 1039619
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1039619&view=rev
>>>> Log:
>>>> rename "readers" to "members" in _security object, keep backwards compatibility with old security objects"
>>>
>>>
>>> I've updated the _security object API to use "members" instead of
>>> "readers" because readers is misleading. Please review this commit.
>>> The only tricky part is the backwards compatibility so that existing
>>> databases and security objects are still functional. We can remove the
>>> backwards compatibility code for 2.0 if we want.
>>>
>>> Chris
>>>> […]
>>>> Modified: couchdb/trunk/src/couchdb/couch_db.erl
>>>> URL: http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_db.erl?rev=1039619&r1=1039618&r2=1039619&view=diff
>>>> ==============================================================================
>>>> --- couchdb/trunk/src/couchdb/couch_db.erl (original)
>>>> +++ couchdb/trunk/src/couchdb/couch_db.erl Sat Nov 27 04:41:20 2010
>>>> @@ -26,7 +26,7 @@
>>>> -export([set_security/2,get_security/1]).
>>>> -export([init/1,terminate/2,handle_call/3,handle_cast/2,code_change/3,handle_info/2]).
>>>> -export([changes_since/5,changes_since/6,read_doc/2,new_revid/1]).
>>>> --export([check_is_admin/1, check_is_reader/1]).
>>>> +-export([check_is_admin/1, check_is_member/1]).
>>>> -export([reopen/1]).
>>>>
>>>> -include("couch_db.hrl").
>>>> @@ -77,7 +77,7 @@ open(DbName, Options) ->
>>>>    case couch_server:open(DbName, Options) of
>>>>        {ok, Db} ->
>>>>            try
>>>> -                check_is_reader(Db),
>>>> +                check_is_member(Db),
>>>>                {ok, Db}
>>>>            catch
>>>>                throw:Error ->
>>>> @@ -297,14 +297,14 @@ check_is_admin(#db{user_ctx=#user_ctx{na
>>>>        ok
>>>>    end.
>>>>
>>>> -check_is_reader(#db{user_ctx=#user_ctx{name=Name,roles=Roles}=UserCtx}=Db) ->
>>>> +check_is_member(#db{user_ctx=#user_ctx{name=Name,roles=Roles}=UserCtx}=Db) ->
>>>>    case (catch check_is_admin(Db)) of
>>>>    ok -> ok;
>>>>    _ ->
>>>> -        {Readers} = get_readers(Db),
>>>> -        ReaderRoles = couch_util:get_value(<<"roles">>, Readers,[]),
>>>> +        {Members} = get_members(Db),
>>>> +        ReaderRoles = couch_util:get_value(<<"roles">>, Members,[]),
>>>>        WithAdminRoles = [<<"_admin">> | ReaderRoles],
>>>> -        ReaderNames = couch_util:get_value(<<"names">>, Readers,[]),
>>>> +        ReaderNames = couch_util:get_value(<<"names">>, Members,[]),
>>>>        case ReaderRoles ++ ReaderNames of
>>
>> I'd s/ReaderRoles/MemberRoles for naming consistency.
>>
>>
>>>>        [] -> ok; % no readers == public access
>>>>        _Else ->
>>>> @@ -326,8 +326,10 @@ check_is_reader(#db{user_ctx=#user_ctx{n
>>>> get_admins(#db{security=SecProps}) ->
>>>>    couch_util:get_value(<<"admins">>, SecProps, {[]}).
>>>>
>>>> -get_readers(#db{security=SecProps}) ->
>>>> -    couch_util:get_value(<<"readers">>, SecProps, {[]}).
>>>> +get_members(#db{security=SecProps}) ->
>>>> +    % we fallback to readers here for backwards compatibility
>>>> +    couch_util:get_value(<<"members">>, SecProps,
>>>> +        couch_util:get_value(<<"readers">>, SecProps, {[]})).
>>>>
>>>> get_security(#db{security=SecProps}) ->
>>>>    {SecProps}.
>>>> @@ -343,9 +345,11 @@ set_security(_, _) ->
>>>>
>>>> validate_security_object(SecProps) ->
>>>>    Admins = couch_util:get_value(<<"admins">>, SecProps, {[]}),
>>>> -    Readers = couch_util:get_value(<<"readers">>, SecProps, {[]}),
>>>> +    % we fallback to readers here for backwards compatibility
>>>> +    Members = couch_util:get_value(<<"members">>, SecProps,
>>>> +        couch_util:get_value(<<"readers">>, SecProps, {[]})),
>>
>> How about making this
>>
>>  Members = get_members(SecProps);
>>
>> by doing:
>>
>> get_members(#db{security=SecProps}) ->
>>    get_members_int(SecProps);
>> get_members(SecProps) ->
>>    get_members_int(SecProps),
>    get_members_int(SecProps).
>
>
>>
>> get_members_int(SecProps) ->
>>    % we fallback to readers here for backwards compatibility
>>    couch_util:get_value(<<"members">>, SecProps,
>>        couch_util:get_value(<<"readers">>, SecProps, {[]})).
>>
>>
>>>>    ok = validate_names_and_roles(Admins),
>>>> -    ok = validate_names_and_roles(Readers),
>>>> +    ok = validate_names_and_roles(Members),
>>>>    ok.
>>>>
>>>> % validate user input
>>>>
>>>>
>>>>
>>
>> Cheers
>> Jan
>> --
>>
>
>



-- 
Chris Anderson
http://jchrisa.net
http://couch.io

Re: svn commit: r1039619 - in /couchdb/trunk: share/www/dialog/ share/www/script/ share/www/script/test/ share/www/spec/ src/couchdb/

Posted by Jan Lehnardt <ja...@apache.org>.
On 27 Nov 2010, at 15:27, Jan Lehnardt wrote:

> Hi Chris,
> 
> Good change, comments inline.
> 
> On 27 Nov 2010, at 07:17, Chris Anderson wrote:
> 
>> On Fri, Nov 26, 2010 at 8:41 PM,  <jc...@apache.org> wrote:
>>> Author: jchris
>>> Date: Sat Nov 27 04:41:20 2010
>>> New Revision: 1039619
>>> 
>>> URL: http://svn.apache.org/viewvc?rev=1039619&view=rev
>>> Log:
>>> rename "readers" to "members" in _security object, keep backwards compatibility with old security objects"
>> 
>> 
>> I've updated the _security object API to use "members" instead of
>> "readers" because readers is misleading. Please review this commit.
>> The only tricky part is the backwards compatibility so that existing
>> databases and security objects are still functional. We can remove the
>> backwards compatibility code for 2.0 if we want.
>> 
>> Chris
>>> […]
>>> Modified: couchdb/trunk/src/couchdb/couch_db.erl
>>> URL: http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_db.erl?rev=1039619&r1=1039618&r2=1039619&view=diff
>>> ==============================================================================
>>> --- couchdb/trunk/src/couchdb/couch_db.erl (original)
>>> +++ couchdb/trunk/src/couchdb/couch_db.erl Sat Nov 27 04:41:20 2010
>>> @@ -26,7 +26,7 @@
>>> -export([set_security/2,get_security/1]).
>>> -export([init/1,terminate/2,handle_call/3,handle_cast/2,code_change/3,handle_info/2]).
>>> -export([changes_since/5,changes_since/6,read_doc/2,new_revid/1]).
>>> --export([check_is_admin/1, check_is_reader/1]).
>>> +-export([check_is_admin/1, check_is_member/1]).
>>> -export([reopen/1]).
>>> 
>>> -include("couch_db.hrl").
>>> @@ -77,7 +77,7 @@ open(DbName, Options) ->
>>>    case couch_server:open(DbName, Options) of
>>>        {ok, Db} ->
>>>            try
>>> -                check_is_reader(Db),
>>> +                check_is_member(Db),
>>>                {ok, Db}
>>>            catch
>>>                throw:Error ->
>>> @@ -297,14 +297,14 @@ check_is_admin(#db{user_ctx=#user_ctx{na
>>>        ok
>>>    end.
>>> 
>>> -check_is_reader(#db{user_ctx=#user_ctx{name=Name,roles=Roles}=UserCtx}=Db) ->
>>> +check_is_member(#db{user_ctx=#user_ctx{name=Name,roles=Roles}=UserCtx}=Db) ->
>>>    case (catch check_is_admin(Db)) of
>>>    ok -> ok;
>>>    _ ->
>>> -        {Readers} = get_readers(Db),
>>> -        ReaderRoles = couch_util:get_value(<<"roles">>, Readers,[]),
>>> +        {Members} = get_members(Db),
>>> +        ReaderRoles = couch_util:get_value(<<"roles">>, Members,[]),
>>>        WithAdminRoles = [<<"_admin">> | ReaderRoles],
>>> -        ReaderNames = couch_util:get_value(<<"names">>, Readers,[]),
>>> +        ReaderNames = couch_util:get_value(<<"names">>, Members,[]),
>>>        case ReaderRoles ++ ReaderNames of
> 
> I'd s/ReaderRoles/MemberRoles for naming consistency.
> 
> 
>>>        [] -> ok; % no readers == public access
>>>        _Else ->
>>> @@ -326,8 +326,10 @@ check_is_reader(#db{user_ctx=#user_ctx{n
>>> get_admins(#db{security=SecProps}) ->
>>>    couch_util:get_value(<<"admins">>, SecProps, {[]}).
>>> 
>>> -get_readers(#db{security=SecProps}) ->
>>> -    couch_util:get_value(<<"readers">>, SecProps, {[]}).
>>> +get_members(#db{security=SecProps}) ->
>>> +    % we fallback to readers here for backwards compatibility
>>> +    couch_util:get_value(<<"members">>, SecProps,
>>> +        couch_util:get_value(<<"readers">>, SecProps, {[]})).
>>> 
>>> get_security(#db{security=SecProps}) ->
>>>    {SecProps}.
>>> @@ -343,9 +345,11 @@ set_security(_, _) ->
>>> 
>>> validate_security_object(SecProps) ->
>>>    Admins = couch_util:get_value(<<"admins">>, SecProps, {[]}),
>>> -    Readers = couch_util:get_value(<<"readers">>, SecProps, {[]}),
>>> +    % we fallback to readers here for backwards compatibility
>>> +    Members = couch_util:get_value(<<"members">>, SecProps,
>>> +        couch_util:get_value(<<"readers">>, SecProps, {[]})),
> 
> How about making this
> 
>  Members = get_members(SecProps);
> 
> by doing:
> 
> get_members(#db{security=SecProps}) ->
>    get_members_int(SecProps);
> get_members(SecProps) ->
>    get_members_int(SecProps),
    get_members_int(SecProps).


> 
> get_members_int(SecProps) ->
>    % we fallback to readers here for backwards compatibility
>    couch_util:get_value(<<"members">>, SecProps,
>        couch_util:get_value(<<"readers">>, SecProps, {[]})).
> 
> 
>>>    ok = validate_names_and_roles(Admins),
>>> -    ok = validate_names_and_roles(Readers),
>>> +    ok = validate_names_and_roles(Members),
>>>    ok.
>>> 
>>> % validate user input
>>> 
>>> 
>>> 
> 
> Cheers
> Jan
> -- 
> 


Re: svn commit: r1039619 - in /couchdb/trunk: share/www/dialog/ share/www/script/ share/www/script/test/ share/www/spec/ src/couchdb/

Posted by Jan Lehnardt <ja...@apache.org>.
Hi Chris,

Good change, comments inline.

On 27 Nov 2010, at 07:17, Chris Anderson wrote:

> On Fri, Nov 26, 2010 at 8:41 PM,  <jc...@apache.org> wrote:
>> Author: jchris
>> Date: Sat Nov 27 04:41:20 2010
>> New Revision: 1039619
>> 
>> URL: http://svn.apache.org/viewvc?rev=1039619&view=rev
>> Log:
>> rename "readers" to "members" in _security object, keep backwards compatibility with old security objects"
> 
> 
> I've updated the _security object API to use "members" instead of
> "readers" because readers is misleading. Please review this commit.
> The only tricky part is the backwards compatibility so that existing
> databases and security objects are still functional. We can remove the
> backwards compatibility code for 2.0 if we want.
> 
> Chris
>> […]
>> Modified: couchdb/trunk/src/couchdb/couch_db.erl
>> URL: http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_db.erl?rev=1039619&r1=1039618&r2=1039619&view=diff
>> ==============================================================================
>> --- couchdb/trunk/src/couchdb/couch_db.erl (original)
>> +++ couchdb/trunk/src/couchdb/couch_db.erl Sat Nov 27 04:41:20 2010
>> @@ -26,7 +26,7 @@
>>  -export([set_security/2,get_security/1]).
>>  -export([init/1,terminate/2,handle_call/3,handle_cast/2,code_change/3,handle_info/2]).
>>  -export([changes_since/5,changes_since/6,read_doc/2,new_revid/1]).
>> --export([check_is_admin/1, check_is_reader/1]).
>> +-export([check_is_admin/1, check_is_member/1]).
>>  -export([reopen/1]).
>> 
>>  -include("couch_db.hrl").
>> @@ -77,7 +77,7 @@ open(DbName, Options) ->
>>     case couch_server:open(DbName, Options) of
>>         {ok, Db} ->
>>             try
>> -                check_is_reader(Db),
>> +                check_is_member(Db),
>>                 {ok, Db}
>>             catch
>>                 throw:Error ->
>> @@ -297,14 +297,14 @@ check_is_admin(#db{user_ctx=#user_ctx{na
>>         ok
>>     end.
>> 
>> -check_is_reader(#db{user_ctx=#user_ctx{name=Name,roles=Roles}=UserCtx}=Db) ->
>> +check_is_member(#db{user_ctx=#user_ctx{name=Name,roles=Roles}=UserCtx}=Db) ->
>>     case (catch check_is_admin(Db)) of
>>     ok -> ok;
>>     _ ->
>> -        {Readers} = get_readers(Db),
>> -        ReaderRoles = couch_util:get_value(<<"roles">>, Readers,[]),
>> +        {Members} = get_members(Db),
>> +        ReaderRoles = couch_util:get_value(<<"roles">>, Members,[]),
>>         WithAdminRoles = [<<"_admin">> | ReaderRoles],
>> -        ReaderNames = couch_util:get_value(<<"names">>, Readers,[]),
>> +        ReaderNames = couch_util:get_value(<<"names">>, Members,[]),
>>         case ReaderRoles ++ ReaderNames of

I'd s/ReaderRoles/MemberRoles for naming consistency.


>>         [] -> ok; % no readers == public access
>>         _Else ->
>> @@ -326,8 +326,10 @@ check_is_reader(#db{user_ctx=#user_ctx{n
>>  get_admins(#db{security=SecProps}) ->
>>     couch_util:get_value(<<"admins">>, SecProps, {[]}).
>> 
>> -get_readers(#db{security=SecProps}) ->
>> -    couch_util:get_value(<<"readers">>, SecProps, {[]}).
>> +get_members(#db{security=SecProps}) ->
>> +    % we fallback to readers here for backwards compatibility
>> +    couch_util:get_value(<<"members">>, SecProps,
>> +        couch_util:get_value(<<"readers">>, SecProps, {[]})).
>> 
>>  get_security(#db{security=SecProps}) ->
>>     {SecProps}.
>> @@ -343,9 +345,11 @@ set_security(_, _) ->
>> 
>>  validate_security_object(SecProps) ->
>>     Admins = couch_util:get_value(<<"admins">>, SecProps, {[]}),
>> -    Readers = couch_util:get_value(<<"readers">>, SecProps, {[]}),
>> +    % we fallback to readers here for backwards compatibility
>> +    Members = couch_util:get_value(<<"members">>, SecProps,
>> +        couch_util:get_value(<<"readers">>, SecProps, {[]})),

How about making this

  Members = get_members(SecProps);

by doing:

get_members(#db{security=SecProps}) ->
    get_members_int(SecProps);
get_members(SecProps) ->
    get_members_int(SecProps),

get_members_int(SecProps) ->
    % we fallback to readers here for backwards compatibility
    couch_util:get_value(<<"members">>, SecProps,
        couch_util:get_value(<<"readers">>, SecProps, {[]})).


>>     ok = validate_names_and_roles(Admins),
>> -    ok = validate_names_and_roles(Readers),
>> +    ok = validate_names_and_roles(Members),
>>     ok.
>> 
>>  % validate user input
>> 
>> 
>> 

Cheers
Jan
--