GeoServer
  1. GeoServer
  2. GEOS-5041

Broken File/URL conversions in security

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.2-beta2, 2.2.x
    • Component/s: Security
    • Labels:
      None
    • Environment:
      Maven 3
    • Number of attachments :
      1

      Description

      The new security password provider is broken in any data directory that contains special characters such as spaces, causing the build to fail:

      Tests in error: 
        testEncryption(org.geoserver.security.password.URLMasterPasswordProviderTest): /home/car605/geoserver/src%20with%20spaces/geoserver-trunk/src/main/target/passwd1191987093163736111tmp (No such file or directory)
        testMasterPasswordChange(org.geoserver.security.password.MasterPasswordChangeTest): java.io.FileNotFoundException: /home/car605/geoserver/src%20with%20spaces/geoserver-trunk/src/main/./target/mock2049237527189537102data/security/mpw1.properties (No such file or directory)
      

      Deployments will also be affected. The underlying problem is code like:

      File f = new File(url.getFile());
      

      in URLMasterpasswordProvider that does not use DataUtilities as required by the project conventions:

      http://docs.geotools.org/latest/developer/conventions/code/url.html

      1. GEOS-5041.patch
        5 kB
        Ben Caradoc-Davies

        Activity

        Hide
        Ben Caradoc-Davies added a comment -

        Justin, there was a time when you configured Hudson to build in a path with spaces. I think that was a great way of catching problems like this.

        Show
        Ben Caradoc-Davies added a comment - Justin, there was a time when you configured Hudson to build in a path with spaces. I think that was a great way of catching problems like this.
        Ben Caradoc-Davies made changes -
        Field Original Value New Value
        Assignee Andrea Aime [ aaime ] Justin Deoliveira [ jdeolive ]
        Hide
        Ben Caradoc-Davies added a comment -

        Justin, please find attached a patch that fixes these failures.

        Note that I may have been a bit aggressive in my use of getCanonicalFile in MasterPasswordChangeTest (the test passes both with and without them); this may reduce your test coverage of relative file URLs for password provider configuration. I am not quite sure of your intent in these tests. By comparison, in URLMasterPasswordProviderTest you explicitly canonicalise the file path before using it. Different code paths will be used for relative and absolute paths, with and without a protocol.

        Show
        Ben Caradoc-Davies added a comment - Justin, please find attached a patch that fixes these failures. Note that I may have been a bit aggressive in my use of getCanonicalFile in MasterPasswordChangeTest (the test passes both with and without them); this may reduce your test coverage of relative file URLs for password provider configuration. I am not quite sure of your intent in these tests. By comparison, in URLMasterPasswordProviderTest you explicitly canonicalise the file path before using it. Different code paths will be used for relative and absolute paths, with and without a protocol.
        Ben Caradoc-Davies made changes -
        Attachment GEOS-5041.patch [ 59513 ]
        Hide
        Ben Caradoc-Davies added a comment -

        Even with this patch I also see multiple test failures in jdbc-sec. Might need another Jira issue:

        Results :

        Failed tests:
        testConfiguration(org.geoserver.security.jdbc.H2UserDetailsServiceTest): org.h2.jdbc.JdbcSQLException: Table USERS already exists; SQL statement:
        create table users(name varchar(128) not null,password varchar(254), enabled char(1) not null, primary key(name)) [42101-119]

        Tests in error:
        testWrapRoleService(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): org.h2.jdbc.JdbcSQLException: Unique index or primary key violation: PRIMARY_KEY_7 ON PUBLIC.GROUPS(NAME); SQL statement:
        insert into groups(name ,enabled) values (?,?) [23001-119]
        testWrapUserGroupService(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): User/group service gaugs already exists
        testHideAdminRole(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): User/group service gaugs already exists
        testHideGroups(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): User/group service gaugs already exists
        testRoleServiceReadOnly(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): User/group service gaugs already exists
        testCreateNewUser(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): User/group service gaugs already exists
        testAssignUserToGroup(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): User/group service gaugs already exists
        testRemoveUserInGroup(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): User/group service gaugs already exists
        testRemoveUserNotInGroup(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): User/group service gaugs already exists
        testRoleDatabaseSetup(org.geoserver.security.jdbc.DB2RoleServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity
        testRemove(org.geoserver.security.jdbc.DB2RoleServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity
        testInsert(org.geoserver.security.jdbc.DB2RoleServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity
        testModify(org.geoserver.security.jdbc.DB2RoleServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity
        testIsModified(org.geoserver.security.jdbc.DB2RoleServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity
        testUserGroupDatabaseSetup(org.geoserver.security.jdbc.PostGisUserGroupServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity
        testRemove(org.geoserver.security.jdbc.PostGisUserGroupServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity
        testInsert(org.geoserver.security.jdbc.PostGisUserGroupServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity
        testModify(org.geoserver.security.jdbc.PostGisUserGroupServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity
        testIsModified(org.geoserver.security.jdbc.PostGisUserGroupServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity
        testEmptyPassword(org.geoserver.security.jdbc.PostGisUserGroupServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity

        Show
        Ben Caradoc-Davies added a comment - Even with this patch I also see multiple test failures in jdbc-sec. Might need another Jira issue: Results : Failed tests: testConfiguration(org.geoserver.security.jdbc.H2UserDetailsServiceTest): org.h2.jdbc.JdbcSQLException: Table USERS already exists; SQL statement: create table users(name varchar(128) not null,password varchar(254), enabled char(1) not null, primary key(name)) [42101-119] Tests in error: testWrapRoleService(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): org.h2.jdbc.JdbcSQLException: Unique index or primary key violation: PRIMARY_KEY_7 ON PUBLIC.GROUPS(NAME); SQL statement: insert into groups(name ,enabled) values (?,?) [23001-119] testWrapUserGroupService(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): User/group service gaugs already exists testHideAdminRole(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): User/group service gaugs already exists testHideGroups(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): User/group service gaugs already exists testRoleServiceReadOnly(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): User/group service gaugs already exists testCreateNewUser(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): User/group service gaugs already exists testAssignUserToGroup(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): User/group service gaugs already exists testRemoveUserInGroup(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): User/group service gaugs already exists testRemoveUserNotInGroup(org.geoserver.security.jdbc.JDBCGroupAdminServiceTest): User/group service gaugs already exists testRoleDatabaseSetup(org.geoserver.security.jdbc.DB2RoleServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity testRemove(org.geoserver.security.jdbc.DB2RoleServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity testInsert(org.geoserver.security.jdbc.DB2RoleServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity testModify(org.geoserver.security.jdbc.DB2RoleServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity testIsModified(org.geoserver.security.jdbc.DB2RoleServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity testUserGroupDatabaseSetup(org.geoserver.security.jdbc.PostGisUserGroupServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity testRemove(org.geoserver.security.jdbc.PostGisUserGroupServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity testInsert(org.geoserver.security.jdbc.PostGisUserGroupServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity testModify(org.geoserver.security.jdbc.PostGisUserGroupServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity testIsModified(org.geoserver.security.jdbc.PostGisUserGroupServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity testEmptyPassword(org.geoserver.security.jdbc.PostGisUserGroupServiceTest): org.geoserver.data.test.LiveData cannot be cast to org.geoserver.security.jdbc.LiveDbmsDataSecurity
        Ben Caradoc-Davies made changes -
        Description Th new security password provider is broken in any data directory that contains special characters such as spaces, causing the build to fail:

        {noformat}
        Tests in error:
          testEncryption(org.geoserver.security.password.URLMasterPasswordProviderTest): /home/car605/geoserver/src%20with%20spaces/geoserver-trunk/src/main/target/passwd1191987093163736111tmp (No such file or directory)
          testMasterPasswordChange(org.geoserver.security.password.MasterPasswordChangeTest): java.io.FileNotFoundException: /home/car605/geoserver/src%20with%20spaces/geoserver-trunk/src/main/./target/mock2049237527189537102data/security/mpw1.properties (No such file or directory)
        {noformat}

        Deployments will also be affected. The underlying problem is code like:

        {noformat}
        File f = new File(url.getFile());
        {noformat}

        in URLMasterpasswordProvider that does not use DataUtilities as required by the project conventions:

        http://docs.geotools.org/latest/developer/conventions/code/url.html
        The new security password provider is broken in any data directory that contains special characters such as spaces, causing the build to fail:

        {noformat}
        Tests in error:
          testEncryption(org.geoserver.security.password.URLMasterPasswordProviderTest): /home/car605/geoserver/src%20with%20spaces/geoserver-trunk/src/main/target/passwd1191987093163736111tmp (No such file or directory)
          testMasterPasswordChange(org.geoserver.security.password.MasterPasswordChangeTest): java.io.FileNotFoundException: /home/car605/geoserver/src%20with%20spaces/geoserver-trunk/src/main/./target/mock2049237527189537102data/security/mpw1.properties (No such file or directory)
        {noformat}

        Deployments will also be affected. The underlying problem is code like:

        {noformat}
        File f = new File(url.getFile());
        {noformat}

        in URLMasterpasswordProvider that does not use DataUtilities as required by the project conventions:

        http://docs.geotools.org/latest/developer/conventions/code/url.html
        Hide
        Justin Deoliveira added a comment -

        Looks good to me Ben, commit these fixes as you please. Also feel free to just commit the fixes in the jdbc security module. As for the master password change test i do believe the intension is to test with a relative fiel url... so if we can maintain that it would be ideal.

        Show
        Justin Deoliveira added a comment - Looks good to me Ben, commit these fixes as you please. Also feel free to just commit the fixes in the jdbc security module. As for the master password change test i do believe the intension is to test with a relative fiel url... so if we can maintain that it would be ideal.
        Hide
        Ben Caradoc-Davies added a comment -

        OK, will do. Looks like the jdbc-sec failures are in Maven3 only (32 and 64 bit). I am running tests with Maven 2 to confirm they all pass and then will commit the patch (removing my added getCanonicalFiles in MasterPasswordChangeTest). I will raise the Maven 3 failures as a separate issue.

        Show
        Ben Caradoc-Davies added a comment - OK, will do. Looks like the jdbc-sec failures are in Maven3 only (32 and 64 bit). I am running tests with Maven 2 to confirm they all pass and then will commit the patch (removing my added getCanonicalFiles in MasterPasswordChangeTest). I will raise the Maven 3 failures as a separate issue.
        Hide
        Ben Caradoc-Davies added a comment -

        Committed in r16912 on trunk (with the two added getCanonicalFiles in MasterPasswordChangeTest omitted as requested).

        Show
        Ben Caradoc-Davies added a comment - Committed in r16912 on trunk (with the two added getCanonicalFiles in MasterPasswordChangeTest omitted as requested).
        Hide
        Ben Caradoc-Davies added a comment -

        Fixed.

        Show
        Ben Caradoc-Davies added a comment - Fixed.
        Ben Caradoc-Davies made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Ben Caradoc-Davies added a comment -

        The sec-jdbc failures are GEOS-5043.

        Show
        Ben Caradoc-Davies added a comment - The sec-jdbc failures are GEOS-5043 .
        Ben Caradoc-Davies made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Ben Caradoc-Davies made changes -
        Resolution Fixed [ 1 ]
        Status Closed [ 6 ] Reopened [ 4 ]
        Assignee Justin Deoliveira [ jdeolive ] Ben Caradoc-Davies [ bencaradocdavies ]
        Ben Caradoc-Davies made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Fix Version/s 2.2.x [ 17106 ]
        Fix Version/s 2.2-beta2 [ 18437 ]
        Resolution Fixed [ 1 ]
        Hide
        Andrea Aime added a comment -

        Switching all issues that have been in "resolved" state for more than one month without further comments to "closed" status

        Show
        Andrea Aime added a comment - Switching all issues that have been in "resolved" state for more than one month without further comments to "closed" status
        Andrea Aime made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Ben Caradoc-Davies
            Reporter:
            Ben Caradoc-Davies
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: