Derby
  1. Derby
  2. DERBY-3146

Adjust length restriction on user identifiers (authorization ids) to same as other identifiers

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.0.2.0, 10.0.2.1, 10.1.1.0, 10.1.2.1, 10.1.3.1, 10.2.1.6, 10.2.2.0, 10.3.1.4, 10.3.2.1, 10.3.3.0, 10.4.1.3, 10.4.2.0, 10.5.1.1, 10.5.2.0, 10.5.3.0, 10.6.1.0, 10.6.2.1, 10.7.1.1, 10.8.1.2, 10.8.2.2
    • Fix Version/s: 10.9.1.0
    • Component/s: JDBC, SQL
    • Labels:
      None
    • Bug behavior facts:
      Deviation from standard, Security

      Description

      While working on roles, I notice that there is a max size of 30 on
      user ids in derby (authorization identifiers), e.g. the check being
      performed in the parser:

      private void checkAuthorizationLength( String authorization)
      :
      checkIdentifierLengthLimit( authorization, Limits.DB2_MAX_USERID_LENGTH);
      :

      where Limits.DB2_MAX_USERID_LENGTH == 30. I have checked, and I don't
      think there are any fundamental reasons why Derby can't lift this DB2
      restriction: Then authorization identifiers would have the same max
      limit as other identifiers: 128 (Limits.MAX_IDENTIFIER_LENGTH).

      Currently, this limit of 30 is enforced for GRANT/REVOKE, i.e. for the
      grantees.

      However, in the CREATE SCHEMA statement, the clause

      AUTHORIZATION <authorization identifier>

      which allows specifying a schema's owner, is not subject to this
      restriction. This is also reflected in the reference documentation for
      system tables:

      SYS.SYSCHEMAS:

      Column Name Type Length Nullability Contents
      -------------------------------------------------------------------
      AUTHORIZATIONID VARCHAR 128 false the authorization
      identifier of the
      owner of the schema

      SYS.SYSTABLEPERMS:

      Column Name Type Length Nullability Contents
      -------------------------------------------------------------------
      GRANTEE VARCHAR 30 False The authorization ID
      of the user to whom
      the privilege is
      granted.

      Furthermore, the limit is enforced in the authorizer code
      (AuthorizationServiceBase#authenticate). It is also reflected in the
      metadata: EmbedDatabaseMetaData#getMaxUserNameLength.

      I think it would be good to harmonize these two different limits for
      authorization identifier and change the limit to 128
      (Limits.MAX_IDENTIFIER_LENGTH).

      1. DERBY-3146.stat
        0.3 kB
        Dag H. Wanvik
      2. DERBY-3146.diff
        3 kB
        Dag H. Wanvik

        Issue Links

          Activity

          Dag H. Wanvik created issue -
          Hide
          Bryan Pendleton added a comment -

          This seems like a good idea to me, thanks for logging it!

          Are there upgrade issues with changing the length of system catalog column?

          Show
          Bryan Pendleton added a comment - This seems like a good idea to me, thanks for logging it! Are there upgrade issues with changing the length of system catalog column?
          Hide
          Dag H. Wanvik added a comment -

          Hi Bryan,

          > Are there upgrade issues with changing the length of system catalog column?

          I don't believe so, because in the catalog code the length restriction is the same
          as for other identifiers:

          • Permissions tables occurences of authorization id:

          (Note that the documentation is slightly misleading here, indicating
          VARCHAR(30), but that is not what is implemented, apparently.)

          ij> select cast(columnname as varchar(10)) as columnname,
          cast(columndatatype as varchar(30)) as columndatatype
          from sys.syscolumns where columnname like 'GRANTOR';

          COLUMNNAME|COLUMNDATATYPE
          -----------------------------------------
          GRANTOR |VARCHAR(128) NOT NULL
          GRANTOR |VARCHAR(128) NOT NULL
          GRANTOR |VARCHAR(128) NOT NULL
          GRANTOR |VARCHAR(128) NOT NULL

          ij> select cast(columnname as varchar(10)) as columnname,
          cast(columndatatype as varchar(30)) as columndatatype
          from sys.syscolumns where columnname like 'GRANTEE';

          COLUMNNAME|COLUMNDATATYPE
          -----------------------------------------
          GRANTEE |VARCHAR(128) NOT NULL
          GRANTEE |VARCHAR(128) NOT NULL
          GRANTEE |VARCHAR(128) NOT NULL
          GRANTEE |VARCHAR(128) NOT NULL

          • SYSSCHEMA occurence of authorization id:

          ij> select cast(columnname as varchar(20)) as columnname,
          cast(columndatatype as varchar(30)) as columndatatype
          from sys.syscolumns where columnname like 'AUTHORIZATIONID';

          COLUMNNAME |COLUMNDATATYPE
          ---------------------------------------------------
          AUTHORIZATIONID |VARCHAR(128) NOT NULL

          Show
          Dag H. Wanvik added a comment - Hi Bryan, > Are there upgrade issues with changing the length of system catalog column? I don't believe so, because in the catalog code the length restriction is the same as for other identifiers: Permissions tables occurences of authorization id: (Note that the documentation is slightly misleading here, indicating VARCHAR(30), but that is not what is implemented, apparently.) ij> select cast(columnname as varchar(10)) as columnname, cast(columndatatype as varchar(30)) as columndatatype from sys.syscolumns where columnname like 'GRANTOR'; COLUMNNAME|COLUMNDATATYPE ----------------------------------------- GRANTOR |VARCHAR(128) NOT NULL GRANTOR |VARCHAR(128) NOT NULL GRANTOR |VARCHAR(128) NOT NULL GRANTOR |VARCHAR(128) NOT NULL ij> select cast(columnname as varchar(10)) as columnname, cast(columndatatype as varchar(30)) as columndatatype from sys.syscolumns where columnname like 'GRANTEE'; COLUMNNAME|COLUMNDATATYPE ----------------------------------------- GRANTEE |VARCHAR(128) NOT NULL GRANTEE |VARCHAR(128) NOT NULL GRANTEE |VARCHAR(128) NOT NULL GRANTEE |VARCHAR(128) NOT NULL SYSSCHEMA occurence of authorization id: ij> select cast(columnname as varchar(20)) as columnname, cast(columndatatype as varchar(30)) as columndatatype from sys.syscolumns where columnname like 'AUTHORIZATIONID'; COLUMNNAME |COLUMNDATATYPE --------------------------------------------------- AUTHORIZATIONID |VARCHAR(128) NOT NULL
          Dag H. Wanvik made changes -
          Field Original Value New Value
          Derby Categories [Security]
          Dag H. Wanvik made changes -
          Component/s Security [ 11411 ]
          Rick Hillegas made changes -
          Link This issue is related to DERBY-5744 [ DERBY-5744 ]
          Dag H. Wanvik made changes -
          Assignee Dag H. Wanvik [ dagw ]
          Dag H. Wanvik made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Hide
          Dag H. Wanvik added a comment -

          Uploading a patch to lift this restriction in the code. I'll file a separate patch for the documentation. Running regressions.

          Show
          Dag H. Wanvik added a comment - Uploading a patch to lift this restriction in the code. I'll file a separate patch for the documentation. Running regressions.
          Dag H. Wanvik made changes -
          Attachment DERBY-3146.diff [ 12526029 ]
          Attachment DERBY-3146.stat [ 12526030 ]
          Hide
          Dag H. Wanvik added a comment -

          Marking release note needed since DataBaseMetaData#getMaxUserNameLength will now return a different number, i.e. 128.

          Show
          Dag H. Wanvik added a comment - Marking release note needed since DataBaseMetaData#getMaxUserNameLength will now return a different number, i.e. 128.
          Dag H. Wanvik made changes -
          Bug behavior facts Security [ 10361 ] Deviation from standard,Security [ 10367,10361 ]
          Issue & fix info Patch Available,Release Note Needed [ 10102,10101 ]
          Hide
          Dag H. Wanvik added a comment -

          Marking "deviation from standard" since the user name is an SQLIdentifier according to the spec.

          Show
          Dag H. Wanvik added a comment - Marking "deviation from standard" since the user name is an SQLIdentifier according to the spec.
          Hide
          Dag H. Wanvik added a comment -

          Regressions passed, please review.

          Show
          Dag H. Wanvik added a comment - Regressions passed, please review.
          Hide
          Kristian Waagan added a comment -

          I had a look at the code changes, and they look good to me. +1

          Based on Bryan's question and Dag's reply, it seems that any upgrade issues have been considered. Also, since we are going from a smaller to a larger value there shouldn't be any problems with existing values.

          Show
          Kristian Waagan added a comment - I had a look at the code changes, and they look good to me. +1 Based on Bryan's question and Dag's reply, it seems that any upgrade issues have been considered. Also, since we are going from a smaller to a larger value there shouldn't be any problems with existing values.
          Hide
          Kim Haase added a comment -

          I'm not sure that a documentation patch will be needed, since we have been documenting the relevant table columns as being VARCHAR(128) since (I think?) 10.5. (If there are any places where we say user names are limited to 30 characters, though, we'd better fix them; I haven't found any.)

          I don't think we have ever documented the return value of DataBaseMetaData#getMaxUserNameLength, so we certainly don't need more than a release note for that.

          Show
          Kim Haase added a comment - I'm not sure that a documentation patch will be needed, since we have been documenting the relevant table columns as being VARCHAR(128) since (I think?) 10.5. (If there are any places where we say user names are limited to 30 characters, though, we'd better fix them; I haven't found any.) I don't think we have ever documented the return value of DataBaseMetaData#getMaxUserNameLength, so we certainly don't need more than a release note for that.
          Hide
          Dag H. Wanvik added a comment - - edited

          Thanks, Kim! I didn't realize we had changed this doc already. Confirmed, I can see it has changed in the 10.5 docs.

          Show
          Dag H. Wanvik added a comment - - edited Thanks, Kim! I didn't realize we had changed this doc already. Confirmed, I can see it has changed in the 10.5 docs.
          Hide
          Dag H. Wanvik added a comment -

          Unticking release note needed, since we are more liberal and the metadata are meant to reflect current limits anyway: the API hasn't changed.

          Show
          Dag H. Wanvik added a comment - Unticking release note needed, since we are more liberal and the metadata are meant to reflect current limits anyway: the API hasn't changed.
          Dag H. Wanvik made changes -
          Issue & fix info Release Note Needed,Patch Available [ 10101,10102 ] Patch Available [ 10102 ]
          Hide
          Dag H. Wanvik added a comment -

          Committed patch at svn 1336775, closing.

          Show
          Dag H. Wanvik added a comment - Committed patch at svn 1336775, closing.
          Dag H. Wanvik made changes -
          Component/s JDBC [ 11407 ]
          Fix Version/s 10.9.0.0 [ 12316344 ]
          Affects Version/s 10.8.2.2 [ 12317968 ]
          Affects Version/s 10.8.1.2 [ 12316362 ]
          Affects Version/s 10.7.1.1 [ 12315564 ]
          Affects Version/s 10.6.2.1 [ 12315343 ]
          Affects Version/s 10.6.1.0 [ 12313727 ]
          Affects Version/s 10.5.3.0 [ 12314117 ]
          Affects Version/s 10.5.2.0 [ 12314116 ]
          Affects Version/s 10.5.1.1 [ 12313771 ]
          Affects Version/s 10.4.2.0 [ 12313345 ]
          Affects Version/s 10.4.1.3 [ 12313111 ]
          Affects Version/s 10.3.3.0 [ 12313142 ]
          Affects Version/s 10.3.2.1 [ 12312876 ]
          Affects Version/s 10.3.1.4 [ 12312590 ]
          Affects Version/s 10.2.2.0 [ 12312027 ]
          Affects Version/s 10.2.1.6 [ 11187 ]
          Affects Version/s 10.1.3.1 [ 12311953 ]
          Affects Version/s 10.1.2.1 [ 12310615 ]
          Affects Version/s 10.1.1.0 [ 10993 ]
          Affects Version/s 10.0.2.1 [ 10991 ]
          Affects Version/s 10.0.2.0 [ 10920 ]
          Issue & fix info Patch Available [ 10102 ]
          Dag H. Wanvik made changes -
          Status In Progress [ 3 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Dag H. Wanvik made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Hide
          Trejkaz added a comment -

          Can we get this back-ported to the 10.8 series?

          Show
          Trejkaz added a comment - Can we get this back-ported to the 10.8 series?
          Hide
          Dag H. Wanvik added a comment -

          We don't usually backport functional changes/improvements to old branches, only bug fixes. If you need these changes, you could always consider building it yourself. I just checked and it merges cleanly to the head of the 10.8 branch (check that out first [1]) as

          svn merge -c 1336775 https://svn.apache.org/repos/asf/db/derby/code/trunk

          [1] svn checkout https://svn.apache.org/repos/asf/db/derby/code/branches/10.8

          Show
          Dag H. Wanvik added a comment - We don't usually backport functional changes/improvements to old branches, only bug fixes. If you need these changes, you could always consider building it yourself. I just checked and it merges cleanly to the head of the 10.8 branch (check that out first [1] ) as svn merge -c 1336775 https://svn.apache.org/repos/asf/db/derby/code/trunk [1] svn checkout https://svn.apache.org/repos/asf/db/derby/code/branches/10.8
          Gavin made changes -
          Workflow jira [ 12415695 ] Default workflow, editable Closed status [ 12799074 ]

            People

            • Assignee:
              Dag H. Wanvik
              Reporter:
              Dag H. Wanvik
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development