db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dag H. Wanvik (JIRA)" <j...@apache.org>
Subject [jira] Updated: (DERBY-3223) SQL roles: make use of privileges granted to roles in actual privilege checking
Date Mon, 14 Jul 2008 22:36:31 GMT

     [ https://issues.apache.org/jira/browse/DERBY-3223?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Dag H. Wanvik updated DERBY-3223:
---------------------------------

    Attachment: derby-3223-activate-roles-2.stat
                derby-3223-activate-roles-2.diff

Thanks for looking at this patch, Knut!

The new version of this patch, derby-3223-activate-roles-2, addresses
most of your comments, except as noted below:

  - if the current role is not granted to the user, would it be more 
    appropriate to raise an exception saying that than to set the role 
    to null? 

No, this is the way I handle role revocation; the session(s) which has
a role set "discover" that they have lost the right the next time they
attempt to make use of it ("lazily"); the revoking session (data base
owner) is thus relieved of synchronizing with these session threads to
set their current roles to none.

  - (in old code that is moved, not new code - same in 
    StatementTablePermission and StatementColumnPermission) error 
    messages are created with hard-coded strings "routine" and 
    "schema" that won't be localized 

Right, I leave that for another issue. I am not sure it needs fixing
though, since both strings are used in SQL syntax (ROUTINE is
valid SQL, although not in the Derby syntax). Maybe it would be good to
make them uppercase to underscore that fact.

  - perhaps the code will be slightly simpler if the role check in 
    hasPermissionOnTable() is factored out into a separate method? 

I chose to let this method be as it is..

  - assertPrivilegeMetadata(): is JDBC.identifierToCNF() needed around 
    the literal "test_dbo"? 

Well, either that or use upper case literal; I chose to use
identifierToCNF throughout when comparing against strings from
metadata queries which are in CNF.

  - assertPrivilegeMetadata(): garbage in comment (ยน) 

I removed this. It was originally a ISO-8859-1 superscript "1" which
is legal in Java comments, but I think the JIRA attachment mangled it
to what you saw. I'll stick to 7-bit ASCII... ;)

  - setRole() looks like a useful helper method. Should it be moved to 
    JDBC? 

Good idea, but I think I will postpone this until I have
resolved the semantics of SET ROLE, cf. the discussion in DERBY-3137.

As for the close method of the iterator, I originally planned to use
this for decrementing a usage counter if/when I get to caching the
grant graphs, but you are right, right now it is totally useless, so I
remove it for now. Better to re-introduce it later if needed.


> SQL roles: make use of privileges granted to roles in actual privilege checking
> -------------------------------------------------------------------------------
>
>                 Key: DERBY-3223
>                 URL: https://issues.apache.org/jira/browse/DERBY-3223
>             Project: Derby
>          Issue Type: Task
>          Components: Security, SQL
>            Reporter: Dag H. Wanvik
>            Assignee: Dag H. Wanvik
>             Fix For: 10.5.0.0
>
>         Attachments: derby-3223-1a.diff, derby-3223-1a.stat, derby-3223-1b.diff, derby-3223-1b.stat,
derby-3223-1c.diff, derby-3223-1c.stat, derby-3223-1d.diff, derby-3223-1d.stat, derby-3223-activate-roles-1.diff,
derby-3223-activate-roles-1.stat, derby-3223-activate-roles-2.diff, derby-3223-activate-roles-2.stat,
derby-3223-revise-iterator-api-b.diff, derby-3223-revise-iterator-api-b.stat, derby-3223-revise-iterator-api.diff,
derby-3223-revise-iterator-api.stat, roles.sql, roles2.sql, roles3.sql
>
>
> Pushing out to 10.5

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message