-
Notifications
You must be signed in to change notification settings - Fork 43
fix(webauthn): use Length.LONG32 for byte[] columns to fix MariaDB row size limit #287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
b441cb5
test: add column mapping test for WebAuthnCredential byte[] length (#…
devondragon 6bc18e5
fix(webauthn): use Length.LONG32 for byte[] columns to fix MariaDB ro…
devondragon d0142c7
fix(webauthn): address PR #287 review feedback
devondragon dd213d5
test(schema): add Testcontainers schema validation for MariaDB and Po…
devondragon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
29 changes: 29 additions & 0 deletions
29
...m/digitalsanctuary/spring/user/persistence/model/WebAuthnCredentialColumnMappingTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| package com.digitalsanctuary.spring.user.persistence.model; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import jakarta.persistence.Column; | ||
| import java.lang.reflect.Field; | ||
| import org.hibernate.Length; | ||
| import org.junit.jupiter.api.DisplayName; | ||
| import org.junit.jupiter.params.ParameterizedTest; | ||
| import org.junit.jupiter.params.provider.ValueSource; | ||
|
|
||
| @DisplayName("WebAuthnCredential Column Mapping Tests") | ||
| class WebAuthnCredentialColumnMappingTest { | ||
|
|
||
| @ParameterizedTest | ||
| @ValueSource(strings = {"attestationObject", "attestationClientDataJson", "publicKey"}) | ||
| @DisplayName("should use Length.LONG32 on byte[] fields for cross-database BLOB compatibility") | ||
| void shouldUseLengthLong32OnBlobFields(String fieldName) throws NoSuchFieldException { | ||
| Field field = WebAuthnCredential.class.getDeclaredField(fieldName); | ||
| Column column = field.getAnnotation(Column.class); | ||
| assertThat(column) | ||
| .as("Field '%s' must have @Column annotation", fieldName) | ||
| .isNotNull(); | ||
| assertThat(column.length()) | ||
| .as("Field '%s' @Column length must be Length.LONG32 (%d) to auto-upgrade " | ||
| + "to LONGBLOB on MariaDB/MySQL and remain bytea on PostgreSQL", | ||
| fieldName, Length.LONG32) | ||
| .isEqualTo(Length.LONG32); | ||
| } | ||
| } | ||
85 changes: 85 additions & 0 deletions
85
...ava/com/digitalsanctuary/spring/user/persistence/schema/AbstractSchemaValidationTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| package com.digitalsanctuary.spring.user.persistence.schema; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import com.digitalsanctuary.spring.user.test.app.TestApplication; | ||
| import java.util.List; | ||
| import java.util.Set; | ||
| import org.junit.jupiter.api.DisplayName; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.springframework.beans.factory.annotation.Autowired; | ||
| import org.springframework.boot.test.context.SpringBootTest; | ||
| import org.springframework.jdbc.core.JdbcTemplate; | ||
|
|
||
| /** | ||
| * Abstract base class for database schema validation tests. Subclasses provide a real database via Testcontainers and | ||
| * configure Spring to connect to it. This test verifies that Hibernate can create the full schema without errors on each | ||
| * target database. | ||
| * | ||
| * <p> | ||
| * The test uses {@code ddl-auto: create} (via Spring Boot properties) and then queries | ||
| * {@code INFORMATION_SCHEMA.TABLES} to verify all expected tables were created. This catches silent DDL failures like | ||
| * the one described in GitHub issue #286. | ||
| * </p> | ||
| */ | ||
| @SpringBootTest(classes = TestApplication.class) | ||
| abstract class AbstractSchemaValidationTest { | ||
|
|
||
| /** | ||
| * All tables expected to be created by Hibernate from the entity model. Includes entity tables and join tables. | ||
| */ | ||
| private static final Set<String> EXPECTED_TABLES = Set.of( | ||
| // Entity tables | ||
| "user_account", "role", "privilege", "verification_token", "password_reset_token", | ||
| "password_history_entry", "user_entities", "user_credentials", | ||
| // Join tables | ||
| "users_roles", "roles_privileges"); | ||
|
|
||
| @Autowired | ||
| private JdbcTemplate jdbcTemplate; | ||
|
|
||
| @Test | ||
| @DisplayName("should create all expected tables without errors") | ||
| void shouldCreateAllExpectedTables() { | ||
| List<String> tables = jdbcTemplate.queryForList( | ||
| "SELECT LOWER(table_name) FROM INFORMATION_SCHEMA.TABLES WHERE LOWER(table_schema) = LOWER(?)", | ||
| String.class, getSchemaName()); | ||
|
|
||
| assertThat(tables) | ||
| .as("All entity and join tables should be created by Hibernate on %s", getDatabaseName()) | ||
| .containsAll(EXPECTED_TABLES); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("should create WebAuthn byte[] columns as BLOB-compatible types (not inline VARBINARY)") | ||
| void shouldCreateWebAuthnBlobColumns() { | ||
| List<String> blobColumns = List.of("public_key", "attestation_object", "attestation_client_data_json"); | ||
|
|
||
| for (String column : blobColumns) { | ||
| String dataType = jdbcTemplate.queryForObject( | ||
| "SELECT LOWER(data_type) FROM INFORMATION_SCHEMA.COLUMNS " | ||
| + "WHERE LOWER(table_schema) = LOWER(?) AND LOWER(table_name) = 'user_credentials' " | ||
| + "AND LOWER(column_name) = ?", | ||
| String.class, getSchemaName(), column); | ||
|
|
||
| assertThat(dataType) | ||
| .as("Column '%s' on %s should be a BLOB-compatible type, not VARBINARY", column, getDatabaseName()) | ||
| .isIn(getAllowedBlobTypes()); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Returns the human-readable database name for assertion messages. | ||
| */ | ||
| protected abstract String getDatabaseName(); | ||
|
|
||
| /** | ||
| * Returns the schema name used in INFORMATION_SCHEMA queries. MariaDB/MySQL uses the database name as schema; | ||
| * PostgreSQL uses 'public' by default. | ||
| */ | ||
| protected abstract String getSchemaName(); | ||
|
|
||
| /** | ||
| * Returns the set of column data types considered acceptable for BLOB columns on this database. | ||
| */ | ||
| protected abstract Set<String> getAllowedBlobTypes(); | ||
| } |
51 changes: 51 additions & 0 deletions
51
...java/com/digitalsanctuary/spring/user/persistence/schema/MariaDBSchemaValidationTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| package com.digitalsanctuary.spring.user.persistence.schema; | ||
|
|
||
| import java.util.Set; | ||
| import org.junit.jupiter.api.DisplayName; | ||
| import org.springframework.test.context.DynamicPropertyRegistry; | ||
| import org.springframework.test.context.DynamicPropertySource; | ||
| import org.testcontainers.containers.MariaDBContainer; | ||
| import org.testcontainers.junit.jupiter.Container; | ||
| import org.testcontainers.junit.jupiter.Testcontainers; | ||
|
|
||
| /** | ||
| * Validates that Hibernate can create the full schema on MariaDB without errors. This specifically catches the InnoDB | ||
| * row-size limit issue described in GitHub issue #286 where VARBINARY(65535) columns caused silent table creation | ||
| * failure. | ||
| */ | ||
| @Testcontainers | ||
| @DisplayName("MariaDB Schema Validation Tests") | ||
| class MariaDBSchemaValidationTest extends AbstractSchemaValidationTest { | ||
|
|
||
| @Container | ||
| static final MariaDBContainer<?> MARIADB = new MariaDBContainer<>("mariadb:11.4") | ||
| .withDatabaseName("testdb") | ||
| .withUsername("test") | ||
| .withPassword("test"); | ||
|
|
||
| @DynamicPropertySource | ||
| static void configureProperties(DynamicPropertyRegistry registry) { | ||
| registry.add("spring.datasource.url", MARIADB::getJdbcUrl); | ||
| registry.add("spring.datasource.username", MARIADB::getUsername); | ||
| registry.add("spring.datasource.password", MARIADB::getPassword); | ||
| registry.add("spring.datasource.driver-class-name", () -> "org.mariadb.jdbc.Driver"); | ||
| registry.add("spring.jpa.hibernate.ddl-auto", () -> "create"); | ||
| registry.add("spring.jpa.database-platform", () -> "org.hibernate.dialect.MariaDBDialect"); | ||
| registry.add("spring.jpa.properties.hibernate.dialect", () -> "org.hibernate.dialect.MariaDBDialect"); | ||
| } | ||
|
|
||
| @Override | ||
| protected String getDatabaseName() { | ||
| return "MariaDB"; | ||
| } | ||
|
|
||
| @Override | ||
| protected String getSchemaName() { | ||
| return "testdb"; | ||
| } | ||
|
|
||
| @Override | ||
| protected Set<String> getAllowedBlobTypes() { | ||
| return Set.of("longblob", "mediumblob", "blob"); | ||
| } | ||
| } |
51 changes: 51 additions & 0 deletions
51
...a/com/digitalsanctuary/spring/user/persistence/schema/PostgreSQLSchemaValidationTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| package com.digitalsanctuary.spring.user.persistence.schema; | ||
|
|
||
| import java.util.Set; | ||
| import org.junit.jupiter.api.DisplayName; | ||
| import org.springframework.test.context.DynamicPropertyRegistry; | ||
| import org.springframework.test.context.DynamicPropertySource; | ||
| import org.testcontainers.containers.PostgreSQLContainer; | ||
| import org.testcontainers.junit.jupiter.Container; | ||
| import org.testcontainers.junit.jupiter.Testcontainers; | ||
|
|
||
| /** | ||
| * Validates that Hibernate can create the full schema on PostgreSQL without errors. Ensures the byte[] columns map to | ||
| * {@code bytea} (not {@code oid}), which would happen if {@code @Lob} were used instead of | ||
| * {@code length = Length.LONG32}. | ||
| */ | ||
| @Testcontainers | ||
| @DisplayName("PostgreSQL Schema Validation Tests") | ||
| class PostgreSQLSchemaValidationTest extends AbstractSchemaValidationTest { | ||
|
|
||
| @Container | ||
| static final PostgreSQLContainer<?> POSTGRES = new PostgreSQLContainer<>("postgres:17") | ||
| .withDatabaseName("testdb") | ||
| .withUsername("test") | ||
| .withPassword("test"); | ||
|
|
||
| @DynamicPropertySource | ||
| static void configureProperties(DynamicPropertyRegistry registry) { | ||
| registry.add("spring.datasource.url", POSTGRES::getJdbcUrl); | ||
| registry.add("spring.datasource.username", POSTGRES::getUsername); | ||
| registry.add("spring.datasource.password", POSTGRES::getPassword); | ||
| registry.add("spring.datasource.driver-class-name", () -> "org.postgresql.Driver"); | ||
| registry.add("spring.jpa.hibernate.ddl-auto", () -> "create"); | ||
| registry.add("spring.jpa.database-platform", () -> "org.hibernate.dialect.PostgreSQLDialect"); | ||
| registry.add("spring.jpa.properties.hibernate.dialect", () -> "org.hibernate.dialect.PostgreSQLDialect"); | ||
| } | ||
|
|
||
| @Override | ||
| protected String getDatabaseName() { | ||
| return "PostgreSQL"; | ||
| } | ||
|
|
||
| @Override | ||
| protected String getSchemaName() { | ||
| return "public"; | ||
| } | ||
|
|
||
| @Override | ||
| protected Set<String> getAllowedBlobTypes() { | ||
| return Set.of("bytea"); | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reflection-based assertion verifies the annotation value but not the actual DDL/type Hibernate generates for MariaDB/MySQL (the regression reported in #286). If feasible, add a test that runs Hibernate schema generation with the MariaDB dialect (or a lightweight Testcontainers MariaDB) and asserts these fields map to a BLOB/LONGBLOB type rather than VARBINARY, so the table-creation failure can’t silently return.