-
Notifications
You must be signed in to change notification settings - Fork 2
#67+#141 Implement Authentication Service and DB versioning with Flyway #145
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
#67+#141 Implement Authentication Service and DB versioning with Flyway #145
Conversation
# Conflicts: # acl-groovy-dsl/src/main/groovy/javasabr/mqtt/service/acl/AclRulesLoader.groovy # application/src/main/java/javasabr/mqtt/broker/application/config/MqttBrokerSpringConfig.java
…nto feature/142-integrate-acl-groovy-based-engine-part-2
…ate-acl-groovy-based-engine-part-2
# Conflicts: # acl-groovy-dsl/src/main/groovy/javasabr/mqtt/acl/groovy/dsl/loader/AclRulesLoader.groovy
…ing-with-flyway # Conflicts: # application/build.gradle # application/src/main/java/javasabr/mqtt/broker/application/config/MqttBrokerSpringConfig.java # settings.gradle
# Conflicts: # application/build.gradle # application/src/main/java/javasabr/mqtt/broker/application/config/MqttBrokerSpringConfig.java # settings.gradle
…lyway # Conflicts: # application/build.gradle # application/src/main/java/javasabr/mqtt/broker/application/config/MqttBrokerSpringConfig.java # settings.gradle
…ing-with-flyway # Conflicts: # application/build.gradle # application/src/main/java/javasabr/mqtt/broker/application/config/MqttBrokerSpringConfig.java # settings.gradle
authentication-api/src/main/java/javasabr/mqtt/auth/api/MqttCredentials.java
Outdated
Show resolved
Hide resolved
...e/src/main/java/javasabr/mqtt/auth/service/config/DatabaseCredentialsSourceSpringConfig.java
Outdated
Show resolved
Hide resolved
...e/src/main/java/javasabr/mqtt/auth/service/config/DatabaseCredentialsSourceSpringConfig.java
Outdated
Show resolved
Hide resolved
| @Bean(initMethod = "migrate") | ||
| Flyway credentialsSourceFlyway( | ||
| DatabaseConnectionProperties databaseCredentialsSourceProperties, | ||
| DatabaseCredentials adminDatabaseCredentials) { |
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.
Where do we define 'adminDatabaseCredentials'?
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.
At the moment it's defined for tests javasabr.mqtt.broker.application.service.DatabaseTestSpringConfig#adminDatabaseCredentials. Let's work on secret storage within a separate ticket
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.
@crazyrokr on your branch I cannot start the broker app:
Factory method 'authenticationService' threw exception with message: Authenticator providers are not configured
...ce/src/test/groovy/javasabr/mqtt/broker/application/service/AuthenticationServiceTest.groovy
Outdated
Show resolved
Hide resolved
|
|
||
| @SuppressWarnings("SqlNoDataSourceInspection") | ||
| private static final String CREDENTIALS_QUERY = """ | ||
| SELECT COUNT(*) > 0 |
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.
why count? not just exists?
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.
Boolean type is not supported by all databases, so the query was rewritten without it
...ntication-service/src/main/java/javasabr/mqtt/auth/service/DefaultAuthenticationService.java
Outdated
Show resolved
Hide resolved
...ice/src/test/groovy/javasabr/mqtt/broker/application/service/IntegrationSpecification.groovy
Show resolved
Hide resolved
...rvice/src/test/groovy/javasabr/mqtt/broker/application/service/DatabaseTestSpringConfig.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| @Bean(initMethod = "start", destroyMethod = "stop") | ||
| public PostgreSQLContainer postgreSQLContainer() { |
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.
You can define in test-support module the spring config like 'PostgresTestContainerSpringConfig' to reuse it later in the app module for example
...rvice/src/test/groovy/javasabr/mqtt/broker/application/service/DatabaseTestSpringConfig.java
Outdated
Show resolved
Hide resolved
authentication-api/src/main/java/javasabr/mqtt/auth/api/file/FileProperties.java
Outdated
Show resolved
Hide resolved
...ntication-service/src/main/java/javasabr/mqtt/auth/service/DefaultAuthenticationService.java
Outdated
Show resolved
Hide resolved
| byte[] authenticationData) { | ||
|
|
||
| public boolean isAnonymous() { | ||
| return StringUtils.isEmpty(username) && ArrayUtils.isEmpty(password); |
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.
What about the case with empty username and not empty password?
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.
JWT authentication provider will handle such case
...e/src/main/java/javasabr/mqtt/auth/service/config/DatabaseCredentialsSourceSpringConfig.java
Outdated
Show resolved
Hide resolved
| @Bean(initMethod = "migrate") | ||
| Flyway credentialsSourceFlyway( | ||
| DatabaseConnectionProperties databaseCredentialsSourceProperties, | ||
| DatabaseCredentials adminDatabaseCredentials) { |
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.
@crazyrokr on your branch I cannot start the broker app:
Factory method 'authenticationService' threw exception with message: Authenticator providers are not configured
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.
Pull request overview
This PR implements a comprehensive authentication service architecture for the MQTT broker, replacing the simple file-based authentication with a modular, extensible system supporting multiple authentication providers and credentials sources. It also introduces Flyway for database schema versioning.
Changes:
- Introduces a modular authentication architecture with 5 new modules (authentication-api, authentication-service, authentication-provider-basic, credentials-source-file, credentials-source-db)
- Implements database-backed credentials storage with Flyway migrations for PostgreSQL
- Replaces legacy authentication classes in core-service with new API-based implementations
Reviewed changes
Copilot reviewed 67 out of 67 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| settings.gradle | Adds 5 new authentication-related modules to the project |
| gradle/libs.versions.toml | Adds dependencies for R2DBC, PostgreSQL, Flyway, Testcontainers, and AssertJ |
| test-support/build.gradle | Adds spring-boot-test and assertj-core for testing support |
| buildSrc/src/main/groovy/configure-java.gradle | Adds -parameters compiler flag for better runtime parameter reflection |
| base/src/main/java/javasabr/mqtt/base/util/* | Adds PropertyAssert and BrokerConfigurationException for configuration validation |
| authentication-api/src/main/java/javasabr/mqtt/auth/api/* | Defines core authentication interfaces, DTOs, and enums for the authentication architecture |
| authentication-service/src/main/java/javasabr/mqtt/auth/service/* | Implements DefaultAuthenticationService with multi-provider support and Spring auto-configuration |
| authentication-provider-basic/src/main/java/javasabr/mqtt/auth/provider/* | Implements basic username/password authentication with ASCII validation |
| credentials-source-file/src/main/java/javasabr/mqtt/auth/credentials/source/* | Implements file-based credentials storage with in-memory caching |
| credentials-source-db/src/main/java/javasabr/mqtt/auth/credentials/source/* | Implements PostgreSQL-backed credentials storage with R2DBC and Flyway |
| authentication-service/src/main/resources/db/migration/* | Adds Flyway migration for user_credentials table schema |
| authentication-service/src/test/* | Adds comprehensive integration tests with Testcontainers for database testing |
| core-service/src/main/java/javasabr/mqtt/service/* | Removes legacy authentication classes (replaced by new authentication-api) |
| core-service/src/main/java/javasabr/mqtt/service/message/handler/impl/ConnectInMqttInMessageHandler.java | Updates to use new MqttCredentials DTO instead of raw username/password parameters |
| core-service/build.gradle | Adds dependency on authentication-api module |
| application/src/main/java/javasabr/mqtt/broker/application/config/MqttBrokerSpringConfig.java | Removes local authentication service beans, imports AuthenticationServiceSpringConfig |
| application/src/main/resources/application.properties | Updates authentication configuration property names |
| application/src/test/* | Updates test configuration and refactors MQTT client creation into factory class |
| application/build.gradle | Adds dependencies on authentication-service and credentials-source modules |
| acl-service/src/main/java/javasabr/mqtt/acl/service/AclEngineBasedAuthorizationService.java | Minor comment grammar fix (unrelated to main feature) |
#67
The main goal of the PR is to prepare a module and class structures covering the most common authentication mechanisms, so it introduces the following entities: