Skip to content

v1.6.36#189

Merged
roncodes merged 23 commits intomainfrom
dev-v1.6.36
Feb 27, 2026
Merged

v1.6.36#189
roncodes merged 23 commits intomainfrom
dev-v1.6.36

Conversation

@roncodes
Copy link
Member

No description provided.

Manus AI and others added 23 commits February 11, 2026 03:40
This enhancement adds the user's updated_at timestamp to the cache key
generation in UserCacheService, enabling automatic cache busting when
user data is modified.

Changes:
- Updated getCacheKey() to accept User object and include updated_at timestamp
- Modified get(), put(), and invalidate() methods to work with User objects
- Updated UserController to pass User object to cache methods
- Added clarifying comments in UserObserver and User model

Benefits:
- Automatic cache invalidation when user data changes
- Improved cache consistency and data freshness
- Aligns with existing ETag implementation
- Simplifies cache management logic
…timestamp

feat: Include updated_at timestamp in UserCacheService cache key
The VerificationCode model was creating records with NULL status because
the generateFor() method never initialized the status field.

This caused issues when querying for verification codes with:
  ->where('status', 'pending')

Now all verification codes are created with status = 'pending' by default,
which is the expected behavior for newly generated codes.

Fixes verification flow for email and SMS verification codes.
…status

fix: Set default status to 'pending' for verification codes
Issue:
- Verification email was displaying raw HTML table code instead of rendered button
- Users saw: <table class="action" align="center"...> in the email body
- Caused by incorrect markdown mail template structure

Root Cause:
- Template used @component('mail::button') without @component('mail::message') wrapper
- Custom <x-mail-layout> component doesn't support nested markdown components
- Laravel markdown renderer couldn't process the button component properly

Solution:
- Replaced custom <x-mail-layout> with standard @component('mail::message')
- Properly wrapped @component('mail::button') inside message component
- Used markdown syntax for formatting (**, `code`, etc.)
- Removed custom layout that was incompatible with markdown components

Before:
<x-mail-layout>
    ...
    @component('mail::button', [...])
    @endcomponent
</x-mail-layout>

After:
@component('mail::message')
    ...
    @component('mail::button', [...])
    @endcomponent
@endcomponent

Result:
✅ Button renders correctly as HTML button
✅ No raw HTML code visible in email
✅ Proper markdown mail formatting
✅ Works with Laravel 10+ mail system
…ibility

Root Cause:
- VerificationMail used Laravel 10+ Content::markdown() syntax
- This processes markdown templates differently than old build()->markdown()
- The new Content API doesn't properly render @component('mail::button') inside custom layouts
- CompanyRegistered (working) uses old build()->markdown() syntax

Solution:
- Changed VerificationMail from content() method to build() method
- Now uses same syntax as CompanyRegistered and other working emails
- Keeps original x-mail-layout template with branding
- Button component now renders correctly

Changes:
1. VerificationMail.php: Replaced envelope()/content() with build()
2. verification.blade.php: Restored original template (no changes needed)

The issue wasn't the template - it was how the Mail class configured markdown processing!
…ndering

fix: Verification email showing raw HTML code
- [CRITICAL] Remove hardcoded SMS auth bypass code '000999' from
  AuthController::authenticateSmsCode(). Replace with a configurable
  env-driven bypass (SMS_AUTH_BYPASS_CODE) that is only active in
  non-production environments. Use hash_equals() for constant-time
  OTP comparison to prevent timing attacks.

- [HIGH] Replace shell_exec() in InstallerController::migrate() with
  Artisan::call('migrate', ['--force' => true]) to eliminate the
  OS command injection vector.

- [HIGH] Remove erroneous shell_exec() call in
  AuthController::sendVerificationSms() and replace with a plain
  string literal for the SMS message body.

- [HIGH] Add a 10-minute Redis TTL (setex) to SMS verification codes
  in sendVerificationSms() to prevent replay attacks.

- [HIGH] Remove the unauthenticated authToken bypass from
  AuthController::login(). Tokens submitted as request body
  parameters bypassed password and 2FA checks entirely.

- [HIGH] Add company-scoped authorization checks to
  UserController::deactivate() to prevent cross-organization IDOR.
  Block self-deactivation and prevent non-admins from deactivating
  admin accounts.

- [MEDIUM] Enforce a consistent strong password policy (min 8 chars,
  mixed case, letters, numbers, symbols, uncompromised) across all
  request validators: SignUpRequest, ResetPasswordRequest,
  UpdatePasswordRequest, and ValidatePasswordRequest.

- [MEDIUM] Prevent user enumeration in AuthController::login() by
  returning a single generic error message for both unknown identity
  and wrong password cases.

- [MEDIUM] Remove 'exists:users,email' rule from LoginRequest to
  prevent direct user enumeration via validation error messages.

Refs: GHSA-3wj9-hh56-7fw7
The authToken login shortcut was removed in the previous commit due to
two bugs: (1) loadMissing() was called before the null check causing a
fatal error when no token was found, and (2) there was no verification
that the token belonged to the user identified by the 'identity' field,
allowing any valid token to authenticate as any user.

This commit restores the feature with both issues corrected:
- The null check on $personalAccessToken now occurs before loadMissing()
- The token owner's email/phone is verified against the request identity
  before accepting the token, preventing token-swap attacks
- If the token is invalid or mismatched, the request falls through
  silently to normal password-based authentication
Add CompanyScope global scope to enforce company-level tenant isolation
across all Eloquent models, closing the cross-tenant IDOR vulnerability
where single-record operations (find, update, delete) queried by
UUID/public_id without verifying the resource belonged to the caller's
company.

Changes
-------

src/Scopes/CompanyScope.php (new)
  - Implements Laravel Scope interface
  - Automatically appends WHERE company_uuid = session('company') to
    every query on models whose table has a company_uuid column
  - Self-guarding: no-ops during CLI execution (artisan, queue workers)
    and when no session company is present (unauthenticated routes,
    installer)
  - Caches Schema::hasColumn() results per table to avoid repeated
    introspection overhead
  - Registers a withoutCompanyScope() query macro as an explicit,
    readable escape hatch for super-admin / system-level queries
  - Provides flushColumnCache() helper for test isolation

src/Models/Model.php
  - Adds static boot() method that registers CompanyScope via
    addGlobalScope(new CompanyScope()) so every subclass inherits
    tenant isolation automatically, including all future models

src/Traits/HasApiModelBehavior.php
  - getById(): adds explicit defence-in-depth company_uuid WHERE clause
  - updateRecordFromRequest(): adds explicit company_uuid WHERE clause
  - findRecordOrFail(): adds explicit company_uuid WHERE clause
  - bulkRemove(): adds explicit company_uuid WHERE clause
  All four methods retain the explicit check so the constraint is
  visible at the call-site and survives any withoutGlobalScope() calls.

src/Traits/HasApiControllerBehavior.php
  - deleteRecord(): adds explicit company_uuid WHERE clause

Refs: GHSA-3wj9-hh56-7fw7
… in Filter

applyFilter() builds a candidate list of method names as both the raw
param name and its Str::camel() equivalent (e.g. 'doesnt_have_driver'
and 'doesntHaveDriver'), and correctly resolves direct methods for
either form. However, the subsequent isExpansion() check only tested
the raw snake_case name. Because expandFromClass() registers expansions
using the PHP method name (always camelCase), a snake_case query param
such as 'doesnt_have_driver' would never match the 'doesntHaveDriver'
expansion key, causing the filter to be silently skipped.

Fix: after failing to find the expansion under the raw name, also try
the camelCase variant so that snake_case query params correctly resolve
to their camelCase expansion counterparts.
…9-hh56-7fw7

security: patch GHSA-3wj9-hh56-7fw7 — hardcoded SMS bypass, shell_exec, IDOR, weak passwords, user enumeration
Auth::isInvalidPassword() has a strict string type declaration on its
$hashedPassword parameter. When a user account has no password set
(e.g. SSO-invited or programmatically provisioned accounts), calling
it with $user->password === null throws a TypeError before any guard
can run.

Fix: null out $user before the isInvalidPassword() call when the
account has no password, so the existing generic credentials error
path handles it. This avoids the TypeError and — crucially — does NOT
return a distinct error message or code for the no-password case,
which would leak account state and enable user enumeration.

The result for a no-password account is identical to a wrong-password
attempt: HTTP 401, 'These credentials do not match our records.'

Refs: GHSA-3wj9-hh56-7fw7
The previous check used isAdmin() (system-level flag) to determine
whether the target user was an administrator, which was incorrect.
The right check for org-level administrators is Spatie's hasRole().

Corrected logic:

  Tier 1 — System admins (isAdmin()) are fully protected: no one can
            deactivate a system admin via this endpoint, not even
            another system admin.

  Tier 2 — Users with the 'Administrator' role can only be deactivated
            by a system admin. A role-based Administrator or any other
            user cannot deactivate another Administrator.

  Tier 3 — Regular users within the same company can be deactivated
            by any authenticated caller who passes the company-scope
            and self-deactivation guards above.

Refs: GHSA-3wj9-hh56-7fw7
User::deactivate() and User::activate() update the global users.status
column, which locks the user out of every organisation they belong to.
For a multi-tenant system this is wrong — deactivating a user in one
organisation should have no effect on their membership in others.

Fix: both UserController::deactivate() and UserController::activate()
now operate directly on the CompanyUser pivot record scoped to the
current session company, leaving the User record and all other
CompanyUser records untouched.

  - deactivate(): sets companyUser->status = 'inactive' for the
    current company only
  - activate():   sets companyUser->status = 'active' for the
    current company only; also adds the same company-scope guard
    and 404 handling that deactivate() already had

The session_status attribute on User already reads from companyUser->status
so the API response value is unchanged.

Refs: GHSA-3wj9-hh56-7fw7
…ser and companyUser status

Activation must update both users.status and company_users.status
because a newly created user starts with a global status of 'inactive'.
Only updating the CompanyUser record would leave users.status as
'inactive', blocking the user from accessing any organisation.

Deactivation remains scoped to the CompanyUser record only (previous
commit) so that deactivating a user in one organisation does not lock
them out of other organisations they belong to.

Refs: GHSA-3wj9-hh56-7fw7
Previously PUT/PATCH /int/v1/users/{id} had no dedicated form request,
so email and phone could be set to an empty string (or any value) with
no validation.

Changes:
- Add UpdateUserRequest (src/Http/Requests/UpdateUserRequest.php)
  Uses 'sometimes' + 'required' (correct Laravel PATCH pattern):
  - email: required when present, must be valid, unique ignoring own row
  - phone: nullable when present, must be valid E.164, unique ignoring own row
  - name:  required when present, min 2 / max 100 chars
  Uniqueness rules use Rule::unique()->ignore($userId, 'uuid') so that
  saving a user's existing email/phone back does not fail.

- Register UpdateUserRequest on UserController
  Adds $updateRequest property so HasApiControllerBehavior::validateRequest()
  picks it up automatically for PUT/PATCH requests.

- Add explicit $this->validateRequest($request) call in updateRecord()
  The UserController overrides updateRecord() to handle role/permission/
  policy sync, bypassing the trait's built-in validateRequest() call.
  The explicit call ensures validation always runs before the update.
…on-user-update

fix: prevent empty email/phone on user update
@roncodes roncodes merged commit 438c9d2 into main Feb 27, 2026
3 checks passed
@roncodes roncodes deleted the dev-v1.6.36 branch February 27, 2026 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant