Fix AWS credentials verification and improve region selection UX (v1.6.4) - #181
Merged
Conversation
Replace stub implementations with complete functionality for three critical
functions in PhoenixKit.EmailSystem.RateLimiter:
1. reduce_user_limits/2
- Creates temporary reduced limits for flagged users
- Stores limits in JSON settings (user_rate_limits_#{user_id})
- Automatically expires after 24 hours
- Reduces limits to 10% of defaults (min 10/50)
- Logs all limit reductions with detailed metadata
2. block_user_emails/2
- Retrieves user email via PhoenixKit.Users.Auth.get_user!/1
- Adds email to phoenix_kit_email_blocklist table
- Sets 7-day expiration for temporary blocks
- Creates monitoring event for blocked user
- Integrates with existing blocklist system
3. monitor_user/3
- Tracks suspicious user behavior events
- Stores events in JSON settings (user_monitoring_#{user_id})
- Maintains event history with automatic 30-day pruning
- Records event type, metadata, and timestamps
- Provides comprehensive audit trail
Additional improvements:
- Add helper functions: get_user_limits/1, get_user_recipient_limit/1,
get_user_sender_limit/1, clear_user_limits/1, get_user_monitoring/1
- Add public API: check_user_limits/1, get_user_limit_status/1,
clear_user_rate_limits/1, get_user_monitoring_events/1
- Update module documentation with user behavior management features
- Fix flag_suspicious_activity/2 to properly call monitor_user/3
- Add comprehensive @doc blocks for all functions
- Include usage examples and data structure documentation
Integration:
- Uses PhoenixKit.Settings for JSON storage
- Integrates with phoenix_kit_email_blocklist table
- Links to PhoenixKit.Users.Auth for user data
- Automatic expiration and cleanup of stale data
- Comprehensive error handling with logging
All functions now provide production-ready anti-spam and rate limiting
capabilities with proper user tracking and automatic enforcement.
…ementations
Resolved merge conflicts by integrating the latest dev changes with
enhanced RateLimiter implementations. The rate_limiter.ex file now includes:
Enhanced Features:
- reduce_user_limits: JSON-based storage with 10% reduction, 24h expiration
- block_user_emails: Email blocking with 7-day expiration and monitoring
- monitor_user: Event history tracking with auto-pruning (30 days)
Public API Added:
- check_user_limits/1: Retrieve user-specific limits
- get_user_limit_status/1: Full user limit status details
- clear_user_rate_limits/1: Admin function to clear limits
- get_user_monitoring_events/1: Retrieve user monitoring history
Implementation Details:
- Single JSON structure per user (user_rate_limits_#{user_id})
- Separate recipient/sender limits with configurable reduction
- Automatic expiration handling
- Comprehensive error handling with rescue blocks
- Monitoring integration for blocked users
- Fix undefined variable error in @moduledoc by changing interpolation syntax - Refactor get_user_limits/1 to use 'with' statement instead of nested case - Reduce nesting depth from 4 to 2 levels to satisfy Credo requirements - Improve code readability while maintaining same functionality
- Remove @doc attributes from 8 private functions (warnings from Elixir compiler) - Remove unused get_user_recipient_limit/1 and get_user_sender_limit/1 functions - Remove default parameter from monitor_user/3 (never used) - Convert @doc comments to regular # comments for private functions - Reduces file by 205 lines of unnecessary documentation
…tubs-011CV2Sy67y3T18ndzESneNd Implement RateLimiter user management functions
Resolved issue where users couldn't save AWS credentials because region selection appeared mandatory, but region list required saved credentials to load. Changes: - Update AWS region select dropdown to clearly indicate region is optional - Add helpful placeholder text explaining workflow: save credentials first, then refresh regions - Improve step-by-step setup guide in email settings to clarify the process: 1. Enter and save credentials 2. Verify credentials 3. Refresh regions list 4. Select region (or use default) 5. Save settings again - Add contextual help messages based on form state - Convert HTML comments to EEx comments in component This allows users to save Access Key and Secret Key first, then load available regions, breaking the circular dependency loop.
…mREMp6iijsduwy6TEVQK Fix AWS region selection circular dependency in email settings
- Fix AWS STS response parsing for ExAws compatibility - Add manual region input with optional dropdown loading - Reduce configuration steps from 7 to 4 - Remove double-save requirement for credentials and region
- Extract verification logic into separate helper functions - Reduce cyclomatic complexity from 14 to acceptable level - Improve code readability and maintainability
Implement two-pass update strategy to prevent app startup failures when
required configuration is missing. This addresses the core issue where
Mix caches configuration at task startup, making runtime config modifications
ineffective.
Changes:
- Add check_required_configuration/0 to detect missing Ueberauth config
- Implement handle_missing_configuration_pass/1 for first-pass config addition
- Implement handle_normal_update_pass/2 for second-pass update completion
- Move Mix.Task.run("app.start") after configuration validation
- Update module and help documentation to explain two-pass strategy
The update process now:
1. First run (if config missing): Adds configuration via Igniter, prompts
user to run command again
2. Second run (config present): Safely starts app and completes update
This prevents the scenario where:
- app.start runs with cached (missing) configuration
- Igniter adds configuration to file
- App fails because cached config lacks required settings
Impact: Prevents upgrade failures for projects missing Ueberauth or other
required configuration. Provides clear user guidance for two-pass updates.
The previous implementation incorrectly called super() from helper functions that don't override any parent methods. Elixir only allows super() calls in functions that override parent class methods. Restructured to keep all super(argv) calls within run/1: - Replace handle_missing_configuration_pass/1 with show_missing_config_message/1 - Replace handle_normal_update_pass/2 with inline code in run/1 - Add show_config_added_message/1 for completion message - All super() calls now happen directly in run/1 case statement This maintains the two-pass update strategy while fixing the compilation error.
Delete ensure_hammer_config_before_start/0 and add_hammer_config_directly/2 functions that were attempting runtime config modification. These functions were the root cause of the configuration timing bug. The proper Igniter-based approach via RateLimiterConfig is now used instead, which was added by the dev team in recent commits. Changes: - Remove ensure_hammer_config_before_start/0 (lines 720-751) - Remove add_hammer_config_directly/2 (lines 754-809) - Add Hammer config check to check_required_configuration/0 - Update user messages to mention both Ueberauth and Hammer config This completes the fix for the configuration timing issue by ensuring all runtime config modification is removed and only proper Igniter-based configuration is used.
…iming-01JAPHWRHmAJBCNqiLdRrFQb Fix configuration timing issue in phoenix_kit.update task
PROBLEM 1: Configuration Detection
- Install/update tasks failed when Hammer config was commented
- String.contains? matched "config :hammer" in comments (#)
- App startup failed with "Missing required config: expiry_ms"
PROBLEM 2: Migration Timing Issue
- First pass created wrong migration (V01->V25 instead of V23->V25)
- Status check happened before app started with proper config
- Database version check failed without running application
SOLUTION:
1. Improved Configuration Detection (3 files):
a) install.ex check_required_configuration():
- Parse lines individually, ignore # comments
- Added has_active_hammer_config?() helper
b) update.ex check_required_configuration():
- Same line-by-line checking logic
- Added has_active_hammer_config?() helper
c) RateLimiterConfig.hammer_config_exists?():
- Replace String.contains? with line checking
- Verify both "config :hammer" and "expiry_ms" are uncommented
2. Two-Pass Strategy with Process Dictionary:
a) First Pass (config_status == :missing):
- Store :missing status in Process dictionary
- Add configuration via Igniter
- Skip status check and migration creation
- Display message to run command again
b) Second Pass (config_status == :ok):
- Store :ok status in Process dictionary
- Start application safely
- Check installation status with running app
- Create correct migration based on current DB version
3. Updated perform_igniter_update():
- Read config_status from Process dictionary
- If :missing → only add config, return early
- If :ok → proceed with full update logic
RESULT:
✅ Commented config properly detected as missing
✅ First pass: Only adds configuration, no migrations
✅ Second pass: Correct migration (V23->V25) created
✅ No "Could not start application hammer" errors
✅ Status check uses actual database version
FILES CHANGED:
- lib/mix/tasks/phoenix_kit.install.ex (+60 lines)
- lib/mix/tasks/phoenix_kit.update.ex (+62 lines)
- lib/phoenix_kit/install/rate_limiter_config.ex (+21 lines)
TESTED:
✅ Code compiles without warnings
✅ Credo passes (no issues found)
✅ Format check passes
Contributor
Author
|
Fix Hammer configuration detection and two-pass update logic also here |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary
This PR fixes critical AWS credentials verification issues and significantly improves the region selection UX, reducing setup complexity from 7 steps to 4.
Changes
Fixed
parse_sts_response/1to handle ExAws automatic XML-to-map conversionCaseClauseErrorwhen verifying credentials with valid AWS keysChanged
AWS Region Selection UX - Streamlined region input workflow from 7 steps to 4
Code Quality - Refactored AWS credentials verification handler
Test Plan
Version
Updated to v1.6.4 with comprehensive CHANGELOG entry.