Re: [Codematch-develop] DataTracker and CodeMatch access control

Henrik Levkowetz <henrik@levkowetz.com> Wed, 16 September 2015 18:13 UTC

Return-Path: <henrik@levkowetz.com>
X-Original-To: codematch-develop@ietfa.amsl.com
Delivered-To: codematch-develop@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id F3EFE1A87A6 for <codematch-develop@ietfa.amsl.com>; Wed, 16 Sep 2015 11:13:42 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -101.9
X-Spam-Level:
X-Spam-Status: No, score=-101.9 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, USER_IN_WHITELIST=-100] autolearn=ham
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id xS-XCzTPgnte for <codematch-develop@ietfa.amsl.com>; Wed, 16 Sep 2015 11:13:38 -0700 (PDT)
Received: from zinfandel.tools.ietf.org (zinfandel.tools.ietf.org [IPv6:2001:1890:123a::1:2a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 8DB231A1BB9 for <codematch-develop@ietf.org>; Wed, 16 Sep 2015 11:13:38 -0700 (PDT)
Received: from localhost ([::1]:36660 helo=vigonier.tools.ietf.org) by zinfandel.tools.ietf.org with esmtp (Exim 4.82_1-5b7a7c0-XX) (envelope-from <henrik@levkowetz.com>) id 1ZcHD1-0000sr-1p; Wed, 16 Sep 2015 11:13:31 -0700
To: Christian O'Flaherty <oflaherty@isoc.org>
References: <0EDC30A8-DD35-4153-B8F1-9AE27CB06E09@inf.ufrgs.br> <55DDD328.6020709@nostrum.com> <5A1D82C2-B60B-4A85-A4B2-E8B524C885D6@inf.ufrgs.br> <55DE0CB3.8020001@nostrum.com> <567B5A53-7A2E-4D1B-A486-37B824D986B0@inf.ufrgs.br> <55DF0B7B.7020902@levkowetz.com> <9B6DB401-A599-4C19-857C-D64241FC467B@inf.ufrgs.br> <AB2E13A5-0592-4419-B402-4A2425E946C0@isoc.org> <ED361C70-6E08-4559-99AF-E26B853A8B04@isoc.org>
From: Henrik Levkowetz <henrik@levkowetz.com>
Message-ID: <55F9B0A7.6000508@levkowetz.com>
Date: Wed, 16 Sep 2015 20:10:47 +0200
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:38.0) Gecko/20100101 Thunderbird/38.2.0
MIME-Version: 1.0
In-Reply-To: <ED361C70-6E08-4559-99AF-E26B853A8B04@isoc.org>
Content-Type: multipart/signed; micalg="pgp-sha256"; protocol="application/pgp-signature"; boundary="9tPdsBPlEu2MvtCM2TnQdNcWJ8EnjGIVr"
X-SA-Exim-Connect-IP: ::1
X-SA-Exim-Rcpt-To: henrik-sent@levkowetz.com, granville@inf.ufrgs.br, rjsparks@nostrum.com, codematch-develop@ietf.org, oflaherty@isoc.org
X-SA-Exim-Mail-From: henrik@levkowetz.com
X-SA-Exim-Scanned: No (on zinfandel.tools.ietf.org); SAEximRunCond expanded to false
Archived-At: <http://mailarchive.ietf.org/arch/msg/codematch-develop/_8nN6JuNy0Gp68bBNclb-1UcK7M>
Cc: Lisandro Zambenedetti Granville <granville@inf.ufrgs.br>, codematch-develop <codematch-develop@ietf.org>, Robert Sparks <rjsparks@nostrum.com>
Subject: Re: [Codematch-develop] DataTracker and CodeMatch access control
X-BeenThere: codematch-develop@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: "\"Discussion forum for the planning, coordination, and development of CodeMatch\"" <codematch-develop.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/codematch-develop>, <mailto:codematch-develop-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/codematch-develop/>
List-Post: <mailto:codematch-develop@ietf.org>
List-Help: <mailto:codematch-develop-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/codematch-develop>, <mailto:codematch-develop-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 16 Sep 2015 18:13:43 -0000

Hi Christian,

Working through my inbox:

On 2015-09-16 00:06, Christian O'Flaherty wrote:
> H Henrik, 
> 
> I don’t want to be pushy but we will have our codematch call tomorrow and we would like to discuss this part:
> 
>> We will need an assessment soon to check what’s missing to be “appropriate” and what would be the next steps to be part of a Datatracker release.
> 
> 
> Can you help us or join our call tomorrow at 16 UTC ?

Umm.  The email above landed in my inbox "today" at 6 minutes past midnight.

I don't know which "tomorrow" you refer to -- a date would be easier.  If
the tomorrow you were thinking of were today, then it's obviously too late.

Possibly Robert made it?

Sorry,

	Henrik

> Thanks,
> 
> Christian O'Flaherty - oflaherty@isoc.org <mailto:oflaherty@isoc.org>
> Regional Development - Internet Society 
> Skype/Gmail/Yahoo!:  christian.oflaherty 
> Phone: +598 98769636
> 
>> On Sep 4, 2015, at 10:57 AM, Christian O'Flaherty <oflaherty@isoc.org> wrote:
>> 
>> Hi Henrik,
>> 
>> We will follow your advise on permissions and roles. 
>> The Datamodel and readability issues (conventions on variable names and class attributes) were also fixed (those that you flagged when we started coding).
>> 
>> We will need an assessment soon to check what’s missing to be “appropriate” and what would be the next steps to be part of a Datatracker release.
>> 
>> Can you help us with that? 
>> 
>> Christian O'Flaherty - oflaherty@isoc.org <mailto:oflaherty@isoc.org>
>> Regional Development - Internet Society 
>> Skype/Gmail/Yahoo!:  christian.oflaherty 
>> Phone: +598 98769636
>> 
>>> On Aug 27, 2015, at 3:30 PM, Lisandro Zambenedetti Granville <granville@inf.ufrgs.br <mailto:granville@inf.ufrgs.br>> wrote:
>>> 
>>> Hello Henrik
>>> 
>>> Thanks for all clarifications. Going to the most important question:
>>> 
>>> "Could we agree on starting an RBAC implementation using has_role(), and
>>> once we have a bit more experience, and only then, consider taking on
>>> the added complexity and technical debt of using 2 different permission
>>> handling systems in this non-COTS software?”
>>> 
>>> Definitely yes. I’m convinced, after reading your arguments, that having two different permission solutions in the same framework can be ok for CodeMatch but it wouldn’t be ok for datatracker. An your suggestion of concentrating now more on who can/cannot do what should be a priority.
>>> 
>>> Best regards,
>>> 
>>> Lisandro
>>> 
>>>> Em 27/08/2015, à(s) 10:07, Henrik Levkowetz <henrik@levkowetz.com <mailto:henrik@levkowetz.com>> escreveu:
>>>> 
>>>> Hi Lisandro,
>>>> 
>>>> Jumping in here with some comments:
>>>> 
>>>> On 2015-08-26 21:38, Lisandro Zambenedetti Granville wrote:
>>>>> Hello Robert
>>>>> 
>>>>>>>> You are not necessarily restricted to the Role names that are
>>>>>>>> defined now (like "chair"). We could add Role names like 
>>>>>>>> "codematcher" or "codematch_approver" or whatever other name
>>>>>>>> matches the semantic for the permission you're wanting to
>>>>>>>> manage.
>>>>>>> 
>>>>>>> That’s great, so that we can expand the current Roles without
>>>>>>> changing the data model.
>>>>>>> 
>>>>>>>> You could then have things like Lisandro Granville is
>>>>>>>> codematch_approver in nmrg.
>>>>>>> 
>>>>>>> That is also what we need.
>>>>>> 
>>>>>> Just to be sure - you're saying that's sufficient, and while you're
>>>>>> explaining the idea further below, you don't need it right now?
>>>>> 
>>>>> Sorry, I was not clear. Creating new Roles and allowing users being
>>>>> assigned to these new Roles are things that we need in CodeMatch too.
>>>>> Further below I mention something that we are proposing (i.e.,
>>>>> has_right) in addition to having new roles.
>>>>> 
>>>> 
>>>>>>>> Does that address the need? If not, could you walk me though a
>>>>>>>> scenario where it makes it harder than it should?
>>>>>>> 
>>>>>>> What you mention above is the link among users, roles, and
>>>>>>> groups. There is another link between roles and permissions that
>>>>>>> seems to be hardcoded in datatracker. When we say
>>>>>>> 
>>>>>>> if has_role(request.user, ["Area Director", "IAB Chair", "Secretariat”]):
>>>>>>> 	// do things the user with "permission X" can do
>>>>>>> 
>>>>>>> it implies that if we want to grant to, e.g.,
>>>>>>> “codematch_approver” the “permission X” above, we should go back
>>>>>>> to the code and change it to
>>>>>>> 
>>>>>>> if has_role(request.user, ["Area Director", "IAB Chair",
>>>>>>> “Secretariat”, “codematch_approver"]):...
>>>>>>> 
>>>>>>> We were then considering the possibility of moving the link
>>>>>>> between roles and permission to the data model and implement
>>>>>>> has_right (or has_permission) like:
>>>>>>> 
>>>>>>> if has_right(request.user, “permission X”):
>>>>>>> 	// do things the user with "permission X" can do
>>>>>>> 
>>>>>>> If we want “permission X” to be granted to other roles, that
>>>>>>> would be a matter of including the proper records in the
>>>>>>> database, instead of changing the code.
>>>>>>> 
>>>>>>> Trying not to go too much into details, the summary is that we
>>>>>>> want to check if assigning permission to roles is ok to be done
>>>>>>> in the database, instead of changing the code.
>>>>>> 
>>>>>> Well, you still have to type has_right(...) in the code, and
>>>>>> specify something concrete in that has_right that would have to
>>>>>> change, so I don't think we're really talking about avoiding
>>>>>> changing code as much as we are about _where_ you model the idea of
>>>>>> a permission in the database.
>>>>> 
>>>>> I agree. We are talking about modeling the link between roles and
>>>>> permission in the database.
>>>> 
>>>> So, the design you propose is elegant and matches the way permissions
>>>> are handled in many contexts, especially in COTS software (and for
>>>> that matter, user-configurable FOSS software).
>>>> 
>>>> It is more flexible, but also has an added complexity, and an added
>>>> abstraction level (in order to be useful, the permissions need to be
>>>> named with names which express what effect they are having in the code,
>>>> and this may be non-trivial).
>>>> 
>>>> There is however one point you don't bring up, which I would like to
>>>> bring up here.
>>>> 
>>>> Summarizing the two ways of writing the code, we can either have code
>>>> which says
>>>> 
>>>> if has_role(user, ["CodeMatch Approver"]):
>>>>    # code to be exectuted
>>>> 
>>>> or code which says
>>>> 
>>>> if has_perm(user, ["approve codematch"]):
>>>>    # code to be exectuted
>>>> 
>>>> For IETF roles, the list of possible roles are very well codified,
>>>> while the only way to know which permissions that need to be named
>>>> and listed in a role_auth_permission table is to go through the
>>>> code and name all the places we check for roles today.  Naming those
>>>> permissions, and making clear the correspondence between the names
>>>> and the actual effect they will have in code is not necessarily
>>>> trivial.
>>>> 
>>>> For the Codematch project, it may be that you have a much better grasp
>>>> of the named permissions needed, and not so good a grasp of the roles.
>>>> 
>>>> However, since irrespective of how the correspondence between roles and
>>>> code is handled, whether it is via has_role() or has_perm(), you actually
>>>> _need_ to be able to describe the roles well, I would suggest that you
>>>> probably also have a much better grasp of the roles than of all the
>>>> named permissions which may be needed.
>>>> 
>>>> Given that, I would suggest that you start out with the same approach
>>>> which has been used in the current datatracker code, and once it is
>>>> possible to inspect the code and see which actual places need to check
>>>> for different roles in order to determine if a given piece of code may
>>>> be executed, _and_ you have experience with the tool so that you can
>>>> speak with confidence about which actions a given role should or should
>>>> not have, then _at that point_ we can consider refining things into
>>>> roles and permissions, instead of listing roles explicitly in the code.
>>>> 
>>>> More below.
>>>> 
>>>>>> I think the real essence of what we're talking about is the
>>>>>> difference in how you would implement "Give all the WG chairs
>>>>>> codematch-approval rights".
>>>>>> 
>>>>>> With your proposal, you would make a role to permission match to do
>>>>>> that - one database row change - pretty efficient (*) With what we
>>>>>> have now, you just add a Role of "codematch-approval" to any Person
>>>>>> that has a wg chair role. So, I think what you're proposing is an
>>>>>> optimization, and it comes with some complexity. We can keep
>>>>>> talking about it, but I don't think we're at the point that we need
>>>>>> it.
>>>>> 
>>>>> This complexity involves:
>>>>> 
>>>>> a) Creating a new intermediate table “role_auth_permission" composed
>>>>> of, at least, two columns: role_id and auth_permission_id;
>>>>> 
>>>>> b) Creating the function "has_right (user, permission_label)" which
>>>>> given the user it retrieves the user’s roles, from roles it retrieves
>>>>> permissions and, among permissions, it checks if permission_label is
>>>>> present.
>>>>> 
>>>>> Of course, the effort of doing a) and b) would be ours. And these
>>>>> implementation would be used in CodeMatch only; it wouldn’t be
>>>>> changing or affecting DataTracker.
>>>> 
>>>> Understood.  But for the datatracker, there is the added complexity of
>>>> maintaining code which uses 2 different mechanisms to handle permissions.
>>>> And that the technical debt of carrying two different solutions forward
>>>> is something you will not have to suffer, but the datatracker project
>>>> will have to deal with.
>>>> 
>>>>>> (*) But, it's not really that easy. Right now we have a role of
>>>>>> "chair" that is applied to all groups. There's no way to speak
>>>>>> differently about WG chairs and RG chairs by simply saying "chair"
>>>>>> - it takes more code. We have chairs of Teams, and _could_ have
>>>>>> (but don't currently) chairs of other SDOs in the database. So, the
>>>>>> apparent simplicity of matching a permission to a role-name isn't
>>>>>> as strong as it first appears.
>>>>> 
>>>>> While coding the CodeMatch system, we notice that some portions of
>>>>> the system may be or may be not available to different user roles.
>>>>> Although we define somes roles now, changes on role-permissions will
>>>>> probably happen along the development process.
>>>> 
>>>> Agreed.  In which case, doing the necessary has_role() or has_perm() 
>>>> code changes will most likely happen along with other code changes.
>>>> 
>>>>> The permissions we want the roles to have define, let me say, the
>>>>> policy of CodeMatch. We wanted to decouple that policy from the
>>>>> system implementation itself, thus implementing an RBAC model; if the
>>>>> link between roles and permissions (i.e., the system policy) migrates
>>>>> to the database, we can tag CodeMatch source code (i.e., define
>>>>> permission labels) and redefine which roles are able to access
>>>>> different parts of the system. Notice, this is just for CodeMatch, of
>>>>> course.
>>>> 
>>>> RBAC can be implemented both with has_role() and has_perm(); the difference
>>>> lies in how easy it is to reconfigure the permissions which are associated
>>>> with a given role.
>>>> 
>>>> For COTS software, where you cannot easily adapt the code itself to the
>>>> customer environment and wishes, other than through data lookup, the
>>>> approach you suggest makes a lot of sense.  For our case, where there is
>>>> one and only one production system, it is far less important, and it is
>>>> not obvious that the added complexity and technical debt incurred is
>>>> offset by the added flexibility.
>>>> 
>>>> Please also consider the pace with which we release software for the
>>>> datatracker:  Formal releases are shown in the page:
>>>> 
>>>>  https://datatracker.ietf.org/release/ <https://datatracker.ietf.org/release/>
>>>> 
>>>> and in addition to that, we often apply patches.  As an example, we
>>>> have done 10 formal releases within the 6.x.y series, but during the
>>>> same time, we have applied 36 patches, resulting in 20 patch release
>>>> numbers in addition to the formal release numbers.  With this release
>>>> pace, and with the possibility of applying patches,  we are probably
>>>> agile enough to handle the necessary code changes almost as fast as you
>>>> could agree on what needs to be done and change the settings in a 
>>>> role_auth_permission table.
>>>> 
>>>> Could we agree on starting an RBAC implementation using has_role(), and
>>>> once we have a bit more experience, and only then, consider taking on
>>>> the added complexity and technical debt of using 2 different permission
>>>> handling systems in this non-COTS software?
>>>> 
>>>> 
>>>> Best regards,
>>>> 
>>>> 	Henrik
>>>> 
>>>> 
>>>>> Thanks,
>>>>> 
>>>>> Lisandro
>>>>> 
>>>>>>> We are of course talking about the CodeMatch code only; we are not talking about the datatracker code at all, although examples were inspired by has_role, which is used in datatracker.
>>>>>>> 
>>>>>>> Lisandro
>>>>>>> 
>>>>>>>> On 8/25/15 1:52 PM, Lisandro Zambenedetti Granville wrote:
>>>>>>>>> Dear All
>>>>>>>>> 
>>>>>>>>> Last week we had our traditional CodeMatch meeting. One of the questions discussed in the meeting was about role-based access control (RBAC) in CodeMatch. We would like to propose adding an extra table in the current data model to support RBAC. However, because we want to be as aligned with DataTracker style as possible, we are sending this message to start a discussion, specially with Henrik and Robert.
>>>>>>>>> 
>>>>>>>>> 1) Style of checking users’ permission
>>>>>>>>> 
>>>>>>>>> We observed the DataTracker code and it seems that in general, permission checking is hardcoded, in the following style (using “Area Director”, “IAB Chair”, and “Secretariat" as examples):
>>>>>>>>> 
>>>>>>>>> if has_role(request.user, ["Area Director", "IAB Chair", "Secretariat"]):
>>>>>>>>> 	// do something that only area diretor, IAB chair, and secretariat could do, like “Create CodeRequest"
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> In CodeMatch, we would like to check permissions in the following way:
>>>>>>>>> 
>>>>>>>>> if has_right(request.user, “Create CodeRequest”):
>>>>>>>>> 	// do something that only authorized people should be able to do, like “Create CodeRequest”
>>>>>>>>> 
>>>>>>>>> The “has_right” function would check the database to retrieve the users’ roles and, for each role, check if it has permission to “Create CodeRequest”. In this way, permissions are associated to roles, and roles associated to users.
>>>>>>>>> 
>>>>>>>>> Because permissions and roles in CodeMatch are being defined together with the implementation of the system prototype, the use of “has_right” would allow us to assign permissions to roles just changing the database, instead of changing the CodeMatch code if we use “has_role” instead.
>>>>>>>>> 
>>>>>>>>> 2) Database
>>>>>>>>> 
>>>>>>>>> Today, permissions are listed in table auth_permission. Roles are listed in table group_role. We would need a intermediate table linking permissions to roles, i.e., a table linking author_permissions and group_role. That would allow us to say, for example, that a mentor (inside group_role) can add documents to a codeRequest (i.e., a permission inside auth_permission). Adding this intermediate table (let’s call it role_permission for the moment) would not affect today’s database, although it would expand today’s data model.
>>>>>>>>> 
>>>>>>>>> Do you think doing that is ok?
>>>>>>>>> 
>>>>>>>>> Best regards,
>>>>>>>>> 
>>>>>>>>> Lisandro, Wanderson, Matheus
>>>>>>>>> 
>>>>>>>> _______________________________________________
>>>>>>>> Codematch-develop mailing list
>>>>>>>> Codematch-develop@ietf.org <mailto:Codematch-develop@ietf.org>
>>>>>>>> https://www.ietf.org/mailman/listinfo/codematch-develop
>>>>>> 
>>>>> 
>>>>> 
>>> 
>>> _______________________________________________
>>> Codematch-develop mailing list
>>> Codematch-develop@ietf.org <mailto:Codematch-develop@ietf.org>
>>> https://www.ietf.org/mailman/listinfo/codematch-develop
>> 
> 
>