refactor: centralize authentication with TokenManager by JanZachmann · Pull Request #61 · omnect/omnect-ui

JanZachmann · 2025-10-26T21:20:18Z

Summary

This PR centralizes authentication infrastructure and improves testing coverage with a simplified TokenManager API and comprehensive integration tests.

Key Changes

🔐 Authentication Refactoring

New: `src/auth/token.rs` (109 lines)

Simplified API: Single token_manager() function returns global singleton
Minimal surface: One constructor TokenManager::new(secret) with 1 parameter
Private internals: Constants (TOKEN_SUBJECT, TOKEN_EXPIRE_HOURS) and struct encapsulated
Global singleton: Uses OnceLock pattern for thread-safe initialization
Methods:
- create_token() - Generate new JWT with 24h expiration
- verify_token(token) - Validate JWT and return subject
Comprehensive tests: 3 unit tests covering token lifecycle

Updated: `src/auth/mod.rs`

Exports only token_manager() function
TokenManager struct kept private

🧹 Removed Duplication

`src/middleware.rs`

❌ Removed local token_manager() duplicate
❌ Removed unnecessary verify_token() wrapper
✅ Uses auth::token_manager() directly
✅ Tests updated to call token_manager().verify_token() directly
✅ Improved error handling and logging

`src/api.rs`

❌ Removed local token_manager() duplicate
✅ Uses auth::token_manager() directly
✅ Consistent token creation and validation

🧪 Testing Improvements

New: `tests/validate_portal_token.rs` (98 lines)

Integration tests for Keycloak token validation covering:

✅ Fleet Administrator with valid tenant
✅ Fleet Administrator with invalid tenant
✅ Fleet Operator with valid fleet
✅ Fleet Operator with invalid fleet
✅ Fleet Observer (insufficient permissions)

Uses mockall for comprehensive test coverage without external dependencies.

New: `tests/http_client.rs` (178 lines)

Integration tests for Unix socket HTTP client:

✅ Success scenarios with valid socket paths
✅ POST request handling
✅ Multiple sequential requests
✅ Error handling for non-existent paths
✅ Error handling for invalid URIs

Benefits

✅ Minimal API surface - One function to get TokenManager
✅ No duplication - Single global token_manager() implementation
✅ No wrappers - Direct access to TokenManager methods
✅ Better encapsulation - Constants and struct are private
✅ Simplified constructor - 1 parameter instead of 3
✅ Code reduction - Net -31 lines while adding features
✅ Better testing - Comprehensive integration test coverage
✅ Type safety - Trait-based design for better compile-time checks

Testing

All tests passing: `cargo test --features mock`

This commit introduces HttpClientFactory to centralize HTTP client creation, eliminating code duplication and ensuring consistent configuration. New Module: http_client.rs - HttpClientFactory with three client types: - https_client(): Cached HTTPS client for external APIs - unix_socket_client(): For local Unix socket communication - workload_client(): For IoT Edge workload API Testing: - Unit tests: 8 tests covering all three client factory methods - test_https_client_is_cached - test_https_client_returns_valid_client - test_https_client_multiple_calls_same_instance - test_workload_client_parses_uri - test_workload_client_rejects_invalid_scheme - test_unix_socket_client_creates_client - test_unix_socket_client_with_relative_path - test_unix_socket_client_with_empty_path - Integration tests: 14 tests in tests/http_client.rs - 4 HTTPS tests (marked #[ignore] for reliability) - Uses httpbin.org endpoints - 10-second timeouts prevent hanging - Run with --ignored flag when network available - 4 Unix socket tests (always run, use mock servers) - 4 Workload client tests (always run, use mock IoT Edge API) - 2 error handling tests (HTTPS connection failures) Test Reliability: External network tests are ignored by default to prevent CI failures from network issues. Local tests (Unix socket, workload client) always run and provide core functionality validation. - Test configuration: Added .cargo/config.toml - Set RUST_TEST_THREADS=1 to prevent rate-limiting issues - Ensures reliable test execution for external API tests - Fixed dead_code warning for workload_client when using mock feature by adding #[cfg_attr(feature = "mock", allow(dead_code))] Benefits: - Eliminates 3 separate OnceLock instances across modules - Single source of truth for HTTP client configuration - Shared connection pools improve performance - Easier to add timeout/retry configuration in future - Better testability with centralized mocking point - Comprehensive test coverage with reliable CI execution Changes by Module: - keycloak_client.rs: Use HttpClientFactory::https_client() - Removed local OnceLock<Client> - 4 lines removed, cleaner code - omnect_device_service_client.rs: Use HttpClientFactory::unix_socket_client() - Simplified client creation - 3 lines removed - certificate.rs: Use HttpClientFactory::workload_client() - Removed manual URI parsing - 8 lines removed, moved to factory Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

This commit addresses all review findings from PR omnect#60 and includes additional improvements to code quality and test coverage. Changes: - Move HTTPS client from global state to KeycloakProvider * Remove global HTTPS_CLIENT static variable * Add client field to KeycloakProvider struct * Move realm_public_key() into KeycloakProvider as instance method - Refactor workload_client() to use unix_socket_client() * Remove duplicate socket client creation logic * Delegate to unix_socket_client() after URI parsing - Improve test quality and clarity * Remove redundant HTTPS client tests (no longer testing our code) * Remove redundant URI validation tests (covered by unit tests) * Remove tests that verify reqwest behavior rather than our code * Add clarifying comments to tests about socket existence * Fix error message assertion to use exact match - Replace sleep() with oneshot channels in integration tests * Deterministic synchronization instead of arbitrary timeouts * 40x faster test execution (0.41s → 0.01s) * Eliminates potential race conditions Test suite improvements: - Reduced from 14 to 4 integration tests - All remaining tests are meaningful and test actual functionality - Clear separation between unit tests and integration tests - Zero clippy warnings, all tests passing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

This commit introduces a centralized, simplified TokenManager for handling JWT token creation and verification with minimal API surface. New Module: auth/token.rs - TokenManager: Centralized token management with hardcoded constants - create_token(): Generate JWT tokens - verify_token(): Validate tokens - Constants: TOKEN_SUBJECT ("omnect-ui"), TOKEN_EXPIRE_HOURS (2) - token_manager(): Global singleton function - Single source of truth for TokenManager instance - Automatically configured with centrifugo client token - Used by both middleware and api modules API Design: - TokenManager::new(secret: &str) - Simplified to single parameter - token_manager() - Global singleton accessor - No public exports of TokenManager struct - use token_manager() instead - Constants (TOKEN_SUBJECT, TOKEN_EXPIRE_HOURS) are private Benefits: - Minimal API surface - one function to get the manager - No duplicate token_manager() implementations - Single source of truth for token configuration - Eliminates unnecessary wrapper functions - Better encapsulation - constants and struct are private - Comprehensive test coverage (3 tests) Code Reduction: - Removed duplicate token_manager() from api.rs and middleware.rs - Removed unnecessary verify_token() wrapper from middleware.rs - Net reduction: 31 lines of code Changes by Module: - auth/token.rs: - TokenManager with simplified constructor (1 param instead of 3) - Global token_manager() function with OnceLock - Private constants for subject and expiration - auth/mod.rs: - Exports only token_manager() function - TokenManager struct is not publicly exported - middleware.rs: - Removed local token_manager() function - Removed verify_token() wrapper function - Uses auth::token_manager() directly - Tests updated to call token_manager().verify_token() directly - api.rs: - Removed local token_manager() function - Uses auth::token_manager() directly Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

empwilli

Overall fine for me; minor concerns wrt. architectural approach.

src/auth/token.rs

Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

Signed-off-by: Buell <50990105+JanZachmann@users.noreply.github.com>

empwilli

LGTM

JanZachmann force-pushed the refactor-auth-and-testing branch 3 times, most recently from 4211618 to a2ca5da Compare November 5, 2025 20:17

JanZachmann and others added 4 commits November 6, 2025 19:13

refactor: address PR review feedback

8549fdb

JanZachmann force-pushed the refactor-auth-and-testing branch from a2ca5da to e776909 Compare November 6, 2025 22:05

JanZachmann requested review from empwilli and ronny-standtke November 7, 2025 09:19

empwilli reviewed Nov 7, 2025

View reviewed changes

src/auth/token.rs Outdated Show resolved Hide resolved

src/auth/token.rs Outdated Show resolved Hide resolved

src/auth/token.rs Outdated Show resolved Hide resolved

JanZachmann force-pushed the refactor-auth-and-testing branch from 076e550 to 08a54db Compare November 7, 2025 14:24

fix: fixed review findings

b681119

Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

JanZachmann force-pushed the refactor-auth-and-testing branch from 08a54db to b681119 Compare November 7, 2025 14:35

Merge branch 'main' into refactor-auth-and-testing

e578ec2

Signed-off-by: Buell <50990105+JanZachmann@users.noreply.github.com>

JanZachmann requested a review from empwilli November 7, 2025 14:37

empwilli approved these changes Nov 10, 2025

View reviewed changes

JanZachmann merged commit 3c7fa22 into omnect:main Nov 10, 2025
4 checks passed

JanZachmann deleted the refactor-auth-and-testing branch December 18, 2025 08:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: centralize authentication with TokenManager#61

refactor: centralize authentication with TokenManager#61
JanZachmann merged 6 commits intoomnect:mainfrom
JanZachmann:refactor-auth-and-testing

JanZachmann commented Oct 26, 2025 •

edited

Loading

Uh oh!

empwilli left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

empwilli left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

JanZachmann commented Oct 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Key Changes

🔐 Authentication Refactoring

New: src/auth/token.rs (109 lines)

Updated: src/auth/mod.rs

🧹 Removed Duplication

src/middleware.rs

src/api.rs

🧪 Testing Improvements

New: tests/validate_portal_token.rs (98 lines)

New: tests/http_client.rs (178 lines)

Benefits

Testing

Uh oh!

empwilli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

empwilli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JanZachmann commented Oct 26, 2025 •

edited

Loading

New: `src/auth/token.rs` (109 lines)

Updated: `src/auth/mod.rs`

`src/middleware.rs`

`src/api.rs`

New: `tests/validate_portal_token.rs` (98 lines)

New: `tests/http_client.rs` (178 lines)