Derby
  1. Derby
  2. DERBY-1858

A schema can be dropped by a non-schema owner in SQL authorization mode

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.2.1.6, 10.2.2.0, 10.3.1.4
    • Fix Version/s: 10.2.1.6, 10.3.1.4
    • Component/s: SQL
    • Labels:
      None
    • Environment:
      Any

      Description

      drop schema user2 restrict should fail since user3 does not own the schema user2.

      ij version 10.3
      ij> connect 'wombat;create=true' user 'user1' as user1;
      WARNING 01J14: SQL authorization is being used without first enabling authentication.
      ij> connect 'wombat;create=true' user 'user2' as user2;
      WARNING 01J01: Database 'wombat' not created, connection made to existing database instead.
      WARNING 01J14: SQL authorization is being used without first enabling authentication.
      ij(USER2)> create schema user2;
      0 rows inserted/updated/deleted
      ij(USER2)> connect 'wombat;create=true' user 'user3' as user3;
      WARNING 01J01: Database 'wombat' not created, connection made to existing database instead.
      WARNING 01J14: SQL authorization is being used without first enabling authentication.
      ij(USER3)> drop schema user2 restrict;
      0 rows inserted/updated/deleted

      sysinfo:

      ------------------ Java Information ------------------
      Java Version: 1.4.2_12
      Java Vendor: Sun Microsystems Inc.
      Java home: c:\jdk142\jre
      Java classpath: classes;.
      OS name: Windows XP
      OS architecture: x86
      OS version: 5.1
      Java user name: Yip
      Java user home: C:\Documents and Settings\Yip
      Java user dir: C:\work3\derby\trunk
      java.specification.name: Java Platform API Specification
      java.specification.version: 1.4
      --------- Derby Information --------
      JRE - JDBC: J2SE 1.4.2 - JDBC 3.0
      [C:\work3\derby\trunk\classes] 10.3.0.0 alpha - (446666)
      ------------------------------------------------------
      ----------------- Locale Information -----------------
      Current Locale : [English/United States [en_US]]
      Found support for locale: [de_DE]
      version: 10.3.0.0 alpha - (446666)
      Found support for locale: [es]
      version: 10.3.0.0 alpha - (446666)
      Found support for locale: [fr]
      version: 10.3.0.0 alpha - (446666)
      Found support for locale: [it]
      version: 10.3.0.0 alpha - (446666)
      Found support for locale: [ja_JP]
      version: 10.3.0.0 alpha - (446666)
      Found support for locale: [ko_KR]
      version: 10.3.0.0 alpha - (446666)
      Found support for locale: [pt_BR]
      version: 10.3.0.0 alpha - (446666)
      Found support for locale: [zh_CN]
      version: 10.3.0.0 alpha - (446666)
      Found support for locale: [zh_TW]
      version: 10.3.0.0 alpha - (446666)
      ------------------------------------------------------

      1. derby1858-trunk-diff01.txt
        10 kB
        Yip Ng
      2. derby1858-trunk-diff02.txt
        10 kB
        Yip Ng
      3. derby1858-trunk-stat01.txt
        0.7 kB
        Yip Ng
      4. derby1858-trunk-stat02.txt
        0.7 kB
        Yip Ng

        Activity

        Hide
        Yip Ng added a comment -

        Attaching patch derby1858-trunk-diff01.txt for DERBY-1858. The problem is that DropSchemaNode's bind phase did not add the required schema privilege for it to check at runtime. derbyall ran successfully.

        Show
        Yip Ng added a comment - Attaching patch derby1858-trunk-diff01.txt for DERBY-1858 . The problem is that DropSchemaNode's bind phase did not add the required schema privilege for it to check at runtime. derbyall ran successfully.
        Hide
        Mike Matrigali added a comment -

        patch is available, making it a 10.2 candidate.

        Show
        Mike Matrigali added a comment - patch is available, making it a 10.2 candidate.
        Hide
        Mamta A. Satoor added a comment -

        Yip, I did a review of the attached package and following couple questions/comments.

        1)I had question about changes to DropSchemaNode. I realize that my question here is related to pre-existing logic that you needed to extend for drop schema. But I wondered why for table privilege, we push the privilege requirement(select/trigger/update etc) by calling CompilerContext.pushCurrentPrivType and use that to determine what kind of privilege is required for the table in CompilerContextImpl.addRequiredTablePriv method. But for privilege requirement for schemas, we pass the privilege type to CompilerContextImpl.addRequiredSchemaPriv rather than pushing it onto CompilerContext and then popping it back from there at the right time. I should have asked this question when the code was originally checked in for CreateSchemaNode. This is not a biggie, just a curiosity question.

        2)In Authorizer.java, there is a constant called PRIV_TYPE_COUNT which has a value of 7. With the patch attached to this Jira entry, we have added 1 more privilege type. Do we need to change PRIV_TYPE_COUNT? May be the purpose of PRIV_TYPE_COUNT is to track only table and routine level privilege types and that is why we didn't need to modify it for this patch.

        3)Can we have some positive test cases for drop schema in grantRevokeDDL2.sql showing that a database owner or schema owner can successfully drop the schema?

        Other than that, the changes look pretty good to me. Thanks, Yip.

        Show
        Mamta A. Satoor added a comment - Yip, I did a review of the attached package and following couple questions/comments. 1)I had question about changes to DropSchemaNode. I realize that my question here is related to pre-existing logic that you needed to extend for drop schema. But I wondered why for table privilege, we push the privilege requirement(select/trigger/update etc) by calling CompilerContext.pushCurrentPrivType and use that to determine what kind of privilege is required for the table in CompilerContextImpl.addRequiredTablePriv method. But for privilege requirement for schemas, we pass the privilege type to CompilerContextImpl.addRequiredSchemaPriv rather than pushing it onto CompilerContext and then popping it back from there at the right time. I should have asked this question when the code was originally checked in for CreateSchemaNode. This is not a biggie, just a curiosity question. 2)In Authorizer.java, there is a constant called PRIV_TYPE_COUNT which has a value of 7. With the patch attached to this Jira entry, we have added 1 more privilege type. Do we need to change PRIV_TYPE_COUNT? May be the purpose of PRIV_TYPE_COUNT is to track only table and routine level privilege types and that is why we didn't need to modify it for this patch. 3)Can we have some positive test cases for drop schema in grantRevokeDDL2.sql showing that a database owner or schema owner can successfully drop the schema? Other than that, the changes look pretty good to me. Thanks, Yip.
        Hide
        Yip Ng added a comment -

        Thanks for reviewing the patch, Mamta. My response to your comments:

        1) Yes, I noticed that also. If you look at the CREATE_SCHEMA_PRIV and MODIFY_SCHEMA_PRIV, they are of boolean types and not like the other 7 privilege types which are declared as integers. So it looks like privilege collection for schema are done abit differently and they were not considered as an integral part of the push/pop privilege logic in compiler context. (They could be, but its not necessary)

        2) The PRIV_TYPE_COUNT should still be 7 since those privilege types(select, insert, update, ..., etc.) are part of
        grant/revoke statement. The other schema privileges are used sorely to differentiate what action to take during permission checking time.

        3) Yes, there is already a testcase to test if database owner can drop the schema. I believe I have uncommented that part of test in grantRevokeDDL2.sql to re-enable it for this jira.

        Show
        Yip Ng added a comment - Thanks for reviewing the patch, Mamta. My response to your comments: 1) Yes, I noticed that also. If you look at the CREATE_SCHEMA_PRIV and MODIFY_SCHEMA_PRIV, they are of boolean types and not like the other 7 privilege types which are declared as integers. So it looks like privilege collection for schema are done abit differently and they were not considered as an integral part of the push/pop privilege logic in compiler context. (They could be, but its not necessary) 2) The PRIV_TYPE_COUNT should still be 7 since those privilege types(select, insert, update, ..., etc.) are part of grant/revoke statement. The other schema privileges are used sorely to differentiate what action to take during permission checking time. 3) Yes, there is already a testcase to test if database owner can drop the schema. I believe I have uncommented that part of test in grantRevokeDDL2.sql to re-enable it for this jira.
        Hide
        Rick Hillegas added a comment -

        Moving to 10.2.2.0.

        Show
        Rick Hillegas added a comment - Moving to 10.2.2.0.
        Hide
        Mike Matrigali added a comment -

        I looked at applying this patch, but it failed with the following warning, not sure if it is my environment or that the patch needs to be regenerated:

        m1_ibm142:52>pwd
        C:/p4/m1/opensource
        m1_ibm142:53>patch --dry-run -p0 -i c:/tmp/derby1858-trunk-diff01.txt
        patching file `java/engine/org/apache/derby/impl/sql/compile/CompilerContextImpl
        .java'
        patching file `java/engine/org/apache/derby/impl/sql/compile/DropSchemaNode.java
        '
        patching file `java/engine/org/apache/derby/iapi/sql/compile/CompilerContext.jav
        a'
        patching file `java/engine/org/apache/derby/iapi/sql/conn/Authorizer.java'
        patching file `java/engine/org/apache/derby/iapi/sql/dictionary/StatementSchemaP
        ermission.java'
        Hunk #3 FAILED at 66.
        1 out of 3 hunks FAILED – saving rejects to java/engine/org/apache/derby/iapi/s
        ql/dictionary/StatementSchemaPermission.java.rej
        patching file `java/testing/org/apache/derbyTesting/functionTests/tests/lang/gra
        ntRevokeDDL2.sql'
        patching file `java/testing/org/apache/derbyTesting/functionTests/master/jdk16/g
        rantRevokeDDL2.out'
        patching file `java/testing/org/apache/derbyTesting/functionTests/master/grantRe
        vokeDDL2.out'

        Show
        Mike Matrigali added a comment - I looked at applying this patch, but it failed with the following warning, not sure if it is my environment or that the patch needs to be regenerated: m1_ibm142:52>pwd C:/p4/m1/opensource m1_ibm142:53>patch --dry-run -p0 -i c:/tmp/derby1858-trunk-diff01.txt patching file `java/engine/org/apache/derby/impl/sql/compile/CompilerContextImpl .java' patching file `java/engine/org/apache/derby/impl/sql/compile/DropSchemaNode.java ' patching file `java/engine/org/apache/derby/iapi/sql/compile/CompilerContext.jav a' patching file `java/engine/org/apache/derby/iapi/sql/conn/Authorizer.java' patching file `java/engine/org/apache/derby/iapi/sql/dictionary/StatementSchemaP ermission.java' Hunk #3 FAILED at 66. 1 out of 3 hunks FAILED – saving rejects to java/engine/org/apache/derby/iapi/s ql/dictionary/StatementSchemaPermission.java.rej patching file `java/testing/org/apache/derbyTesting/functionTests/tests/lang/gra ntRevokeDDL2.sql' patching file `java/testing/org/apache/derbyTesting/functionTests/master/jdk16/g rantRevokeDDL2.out' patching file `java/testing/org/apache/derbyTesting/functionTests/master/grantRe vokeDDL2.out'
        Hide
        Yip Ng added a comment -

        Regenerate patch for trunk - derby1858-trunk-diff02.txt

        Show
        Yip Ng added a comment - Regenerate patch for trunk - derby1858-trunk-diff02.txt
        Hide
        Mike Matrigali added a comment -

        thanks, the 2nd patch applied cleanly for me.

        Show
        Mike Matrigali added a comment - thanks, the 2nd patch applied cleanly for me.
        Hide
        Mike Matrigali added a comment -

        committed change to the trunk.

        m1_ibm142:161>svn commit

        Sending java\engine\org\apache\derby\iapi\sql\compile\CompilerContext.jav
        a
        Sending java\engine\org\apache\derby\iapi\sql\conn\Authorizer.java
        Sending java\engine\org\apache\derby\iapi\sql\dictionary\StatementSchemaP
        ermission.java
        Sending java\engine\org\apache\derby\impl\sql\compile\CompilerContextImpl
        .java
        Sending java\engine\org\apache\derby\impl\sql\compile\DropSchemaNode.java

        Sending java\testing\org\apache\derbyTesting\functionTests\master\grantRe
        vokeDDL2.out
        Sending java\testing\org\apache\derbyTesting\functionTests\master\jdk16\g
        rantRevokeDDL2.out
        Sending java\testing\org\apache\derbyTesting\functionTests\tests\lang\gra
        ntRevokeDDL2.sql
        Transmitting file data ........
        Committed revision 449869.

        Show
        Mike Matrigali added a comment - committed change to the trunk. m1_ibm142:161>svn commit Sending java\engine\org\apache\derby\iapi\sql\compile\CompilerContext.jav a Sending java\engine\org\apache\derby\iapi\sql\conn\Authorizer.java Sending java\engine\org\apache\derby\iapi\sql\dictionary\StatementSchemaP ermission.java Sending java\engine\org\apache\derby\impl\sql\compile\CompilerContextImpl .java Sending java\engine\org\apache\derby\impl\sql\compile\DropSchemaNode.java Sending java\testing\org\apache\derbyTesting\functionTests\master\grantRe vokeDDL2.out Sending java\testing\org\apache\derbyTesting\functionTests\master\jdk16\g rantRevokeDDL2.out Sending java\testing\org\apache\derbyTesting\functionTests\tests\lang\gra ntRevokeDDL2.sql Transmitting file data ........ Committed revision 449869.
        Hide
        Yip Ng added a comment -

        I think this jira can be marked as resolved once it is backported to 10.2.

        Show
        Yip Ng added a comment - I think this jira can be marked as resolved once it is backported to 10.2.
        Hide
        Andrew McIntyre added a comment -

        Patch applied to trunk and 10.2. Marking resolved.

        Show
        Andrew McIntyre added a comment - Patch applied to trunk and 10.2. Marking resolved.
        Hide
        Andrew McIntyre added a comment -

        This issue has been resolved for over a year with no further movement. Closing.

        Show
        Andrew McIntyre added a comment - This issue has been resolved for over a year with no further movement. Closing.

          People

          • Assignee:
            Yip Ng
            Reporter:
            Yip Ng
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development