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

From: Lisandro Zambenedetti Granville <[email-masked]>
Date: Tue, 25 Aug 2015 15:52:19 -0300

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

---

From: Robert Sparks <[email-masked]>
Date: Wed, 26 Aug 2015 09:54:32 -0500

Hi Lisandro -

Before we go too deep into this, I want to call out a way we expected 
Role to be used and make sure you've considered it.

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.

You could then have things like Lisandro Granville is codematch_approver 
in nmrg.

Does that address the need? If not, could you walk me though a scenario 
where it makes it harder than it should?

RjS

---

From: Lisandro Zambenedetti Granville <[email-masked]>
Date: Wed, 26 Aug 2015 15:26:37 -0300

Hello Roberts

Comments inline...

> 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.

> 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. 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

> _______________________________________________
> Codematch-develop mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/codematch-develop

---

From: Robert Sparks <[email-masked]>
Date: Wed, 26 Aug 2015 14:00:03 -0500

> 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?
> 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 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.

(*) 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.
>   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
>

---

From: Lisandro Zambenedetti Granville <[email-masked]>
Date: Wed, 26 Aug 2015 16:38:07 -0300

Hello Robert

> 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.

> 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.

> I think the real essence of what we're talking about is the difference in how you would implement
> 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

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.

> (*) 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. 

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. 

Thanks,

Lisandro

---

From: Henrik Levkowetz <[email-masked]>
Date: Thu, 27 Aug 2015 15:07:07 +0200

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.
>>> 
> 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
>>>
>>> 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 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.
> 

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.
> 
> 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/

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

---

From: Lisandro Zambenedetti Granville <[email-masked]>
Date: Thu, 27 Aug 2015 15:30:47 -0300

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 <[email-masked]> escreveu:
> 
> Hi Lisandro,
>