Database assumed to accept integer for boolean columns

[ioigoume]

        $stmt = sprintf(
            "UPDATE %s SET is_revoked = 1 WHERE auth_code_id = :auth_code_id",
            $this->getTableName(),
        );

the SQL from above will not work for PostgreSQL since the boolean value is hardcoded to an int.

[pradtke]

Thanks for catching this.

I'm thinking we should figure out how to run some phpunit tests against postgresql and mysql, and not just sqlite. In our java code we use the https://testcontainers.com/ to launch different dependencies in docker images. https://github.com/shyim/testcontainer seems like an equivalent for php.
As to how to structure the tests I'm not sure. Does it make sense to try running all the *RepositoryTest classes against all 3 DB, or do we create a DBCompatabilityTest class that runs against all 3, and does the actions we are worried about (db migration, create a token and revoke a token)?

[cicnavi]

I was thinking to move away from SSP database implementation and use our own abstracted store. For example, we could implement it using Doctrine DBAL, which supports the following DBs out-of-the-box: DB2, MariaDB, MySQL, Oracle, PostgreSQL, SQL Server, SQLite (https://www.doctrine-project.org/projects/doctrine-dbal/en/4.1/reference/introduction.html).

Already noted it as TODO, but not sure if it will actually go in v6: https://github.com/simplesamlphp/simplesamlphp-module-oidc/blob/72903c122b3f27d749bb11fc696eb3ae59ade4d6/UPGRADE.md?plain=1#L11

[ioigoume]

@cicnavi thank you for the feedback. Would it make sense to push any easy fixes we can, before moving to a more holistic solution? As for the Doctrine DBAL solution +1

[cicnavi]

@cicnavi thank you for the feedback. Would it make sense to push any easy fixes we can, before moving to a more holistic solution? As for the Doctrine DBAL solution +1

Absolutely...