You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-commits@db.apache.org by da...@apache.org on 2010/09/21 21:40:09 UTC

svn commit: r999570 - in /db/derby/code/trunk/java: engine/org/apache/derby/impl/jdbc/EmbedResultSet.java engine/org/apache/derby/impl/sql/execute/BaseActivation.java testing/org/apache/derbyTesting/functionTests/tests/lang/RoutinesDefinersRightsTest.java

Author: dag
Date: Tue Sep 21 19:40:08 2010
New Revision: 999570

URL: http://svn.apache.org/viewvc?rev=999570&view=rev
Log:
DERBY-4551 Allow database user to execute stored procedures with same permissions as database owner and/or routine definer

Follow-up patch derby-4551-followup-1b (plus some small hygiene adjustments).

The problem is that the substatement executed as part of
ResultSet.{insertRow, updateRow,deleteRow} pushes a new statement
context. This statement context is consulted when constructing the
activation for the substatement, to see if the activation shall have a
parent activation (which is used to get the correct SQL session
context),
cf. GenericLanguageConnectionContext#getCurrentSQLSessionContext.

However, the newly pushed statement context was missing its parent's
activation, so the substatement instead get the top level session
context, whose current user is not the DEFINER bur rather the session
user.  cf BaseActivation#setupSQLSessionContextForChildren, hence the
authorization error.

The patch makes sure the nested statement context initially gets the
(new) parent context set. 


Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/EmbedResultSet.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/BaseActivation.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/RoutinesDefinersRightsTest.java

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/EmbedResultSet.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/EmbedResultSet.java?rev=999570&r1=999569&r2=999570&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/EmbedResultSet.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/EmbedResultSet.java Tue Sep 21 19:40:08 2010
@@ -3611,6 +3611,13 @@ public abstract class EmbedResultSet ext
                 valuesSQL.append(") ");
                 insertSQL.append(valuesSQL);
 
+                StatementContext currSC = lcc.getStatementContext();
+                Activation parentAct = null;
+
+                if (currSC != null) {
+                    parentAct = currSC.getActivation();
+                }
+
                 // Context used for preparing, don't set any timeout (use 0)
                 statementContext = lcc.pushStatementContext(
                         isAtomic, 
@@ -3619,6 +3626,14 @@ public abstract class EmbedResultSet ext
                         null, 
                         false, 
                         0L);
+
+                // A priori, the new statement context inherits the activation
+                // of the existing statementContext, so that that activation
+                // ends up as parent of the new activation 'act' created below,
+                // which will be the activation of the pushed statement
+                // context.
+                statementContext.setActivation(parentAct);
+
                 org.apache.derby.iapi.sql.PreparedStatement ps = 
                         lcc.prepareInternalStatement(insertSQL.toString());
                 Activation act = ps.getActivation(lcc, false);
@@ -3700,8 +3715,22 @@ public abstract class EmbedResultSet ext
             updateWhereCurrentOfSQL.append(" WHERE CURRENT OF " + 
                     IdUtil.normalToDelimited(getCursorName()));
 
+            StatementContext currSC = lcc.getStatementContext();
+            Activation parentAct = null;
+
+            if (currSC != null) {
+                parentAct = currSC.getActivation();
+            }
+
             // Context used for preparing, don't set any timeout (use 0)
             statementContext = lcc.pushStatementContext(isAtomic, false, updateWhereCurrentOfSQL.toString(), null, false, 0L);
+
+            // A priori, the new statement context inherits the activation of
+            // the existing statementContext, so that that activation ends up
+            // as parent of the new activation 'act' created below, which will
+            // be the activation of the pushed statement context.
+            statementContext.setActivation(parentAct);
+
             org.apache.derby.iapi.sql.PreparedStatement ps = lcc.prepareInternalStatement(updateWhereCurrentOfSQL.toString());
             Activation act = ps.getActivation(lcc, false);
 
@@ -3768,9 +3797,24 @@ public abstract class EmbedResultSet ext
                 //using quotes around the cursor name to preserve case sensitivity
                 deleteWhereCurrentOfSQL.append(" WHERE CURRENT OF " + 
                         IdUtil.normalToDelimited(getCursorName()));
-                
+
+                StatementContext currSC = lcc.getStatementContext();
+                Activation parentAct = null;
+
+                if (currSC != null) {
+                    parentAct = currSC.getActivation();
+                }
+
                 // Context used for preparing, don't set any timeout (use 0)
                 statementContext = lcc.pushStatementContext(isAtomic, false, deleteWhereCurrentOfSQL.toString(), null, false, 0L);
+
+                // A priori, the new statement context inherits the activation
+                // of the existing statementContext, so that that activation
+                // ends up as parent of the new activation 'act' created below,
+                // which will be the activation of the pushed statement
+                // context.
+                statementContext.setActivation(parentAct);
+
                 org.apache.derby.iapi.sql.PreparedStatement ps = lcc.prepareInternalStatement(deleteWhereCurrentOfSQL.toString());
                 // Get activation, so that we can get the warning from it
                 Activation act = ps.getActivation(lcc, false);

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/BaseActivation.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/BaseActivation.java?rev=999570&r1=999569&r2=999570&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/BaseActivation.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/BaseActivation.java Tue Sep 21 19:40:08 2010
@@ -1410,6 +1410,9 @@ public abstract class BaseActivation imp
 	 * Return the current SQL session context for all immediately
 	 * nested connections stemming from the call or function
 	 * invocation of the statement corresponding to this activation.
+     * <p/>
+     * Substatements (e.g. used in rs.updateRow), inherit the SQL session
+     * context via its parent activation.
 	 * @see org.apache.derby.iapi.sql.Activation#getSQLSessionContextForChildren
 	 */
 	public SQLSessionContext getSQLSessionContextForChildren() {

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/RoutinesDefinersRightsTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/RoutinesDefinersRightsTest.java?rev=999570&r1=999569&r2=999570&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/RoutinesDefinersRightsTest.java (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/RoutinesDefinersRightsTest.java Tue Sep 21 19:40:08 2010
@@ -88,8 +88,8 @@ public class RoutinesDefinersRightsTest 
             // DriverManager is not supported with JSR169.
             suite.addTest(makeSuite());
 
-            // suite.addTest(
-            //     TestConfiguration.clientServerDecorator(makeSuite()));
+            suite.addTest(
+                TestConfiguration.clientServerDecorator(makeSuite()));
         }
 
 
@@ -140,10 +140,23 @@ public class RoutinesDefinersRightsTest 
                          "reads sql data called on null input");
 
                     s.execute
+                        ("create procedure s1.updateWage() " +
+                         "language java parameter style java " +
+                         "modifies sql data " +
+                         "external name " +
+                         "'org.apache.derbyTesting.functionTests.tests.lang." +
+                         "RoutinesDefinersRightsTest.updateWage' " +
+                         "EXTERNAL SECURITY DEFINER ");
+
+                    s.execute
                         ("grant execute on function s1.lookupWageFootSoldier " +
                          "   to phb");
 
                     s.execute
+                        ("grant execute on procedure s1.updateWage " +
+                         "   to phb");
+
+                    s.execute
                         ("create function s1.lookupWageFootSoldierI(int) " +
                          "returns double " +
                          "language java parameter style java external name " +
@@ -272,6 +285,10 @@ public class RoutinesDefinersRightsTest 
         stm = c.createStatement();
         rs = stm.executeQuery("values s1.lookupWageFootSoldier(1002)");
         JDBC.assertSingleValueResultSet(rs, "100.0");
+
+        // Try as PHB to update, delete and insert on a result set
+        stm.executeUpdate("call s1.updateWage()");
+
         stm.close();
         c.close();
 
@@ -430,6 +447,63 @@ public class RoutinesDefinersRightsTest 
         }
     }
 
+
+    /**
+     * Test that PHB can actually update using {@code ResultSet.insertRow},
+     * {@code ResultSet.updateRow} and {@code ResultSet.deleteRow}.
+     * <p/>
+     * Aside: This test is somewhat artificial here, since the middle manager
+     * would not be allowed to do this, presumably; just added here to test
+     * this functionality (which was initially broken by the first patch for
+     * DERBY-4551).
+     * <p/>
+     * The problem was that the nested statement contexts used for SQL
+     * substatements generated for these ResultSet operations were not
+     * correctly set up, so the effective user id would not be the DEFINER
+     * (DBO), and authorization would fail. Cf DERBY-4551 and DERBY-3327
+     * for more detail.
+     */
+    public static void updateWage()
+            throws SQLException
+    {
+        Connection c = null;
+
+        c = DriverManager.getConnection("jdbc:default:connection");
+        Statement cStmt = c.createStatement(
+            ResultSet.TYPE_SCROLL_INSENSITIVE,
+            ResultSet.CONCUR_UPDATABLE);
+
+        // Try nested statements by inserting, updating and deleting a bogus
+        // row
+        ResultSet rs = cStmt.executeQuery(
+            "select * from s1.wages");
+        assertTrue(rs.isBeforeFirst());
+        rs.moveToInsertRow();
+        rs.updateInt("EMPLOYEEID", 666);
+        rs.updateInt("CATEGORY", 667);
+        rs.updateDouble("SALARY", 666.0);
+        rs.updateString("NAME", "N.N.");
+        rs.insertRow();
+        rs.close();
+
+        rs = cStmt.executeQuery(
+            "select * from s1.wages where name = 'N.N.'");
+        rs.next();
+        rs.updateDouble("SALARY", 666.1);
+        rs.updateRow();
+        rs.close();
+
+        rs = cStmt.executeQuery(
+            "select * from s1.wages where name = 'N.N.'");
+        rs.next();
+        rs.deleteRow();
+        rs.close();
+
+        cStmt.close();
+        c.close();
+    }
+
+
     public static ResultSet selectFootSoldiers()
             throws SQLException
     {