[D9] WebTotem Security

• What follows is a quick review of the project; it doesn't mean to be complete
• For every point, the review doesn't make a complete list of lines that should be fixed, but an example of what is wrong in the code
• A review is about code that doesn't follow the coding standards, contains possible security issue, or doesn't correctly use the Drupal API; if a point isn't about that, it makes it clear

spl_autoload_register(function ($class) {

  // Register class auto loader.
  // Custom modules.
  if (strpos($class, 'Wtotem') !== FALSE) {
    $class = str_replace('\\', DIRECTORY_SEPARATOR, $class);
    $class_file = __DIR__ . DIRECTORY_SEPARATOR . $class . '.php';
    if (file_exists($class_file)) {
      require_once $class_file;
    }
  }
});

There is no need to use a custom class loader. As long as the classes are written in the correct directory and the correct file, they are automatically loaded from Drupal.

  public function settings() {
    // Checking whether the user was authorized by the api key.
    if (!App::authorized()) {
      return new RedirectResponse('/admin/wtotem/authorization_needed');
    }

    // Header with menu.
    $build[] = [
      "#theme_mode" => Helper::mGetThemeMode(),
      "#notifications" => Helper::mGetNotifications(),
      "#is_active" => ['settings' => 'wtotem_nav__link_active'],
      '#attached' => [
        'library' => [
          'wtotem/wtotem.main_css',
          'wtotem/wtotem.main_js',
          'wtotem/wtotem.header',
        ],
      ],
      '#theme' => 'wtotem_header',
    ];

    // Configs form.
    $form_state = new FormState();
    $build[] = $this->formBuilder()->buildForm('Drupal\wtotem\Form\WTotemConfigsForm', $form_state);

    // Allow/Deny ip list adding.
    $form_state = new FormState();
    $build[] = $this->formBuilder()->buildForm('Drupal\wtotem\Form\WTotemAllowDenyMultiAddForm', $form_state);

    // Footer.
    $build[] = [
      '#attached' => [
        'library' => [
          'wtotem/wtotem.main_css',
        ],
      ],
      '#theme' => 'wtotem_footer',
    ];

    return $build;
  }

Why is a setting form built using a controller class instead of a form class? Drupal uses form classes to show forms.

    if (!App::authorized()) {
      return new RedirectResponse('/admin/wtotem/authorization_needed');
    }

The Drupal way of controlling the access to routes is using a permission or a custom route access checker, as described in Custom route access checking.

  /**
   * {@inheritdoc}
   */
  public static function create(ContainerInterface $container) {
    return new static(
      $container->get('current_user')
    );
  }

An event handler doesn't implement create() like every service defined in a .services.yml file.

  /**
   * Check permission.
   */
  private function checkPermission() {
    if ($this->currentUser->hasPermission('administer') or
      array_intersect(['administrator', 'editor'], $this->currentUser->getRoles())) {
      return FALSE;
    }
    return TRUE;
  }

The documentation comment needs to document the returned value.
Is the code correctly disallowing the permission, or is the logic incorrectly converted? administer isn't an existing permission. The code should rather check the permissions an account has, not its roles.

  /**
   * {@inheritdoc}
   */
  public function submitForm(array &$form, FormStateInterface $form_state) {
    // The form will sent by ajax.
  }

Even in AJAX forms, the form submission handler is still submitForm().

  /**
   * Returns a unique string identifying the form.
   *
   * The returned ID should be a unique string that can be a valid PHP function
   * name, since it's used in hook implementation names such as
   * hook_form_FORM_ID_alter().
   *
   * @return string
   *   The unique string identifying the form.
   */
  public function getFormId() {
    return 'wtotem_allow_deny_multi_add_form';
  }

Since that is a method defined in the parent class, the document comment just needs to be inherited from the parent method.

    $wtotem_api_key = $form_state->getValue('api_key');

    if (!$wtotem_api_key) {
      $form_state->setErrorByName('api_key', $this->t('You have to set API key.'));
    }

When a form element is required, users cannot avoid to enter a value for that element. That code isn't necessary.

namespace Wtotem\Common;

The namespace is wrong. The namespace of a module whose machine name is wtotem starts with Drupal\wtotem.

class API extends ControllerBase {

It doesn't seem the class is a controller, as it's not used in any route. Only controller classes can extend ControllerBase.

  public static function mAuth($api_key) {

As per Drupal coding standards, method names aren't prefixed by m.

    $site_url = \Drupal::request()->getHost();

Controller classes use DI. In the case of the request object, that is automatically injected in the class constructor, if that uses a specific argument, as described in Routing API (Route controllers for simple routes).

A form can contain any render item, including a markup render item.

Thank you for your contribution! I am going to update your account.

These are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.