Skip to content
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

[PLA-1694] Laravel Standalone & Docker #38

Draft
wants to merge 24 commits into
base: master
Choose a base branch
from
Draft

Conversation

leonardocustodio
Copy link
Member

@leonardocustodio leonardocustodio commented Mar 25, 2024

Type

enhancement, documentation


Description

  • Adds and configures various application settings including Beam processing, GraphiQL UI, and Marketplace.
  • Sets up the application entry point, request handling, and bootstrapping.
  • Defines User model and factory, along with migrations for personal access tokens and failed jobs.
  • Registers event listeners in the EventServiceProvider.

Changes walkthrough

Relevant files
Configuration changes
3 files
enjin-platform-beam.php
Add Beam Processing Configuration                                               

config/enjin-platform-beam.php

  • Adds configuration for beam processing including scan limit, batch
    processing, unit price, claim redirect URL, and pruning expired
    claims.
  • +63/-0   
    graphiql.php
    Configure GraphiQL UI Routes and Availability                       

    config/graphiql.php

  • Configures routes for GraphiQL UI with different endpoints.
  • Enables or disables GraphiQL UI based on environment variable.
  • +52/-1   
    enjin-platform-marketplace.php
    Add Marketplace Configuration                                                       

    config/enjin-platform-marketplace.php

    • Adds configuration for marketplace listing block offset.
    +8/-0     
    Miscellaneous
    2 files
    index.php
    Setup Application Entry Point and Request Handling             

    public/index.php

  • Sets up the entry point for the application, handling maintenance mode
    and auto-loading.
  • Handles incoming requests using the application's HTTP kernel.
  • +55/-0   
    app.php
    Bootstrap Application and Bind Interfaces                               

    bootstrap/app.php

  • Creates the Laravel application instance and binds important
    interfaces.
  • Returns the application instance for running and sending responses.
  • +55/-0   
    Tests
    1 files
    UserFactory.php
    Define User Factory Model State                                                   

    database/factories/UserFactory.php

  • Defines the model's default state for user factory.
  • Adds unverified state configuration.
  • +44/-0   
    Enhancement
    5 files
    RouteServiceProvider.php
    Configure Route Service Provider                                                 

    app/Providers/RouteServiceProvider.php

  • Defines route model bindings and middleware for web and API routes.
  • +40/-0   
    User.php
    Define User Model                                                                               

    app/Models/User.php

    • Defines the User model with attributes, hidden fields, and casts.
    +45/-0   
    2019_12_14_000001_create_personal_access_tokens_table.php
    Add Personal Access Tokens Migration                                         

    database/migrations/2019_12_14_000001_create_personal_access_tokens_table.php

    • Creates migration for personal access tokens table.
    +33/-0   
    EventServiceProvider.php
    Register Event Service Provider Listeners                               

    app/Providers/EventServiceProvider.php

  • Registers event listener for sending email verification notifications.

  • +38/-0   
    2019_08_19_000000_create_failed_jobs_table.php
    Add Failed Jobs Migration                                                               

    database/migrations/2019_08_19_000000_create_failed_jobs_table.php

    • Creates migration for failed jobs table.
    +32/-0   

    PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @leonardocustodio leonardocustodio added the enhancement New feature or request label Mar 25, 2024
    @leonardocustodio leonardocustodio self-assigned this Mar 25, 2024
    @leonardocustodio leonardocustodio marked this pull request as draft March 25, 2024 11:43
    @github-actions github-actions bot added the documentation Improvements or additions to documentation label Mar 25, 2024
    Copy link

    PR Description updated to latest commit (5865200)

    Copy link

    PR Review

    ⏱️ Estimated effort to review [1-5]

    5, because the PR introduces a significant amount of new configuration files and code across various aspects of the Laravel application, including database, authentication, caching, logging, and more. Reviewing this PR requires a thorough understanding of Laravel's configuration system, as well as the specific requirements of the application being developed. Additionally, the PR includes changes to key application files such as Kernel.php and User.php, which are central to the application's operation. The reviewer needs to ensure that these changes are compatible with the rest of the application and do not introduce any security vulnerabilities or performance issues.

    🧪 Relevant tests

    No

    🔍 Possible issues

    Possible Configuration Overload: The PR introduces a large number of configuration files. While this is common in Laravel applications, it's important to ensure that each configuration file is necessary and correctly set up to avoid potential issues in the application's behavior or performance.

    Security Concerns: Changes in files like config/auth.php, config/sanctum.php, and the introduction of personal access tokens and API tokens require careful review to ensure that the authentication and authorization mechanisms are secure and correctly implemented.

    Database Changes: The addition of new database migrations, such as the creation of personal access tokens table, requires reviewing to ensure they meet the application's data requirements and are correctly indexed for performance.

    🔒 Security concerns

    No direct security concerns identified without deeper context into the application's specific use cases. However, given the nature of changes involving authentication, authorization, and database configurations, it's crucial to ensure that these configurations do not inadvertently introduce security vulnerabilities, such as improper access control or data exposure.

    Code feedback:

    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
    
    [pr_reviewer]
    some_config1=...
    some_config2=...
    

    See the review usage page for a comprehensive guide on using this tool.

    Copy link

    github-actions bot commented Mar 25, 2024

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Security
    Define trusted proxies explicitly to enhance security.

    It's recommended to explicitly define your trusted proxies to prevent trusting potentially
    malicious proxy servers. This is especially important if your application is running
    behind a load balancer or reverse proxy.

    app/Http/Middleware/TrustProxies.php [15]

    -protected $proxies;
    +protected $proxies = ['192.168.1.1', '192.168.1.2'];
     
    Remove incorrect casting of the password attribute to prevent double hashing.

    The password attribute should not be cast as 'hashed' in the model. Laravel automatically
    hashes passwords when they are set. If you attempt to hash an already hashed password, it
    may lead to unexpected behavior or security vulnerabilities.

    app/Models/User.php [43]

    -'password' => 'hashed',
    +'password' => 'string',
     
    Enable TrustHosts middleware to enhance application security.

    Consider enabling the \App\Http\Middleware\TrustHosts::class middleware to ensure that
    your application only responds to requests from trusted hosts, enhancing its security.

    app/Http/Kernel.php [17]

    -// \App\Http\Middleware\TrustHosts::class,
    +\App\Http\Middleware\TrustHosts::class,
     
    Specify CORS policy for better security.

    Consider adding more specific values for 'allowed_origins', 'allowed_methods', and
    'allowed_headers' to enhance security by limiting the allowed origins, methods, and
    headers that can interact with your application.

    config/cors.php [19-25]

    -'allowed_methods' => ['*'],
    -'allowed_origins' => ['*'],
    -'allowed_headers' => ['*'],
    +'allowed_methods' => ['POST', 'GET', 'OPTIONS', 'PUT', 'DELETE'],
    +'allowed_origins' => ['https://example.com'],
    +'allowed_headers' => ['Content-Type', 'X-Requested-With', 'Authorization'],
     
    Conditionally disable GraphiQL UI in production for enhanced security.

    To enhance security, consider restricting access to the GraphiQL UI in production
    environments. This can be done by conditionally enabling the GraphiQL routes based on the
    application environment or by adding authentication middleware to the GraphiQL routes.

    config/graphiql.php [50]

    -'enabled' => env('GRAPHIQL_ENABLED', true),
    +'enabled' => env('APP_ENV') !== 'production' && env('GRAPHIQL_ENABLED', false),
     
    Use a more secure hashing algorithm for enhanced security.

    Considering the advancements in hardware and software capabilities, it's advisable to use
    a more secure hashing algorithm like argon2id instead of bcrypt. This change enhances the
    security of hashed data, such as passwords, stored by the application.

    config/hashing.php [18]

    -'driver' => 'bcrypt',
    +'driver' => 'argon2id',
     
    Secure Horizon's dashboard and API with authentication middleware.

    To ensure that Horizon's dashboard and API are only accessible to authorized users,
    consider adding authentication middleware to the Horizon configuration. This can prevent
    unauthorized access to job monitoring and management features.

    config/horizon.php [73]

    -'middleware' => ['web'],
    +'middleware' => ['web', 'auth'],
     
    Change the default state of Log Viewer to disabled to enhance security.

    To improve security, consider restricting access to the Log Viewer by default. You can
    change the default value of 'LOG_VIEWER_ENABLED' from true to false. This ensures that Log
    Viewer is explicitly enabled in environments where it's needed, reducing the risk of
    exposing sensitive log information unintentionally.

    config/log-viewer.php [13]

    -'enabled' => env('LOG_VIEWER_ENABLED', true),
    +'enabled' => env('LOG_VIEWER_ENABLED', false),
     
    Enhancement
    Uncomment useful query string parameters to ignore in signature validation.

    Uncommenting and specifying query string parameters to ignore in signature validation can
    be useful for tracking or marketing parameters that do not affect the resource being
    accessed. Consider uncommenting some of these parameters if they are used in your
    application.

    app/Http/Middleware/ValidateSignature.php [15-21]

    -// 'fbclid',
    -// 'utm_campaign',
    -// 'utm_content',
    -// 'utm_medium',
    -// 'utm_source',
    -// 'utm_term',
    +'fbclid',
    +'utm_campaign',
    +'utm_content',
    +'utm_medium',
    +'utm_source',
    +'utm_term',
     
    Use environment variables for authentication defaults configuration.

    Consider using environment variables for setting 'guard' and 'passwords' under 'defaults'
    to allow easy configuration changes without modifying the codebase.

    config/auth.php [16-19]

     'defaults' => [
    -    'guard' => 'web',
    -    'passwords' => 'users',
    +    'guard' => env('AUTH_DEFAULT_GUARD', 'web'),
    +    'passwords' => env('AUTH_DEFAULT_PASSWORDS', 'users'),
     ],
     
    Allow dynamic configuration of middleware through environment variables.

    For the 'middleware' and 'api_middleware' configurations, consider adding a configuration
    option to easily customize the middleware stack from the environment or application
    settings. This allows for more dynamic configuration based on the environment without
    changing the code.

    config/log-viewer.php [70-86]

    -'middleware' => [
    -    'web',
    -    \Opcodes\LogViewer\Http\Middleware\AuthorizeLogViewer::class,
    -],
    -'api_middleware' => [
    -    \Opcodes\LogViewer\Http\Middleware\EnsureFrontendRequestsAreStateful::class,
    -    \Opcodes\LogViewer\Http\Middleware\AuthorizeLogViewer::class,
    -],
    +'middleware' => explode(',', env('LOG_VIEWER_MIDDLEWARE', 'web,\Opcodes\LogViewer\Http\Middleware\AuthorizeLogViewer')),
    +'api_middleware' => explode(',', env('LOG_VIEWER_API_MIDDLEWARE', '\Opcodes\LogViewer\Http\Middleware\EnsureFrontendRequestsAreStateful,\Opcodes\LogViewer\Http\Middleware\AuthorizeLogViewer')),
     
    Maintainability
    Use a guard clause for early return to improve code readability.

    To improve readability and maintainability, consider using a guard clause to return early
    if any guard is authenticated, instead of nesting the logic inside the foreach loop.

    app/Http/Middleware/RedirectIfAuthenticated.php [22-26]

    -foreach ($guards as $guard) {
    -    if (Auth::guard($guard)->check()) {
    -        return redirect(RouteServiceProvider::HOME);
    -    }
    +if ($guards && Auth::guard($guards[0])->check()) {
    +    return redirect(RouteServiceProvider::HOME);
     }
     
    Group related configurations into nested arrays for better organization.

    To enhance the maintainability of the configuration file, consider grouping related
    configurations into nested arrays. For example, 'back_to_system_url' and
    'back_to_system_label' can be grouped under a 'back_to_system' key. This approach improves
    readability and organization of configuration options.

    config/log-viewer.php [47-49]

    -'back_to_system_url' => config('app.url', null),
    -'back_to_system_label' => null,
    +'back_to_system' => [
    +    'url' => config('app.url', null),
    +    'label' => null,
    +],
     
    Best practice
    Validate environment variables for the 'pusher' broadcasting configuration.

    For the 'pusher' connection options, consider validating the environment variables to
    ensure they are set before using them. This can prevent runtime errors due to missing
    configuration.

    config/broadcasting.php [33-44]

     'pusher' => [
         'driver' => 'pusher',
    -    'key' => env('PUSHER_APP_KEY'),
    -    'secret' => env('PUSHER_APP_SECRET'),
    -    'app_id' => env('PUSHER_APP_ID'),
    +    'key' => env('PUSHER_APP_KEY') ?? throw new \InvalidArgumentException("PUSHER_APP_KEY is not set."),
    +    'secret' => env('PUSHER_APP_SECRET') ?? throw new \InvalidArgumentException("PUSHER_APP_SECRET is not set."),
    +    'app_id' => env('PUSHER_APP_ID') ?? throw new \InvalidArgumentException("PUSHER_APP_ID is not set."),
         'options' => [
             'cluster' => env('PUSHER_APP_CLUSTER'),
             'host' => env('PUSHER_HOST') ?: 'api-'.env('PUSHER_APP_CLUSTER', 'mt1').'.pusher.com',
             'port' => env('PUSHER_PORT', 443),
             'scheme' => env('PUSHER_SCHEME', 'https'),
             'encrypted' => true,
             'useTLS' => env('PUSHER_SCHEME', 'https') === 'https',
         ],
     ],
     
    Validate environment variables for the 'memcached' cache configuration.

    For the 'memcached' cache store, consider adding error handling or validation to ensure
    that the MEMCACHED_HOST and MEMCACHED_PORT environment variables are set. This can prevent
    connection issues.

    config/cache.php [58-73]

     'memcached' => [
         'driver' => 'memcached',
         'persistent_id' => env('MEMCACHED_PERSISTENT_ID'),
         'sasl' => [
             env('MEMCACHED_USERNAME'),
             env('MEMCACHED_PASSWORD'),
         ],
         'servers' => [
             [
    -            'host' => env('MEMCACHED_HOST', '127.0.0.1'),
    +            'host' => env('MEMCACHED_HOST') ?? throw new \InvalidArgumentException("MEMCACHED_HOST is not set."),
                 'port' => env('MEMCACHED_PORT', 11211),
                 'weight' => 100,
             ],
         ],
     ],
     
    Enable strict mode for MySQL connection configuration.

    For the 'mysql' connection, consider enabling the 'strict' mode to enforce strict SQL to
    prevent silent data truncation and conversion errors. This can help in identifying
    potential issues during development.

    config/database.php [46-60]

     'mysql' => [
         'driver' => 'mysql',
         'host' => env('DB_HOST', '127.0.0.1'),
         'port' => env('DB_PORT', '3306'),
         'database' => env('DB_DATABASE', 'forge'),
         'username' => env('DB_USERNAME', 'forge'),
         'password' => env('DB_PASSWORD', ''),
         'charset' => 'utf8mb4',
         'collation' => 'utf8mb4_unicode_ci',
         'prefix' => '',
    -    'strict' => true,
    +    'strict' => env('DB_STRICT_MODE', true),
         'engine' => null,
     ],
     
    Add validation and default values for critical environment variable configurations.

    It's recommended to validate environment variables and provide default values where
    possible to ensure the application behaves predictably even when environment variables are
    missing or misconfigured. For critical configurations like authentication drivers,
    blockchain networks, and service endpoints, consider adding validation logic in the
    application bootstrapping process to check for the presence and validity of these
    configurations.

    config/enjin-platform.php [16]

    -'auth' => env('AUTH_DRIVER'),
    +'auth' => env('AUTH_DRIVER', 'default_auth_driver'),
     
    Explicitly specify file visibility for S3 disk configuration.

    For the S3 disk configuration, it's a good practice to explicitly specify the visibility
    of files to ensure that file access permissions are intentionally set. This helps in
    avoiding unintentional public access to sensitive files. Consider adding a 'visibility'
    key with appropriate value ('private' or 'public').

    config/filesystems.php [47-57]

     's3' => [
         'driver' => 's3',
         'key' => env('AWS_ACCESS_KEY_ID'),
         'secret' => env('AWS_SECRET_ACCESS_KEY'),
         'region' => env('AWS_DEFAULT_REGION'),
         'bucket' => env('AWS_BUCKET'),
         'url' => env('AWS_URL'),
         'endpoint' => env('AWS_ENDPOINT'),
         'use_path_style_endpoint' => env('AWS_USE_PATH_STYLE_ENDPOINT', false),
    +    'visibility' => 'private',  # or 'public' based on your needs
         'throw' => false,
     ],
     
    Validate environment variables to ensure they are set to expected values.

    Consider validating the environment variables used in the configuration file to ensure
    they are set to expected values and types. This can prevent potential issues when the
    environment variables are missing or misconfigured. For example, you can create a custom
    validation function or use a package that validates environment variables at the
    application startup.

    config/log-viewer.php [13-199]

    -'enabled' => env('LOG_VIEWER_ENABLED', true),
    -'cache_driver' => env('LOG_VIEWER_CACHE_DRIVER', null),
    +'enabled' => (bool) env('LOG_VIEWER_ENABLED', true),
    +'cache_driver' => env('LOG_VIEWER_CACHE_DRIVER', config('cache.default')),
     
    Use environment variables for log paths to increase configuration flexibility.

    It's a good practice to avoid hardcoding paths in the configuration files. For the log
    paths included under various operating systems, consider using environment variables or
    application configuration values. This approach makes the configuration more flexible and
    easier to adjust for different environments without modifying the code.

    config/log-viewer.php [137-141]

    -'/var/log/httpd/*',
    -'/var/log/nginx/*',
    -'/opt/homebrew/var/log/nginx/*',
    +env('LOG_PATH_HTTPD', '/var/log/httpd/*'),
    +env('LOG_PATH_NGINX', '/var/log/nginx/*'),
    +env('LOG_PATH_HOMEBREW_NGINX', '/opt/homebrew/var/log/nginx/*'),
     

    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    documentation Improvements or additions to documentation enhancement New feature or request Review effort [1-5]: 5
    Development

    Successfully merging this pull request may close these issues.

    None yet

    2 participants