Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: SSP 2.0.0, SSP 2.1.0, SSP 2.0.1
    • Fix Version/s: SSP 2.1.0, SSP 2.0.1
    • Component/s: Database
    • Labels:
      None

      Description

      Except for student search by school_id, all person and external_person lookups by username or school_id appear to be case-sensitive in Postgres deployments. This is almost certainly not what a deployer typically wants by default and has already caused a problem with the North Carolina schools. At a minimum, lookups need to compare these fields case-insensitively, but we really should also force these fields to consistent casing in the db in order to properly take advantage of uniqueness constraints. Postgres might let us set up function-based indexes/constraints, though.

        Issue Links

          Activity

          Hide
          Dan McCallum added a comment -

          For this commit https://github.com/Jasig/SSP/commit/8d3dcbe8a3377a78d8acf235e92fcd32965a933c ... why the Hibernate ignoreCase() in PersonDao#fromUsername() instead of forcing the username arg to lower case? Without a corresponding lower(username) index in the person table I think it's going to cause Postgres to skip the index on that column. Since we're now forcing that field to lower case in the Person POJO and existing values to lowercase via Liquibase, I think converting the arg to lowercase should work better?

          Show
          Dan McCallum added a comment - For this commit https://github.com/Jasig/SSP/commit/8d3dcbe8a3377a78d8acf235e92fcd32965a933c ... why the Hibernate ignoreCase() in PersonDao#fromUsername() instead of forcing the username arg to lower case? Without a corresponding lower(username) index in the person table I think it's going to cause Postgres to skip the index on that column. Since we're now forcing that field to lower case in the Person POJO and existing values to lowercase via Liquibase, I think converting the arg to lowercase should work better?
          Hide
          Anthony Arland added a comment -

          Yeah, I changed it to be consistent without thinking much about the consequence. I'll roll back the change

          Show
          Anthony Arland added a comment - Yeah, I changed it to be consistent without thinking much about the consequence. I'll roll back the change
          Hide
          Dan McCallum added a comment -

          Once you've got that done, if you could please merge to rel-2-0-patches so Vicky can grab it, that would be great.

          Show
          Dan McCallum added a comment - Once you've got that done, if you could please merge to rel-2-0-patches so Vicky can grab it, that would be great.
          Hide
          Anthony Arland added a comment -

          merged

          Show
          Anthony Arland added a comment - merged
          Hide
          Dan McCallum added a comment -

          Reopening to add 2.0.1 fix version

          Show
          Dan McCallum added a comment - Reopening to add 2.0.1 fix version

            People

            • Assignee:
              Anthony Arland
              Reporter:
              Dan McCallum
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: