Add configurable number-allocation strategy for getNextNumber() (avoid reusing freed uidNumber/gidNumber)

[TomokiMuto]

Is your feature request related to a problem? Please describe.

App\Classes\Template::getNextNumber() currently always selects the first available number starting from pla.template.getnextnumber.<attr>.

For example, if numbers 5000 through 5114 have been allocated but 5017 was freed by deleting a user, the next entry created receives uidNumber=5017 instead of 5115.

In POSIX/Samba environments, reusing a freed uidNumber or gidNumber can be unsafe. The old numeric ID may still be referenced by file ownership, home directories, backups, or ACLs. Reassigning it can therefore unintentionally grant the new account access to files belonging to the previous account.

Describe the solution you'd like

Add a configurable allocation strategy alongside the existing per-attribute starting values, for example:

'getnextnumber' => [
    'uidnumber' => env('LDAP_TEMPLATE_UIDNUMBER_START', 1000),
    'gidnumber' => env('LDAP_TEMPLATE_GIDNUMBER_START', 1000),
    'strategy'  => env('LDAP_TEMPLATE_GETNEXTNUMBER_STRATEGY', 'first_gap'),
],

The supported strategies could be:

• first_gap: select the first available number from the configured starting value. This is the current behavior and should remain the default for backward compatibility.
• max_plus_one: scan all existing values and return the highest value found plus one.

For max_plus_one, the maximum should be determined only after the complete directory scan, including the existing pagination/sizelimit loop, has finished.

Describe alternatives you've considered

The next value can be determined externally by querying all existing uidNumber or gidNumber values and calculating the maximum plus one. However, this requires manual work or separate provisioning logic and bypasses the convenience of the template’s automatic number allocation.

Increasing the configured starting value also does not solve the underlying issue, because gaps above that value can still be reused.

Additional context

The existing first_gap behavior should remain the default so that current installations are not affected.

I would be happy to submit a PR if this approach sounds reasonable. I wanted to confirm the preferred direction first because this changes shared numbering behavior.

[TomokiMuto]

I put together a small patch implementing the strategy option proposed above.

It keeps first_gap as the default, so existing installations are unaffected, and adds max_plus_one as an opt-in strategy.

diff --git a/config/pla.php b/config/pla.php
index xxxxxxx..xxxxxxx 100644
--- a/config/pla.php
+++ b/config/pla.php
@@ -125,6 +125,7 @@
               'getnextnumber' => [
                       'gidnumber' => env('LDAP_TEMPLATE_GIDNUMBER_START', 1000),
                       'uidnumber' => env('LDAP_TEMPLATE_UIDNUMBER_START', 1000),
+                      'strategy' => env('LDAP_TEMPLATE_GETNEXTNUMBER_STRATEGY', 'first_gap'), // 'first_gap' | 'max_plus_one'
               ],
       ],

diff --git a/app/Classes/Template.php b/app/Classes/Template.php
index xxxxxxx..xxxxxxx 100644
--- a/app/Classes/Template.php
+++ b/app/Classes/Template.php
@@ -313,15 +313,25 @@ private function getNextNumber(string $arg): int|NULL

                       Log::debug(sprintf('%s:Result has [%s]',self::LOGKEY,$result->join('|')));

-                      if ($result->count())
-                              foreach ($result as $item) {
-                                      Log::debug(sprintf('%s:Checking [%d] against [%s]',self::LOGKEY,$number,$item));
-                                      if ($number < $item)
-                                              break;
-
-                                      $number += 1;
+                      $strategy = config('pla.template.getnextnumber.strategy','first_gap');
+
+                      if ($result->count()) {
+                              if ($strategy === 'max_plus_one') {
+                                      // Never reuse a freed number; always allocate after the highest value found.
+                                      Log::debug(sprintf('%s:Using max_plus_one strategy, highest existing is [%d]',self::LOGKEY,$result->max()));
+                                      $number = $result->max()+1;
+
+                              } else {
+                                      foreach ($result as $item) {
+                                              Log::debug(sprintf('%s:Checking [%d] against [%s]',self::LOGKEY,$number,$item));
+                                              if ($number < $item)
+                                                      break;
+
+                                              $number += 1;
+                                      }
                               }

-                      else
+                      } else {
                               $number += 1;
+                      }

                       // Remove redundant entries
                       $result = $result->filter(fn($item)=>$item>$number);

Notes

• The existing pagination/sizelimit loop is left unchanged.
• first_gap remains the default for backward compatibility.
• max_plus_one always advances beyond the highest value found instead of filling a gap left by a deleted entry.
• The patch applies cleanly with patch -p1.
• Both modified files pass php -l on PHP 8.4, which is the version currently targeted by the project and supports features used by the codebase, such as private(set).

Testing status

I have not yet been able to exercise getNextNumber() end-to-end in a running instance.

The container image does not appear to provide a console REPL, and the method depends on config('server'), which is only bound during an HTTP request.

I would appreciate it if someone could either:

• confirm the behavior against a live directory; or
• point me toward the preferred way to unit-test Template in isolation.

Happy to open this as a PR if the direction looks good.