db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Daniel John Debrunner (JIRA)" <derby-...@db.apache.org>
Subject [jira] Commented: (DERBY-1330) Provide runtime privilege checking for grant/revoke functionality
Date Sat, 08 Jul 2006 05:44:31 GMT
    [ http://issues.apache.org/jira/browse/DERBY-1330?page=comments#action_12419836 ] 

Daniel John Debrunner commented on DERBY-1330:
----------------------------------------------

What would be really helpful if was if this patch was split into multiple patches.

I'm finally realizing the patch is making several changes:

1) Add mechanism to stop collection of privilege information during compilation

2) Use mechanism of 1) for view execution

3) Add a UUID to the existing privilege systems tables.

4) Change the PrivilegeDescriptor classes to be Providers as well as being
      TupleDescriptors
 
5) Allowing GRANT to execute on views

6) Make CREATE VIEW depend  on the(existing)  list of priviliges collected

7) Stuff added for triggers added recently that I haven't managed to look at.

8) Stuff added for constraints added recently that I haven't managed to look at.

9) Others I've missed due to the size of the patch

+) Test changes.

So 8+) seperate changes "forced" into a single 352k patch. Personally I find patches like
this really hard to review due to their size  and multiple issues.  I believe the risk of
missing something in a review or in writing the code is increased by the size of the change,
thus the quality of Derby can be degraded by accepting large patches. Incremental development
is good.

I know it's hard to anticipate & identify incremental changes from the start of  implementing
functionality,  but it's a good habit to get into. As one develops something and realise that
a new piece of functionality is needed, stop and think if it could be implemented in a separate
clean code line and delivered as a separate patch. For example, with this change the requirement
for the UUID to be added to the existing system tables could have been identified as a stand-alone
item.
Another technique I use is to separate items after the fact. Once a codeline becomes clogged
with multiple changes, I pull individual changes into a separate code line and then test &
submit incrementally. I will write this up in the wiki.

Now, I do believe that the we, the community, could maybe do a better job to encourage smaller
patches, faster turn around on submitted small patches etc., but I also believe, in general,
the contributors can help a lot here. At least by following the guidelines from the wiki.
I think we are in somewhat of a negative-feedback situation, patches are slow to be reviewed
& committed due to time constraints and patch complexity leading to contributors trying
to cram more things into a single patch to only have to undergo a single commit leading to
longer review times etc. etc. If we could break the cycle and have fast turnaround on small
patches it would move more people to the incremental development model.

As for this specific patch, I'm not sure it's a good plan to have you submit 8+ patches &
the testing associated with it, given we have a working patch here. There are some more changes
I would like to see, and I would like you to refrain from adding any more functionality into
the patch. I wll add specific review comments in another comment.


> Provide runtime privilege checking for grant/revoke functionality
> -----------------------------------------------------------------
>
>          Key: DERBY-1330
>          URL: http://issues.apache.org/jira/browse/DERBY-1330
>      Project: Derby
>         Type: Sub-task

>   Components: SQL
>     Versions: 10.2.0.0
>     Reporter: Mamta A. Satoor
>     Assignee: Mamta A. Satoor
>  Attachments: AuthorizationModelForDerbySQLStandardAuthorization.html, AuthorizationModelForDerbySQLStandardAuthorizationV2.html,
Derby1330PrivilegeCollectionV2diff.txt, Derby1330PrivilegeCollectionV2stat.txt, Derby1330ViewPrivilegeCollectionV1diff.txt,
Derby1330ViewPrivilegeCollectionV1stat.txt
>
> Additional work needs to be done for grant/revoke to make sure that only users with required
privileges can access various database objects. In order to do that, first we need to collect
the privilege requirements for various database objects and store them in SYS.SYSREQUIREDPERM.
Once we have this information then when a user tries to access an object, the required SYS.SYSREQUIREDPERM
privileges for the object will be checked against the user privileges in SYS.SYSTABLEPERMS,
SYS.SYSCOLPERMS and SYS.SYSROUTINEPERMS. The database object access will succeed only if the
user has the necessary privileges.
> SYS.SYSTABLEPERMS, SYS.SYSCOLPERMS and SYS.SYSROUTINEPERMS are already populated by Satheesh's
work on DERBY-464. But SYS.SYSREQUIREDPERM doesn't have any information in it at this point
and hence no runtime privilege checking is getting done at this point.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


Mime
View raw message