Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,11 @@ PAYMENTS_SERVICE_OAUTH2_CLIENT_ID=
PAYMENTS_SERVICE_OAUTH2_CLIENT_SECRET=
PAYMENTS_SERVICE_OAUTH2_SCOPES=payment-profile/read

# DROPBOX MATERIALIZER SERVICE

DROPBOX_MATERIALIZER_URL=http://localhost:8100
DROPBOX_MATERIALIZER_KEY=
DROPBOX_MATERIALIZER_TIMEOUT=10
Comment on lines +271 to +273
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Resolve dotenv key-order warnings for the new materializer vars.

dotenv-linter flags the current order; this can create avoidable CI noise.

♻️ Proposed fix
-DROPBOX_MATERIALIZER_URL=http://localhost:8100
 DROPBOX_MATERIALIZER_KEY=
 DROPBOX_MATERIALIZER_TIMEOUT=10
+DROPBOX_MATERIALIZER_URL=http://localhost:8100
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
DROPBOX_MATERIALIZER_URL=http://localhost:8100
DROPBOX_MATERIALIZER_KEY=
DROPBOX_MATERIALIZER_TIMEOUT=10
DROPBOX_MATERIALIZER_KEY=
DROPBOX_MATERIALIZER_TIMEOUT=10
DROPBOX_MATERIALIZER_URL=http://localhost:8100
🧰 Tools
🪛 dotenv-linter (4.0.0)

[warning] 272-272: [UnorderedKey] The DROPBOX_MATERIALIZER_KEY key should go before the DROPBOX_MATERIALIZER_URL key

(UnorderedKey)


[warning] 273-273: [UnorderedKey] The DROPBOX_MATERIALIZER_TIMEOUT key should go before the DROPBOX_MATERIALIZER_URL key

(UnorderedKey)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.env.example around lines 271 - 273, The dotenv-linter warning is caused by
the ordering of the new environment keys; reorder the DROPBOX_MATERIALIZER_*
variables to match the repository's dotenv key-order convention (alphabetical
within the block) so the keys DROPBOX_MATERIALIZER_KEY,
DROPBOX_MATERIALIZER_TIMEOUT, and DROPBOX_MATERIALIZER_URL appear in the correct
sorted order; update the .env.example entry for these three variables
accordingly.


# L5_FORMAT_TO_USE_FOR_DOCS=yaml
# L5_SWAGGER_GENERATE_ALWAYS=true # Dev setting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ public static function buildForAdd(array $payload = []): array
'registration_send_order_email_automatically' => 'sometimes|boolean',
'registration_allow_automatic_reminder_emails' => 'sometimes|boolean',
'allow_update_attendee_extra_questions' => 'sometimes|boolean',
'dropbox_sync_enabled' => 'sometimes|boolean',
'time_zone_label' => 'sometimes|string',
'registration_allowed_refund_request_till_date' => 'nullable|date_format:U|epoch_seconds',
'registration_slug_prefix' => 'required|string|max:50',
Expand Down Expand Up @@ -181,6 +182,7 @@ public static function buildForUpdate(array $payload = []): array
'registration_send_order_email_automatically' => 'sometimes|boolean',
'registration_allow_automatic_reminder_emails' => 'sometimes|boolean',
'allow_update_attendee_extra_questions' => 'sometimes|boolean',
'dropbox_sync_enabled' => 'sometimes|boolean',
'time_zone_label' => 'sometimes|string',
'registration_allowed_refund_request_till_date' => 'nullable|date_format:U|epoch_seconds',
'registration_slug_prefix' => 'sometimes|string|max:50',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
<?php namespace App\Http\Controllers;
/**
* Copyright 2026 OpenStack Foundation
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
**/

use App\Models\Exceptions\AuthzException;
use App\Models\Foundation\Main\IGroup;
use App\Models\Foundation\Summit\Locations\SummitVenue;
use App\Services\Apis\IDropboxMaterializerApi;
use models\oauth2\IResourceServerContext;
use models\summit\ISummitRepository;
use models\summit\Summit;
use models\exceptions\ValidationException;
use models\exceptions\EntityNotFoundException;

/**
* Class OAuth2SummitDropboxSyncApiController
* @package App\Http\Controllers
*/
final class OAuth2SummitDropboxSyncApiController extends OAuth2ProtectedController
{
/**
* @var IDropboxMaterializerApi
*/
private $materializer_api;

/**
* @param ISummitRepository $summit_repository
* @param IDropboxMaterializerApi $materializer_api
* @param IResourceServerContext $resource_server_context
*/
public function __construct(
ISummitRepository $summit_repository,
IDropboxMaterializerApi $materializer_api,
IResourceServerContext $resource_server_context
)
{
parent::__construct($resource_server_context);
$this->repository = $summit_repository;
$this->materializer_api = $materializer_api;
}

/**
* @param Summit $summit
* @return void
* @throws \Exception
*/
private function checkAdminPermission(Summit $summit): void
{
$current_member = $this->resource_server_context->getCurrentUser();
if (!is_null($current_member) && !$current_member->isAdmin() && !$current_member->hasPermissionForOnGroup($summit, IGroup::SummitAdministrators))
throw new AuthzException(
sprintf("Member %s has not permission for this Summit", $current_member->getId())
);
}
Comment on lines +57 to +64
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fail-closed authorization required when member context is missing.

The checkAdminPermission() method has a fail-open vulnerability: when getCurrentUser() returns null, the compound condition evaluates to false, allowing the request to proceed without any authorization check. This bypasses the admin and group permission validation entirely. Reject null context upfront with a dedicated check.

Suggested fix
     private function checkAdminPermission(Summit $summit): void
     {
         $current_member = $this->resource_server_context->getCurrentUser();
+        if (is_null($current_member)) {
+            throw new AuthzException('A current member context is required.');
+        }
-        if (!is_null($current_member) && !$current_member->isAdmin() && !$current_member->hasPermissionForOnGroup($summit, IGroup::SummitAdministrators))
+        if (!$current_member->isAdmin() && !$current_member->hasPermissionForOnGroup($summit, IGroup::SummitAdministrators))
             throw new AuthzException(
                 sprintf("Member %s has not permission for this Summit", $current_member->getId())
             );
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitDropboxSyncApiController.php`
around lines 57 - 64, checkAdminPermission currently allows a null member to
pass due to the compound condition; explicitly reject a missing user by checking
resource_server_context->getCurrentUser() and throwing an AuthzException if it
is null before evaluating isAdmin() or hasPermissionForOnGroup; update
checkAdminPermission (the method name) to first set $current_member =
$this->resource_server_context->getCurrentUser() and immediately throw a clear
AuthzException (same type used elsewhere) when $current_member is null, then
continue with the existing admin and IGroup::SummitAdministrators permission
checks.


/**
* @param int $summit_id
* @return Summit
* @throws EntityNotFoundException
*/
private function findSummit(int $summit_id): Summit
{
$summit = $this->repository->getById($summit_id);
if (is_null($summit) || !$summit instanceof Summit)
throw new EntityNotFoundException(sprintf("Summit %s not found", $summit_id));
return $summit;
}

/**
* @param Summit $summit
* @throws ValidationException
*/
private function requireSyncEnabled(Summit $summit): void
{
if (!$summit->isDropboxSyncEnabled())
throw new ValidationException("Dropbox sync is not enabled for this summit.");
}

/**
* POST /api/v1/summits/{id}/dropbox-sync/materialize
*/
public function materialize($summit_id)
{
return $this->processRequest(function () use ($summit_id) {
$summit = $this->findSummit(intval($summit_id));
$this->checkAdminPermission($summit);
$this->requireSyncEnabled($summit);

$result = $this->materializer_api->materialize($summit->getId());
return $this->ok($result);
Comment on lines +94 to +100
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Reject malformed IDs instead of coercing them.

All handlers use intval() on route parameters. Values like 1foo become 1, so malformed admin requests can target the wrong summit, venue, or room instead of failing fast. Add numeric route constraints or strict validation before lookup.

One way to harden the parsing
+    private function parsePositiveInt($value, string $field): int
+    {
+        $value = (string)$value;
+        if (!ctype_digit($value)) {
+            throw new ValidationException(sprintf('%s must be a positive integer.', $field));
+        }
+        return (int)$value;
+    }
...
-            $summit = $this->findSummit(intval($summit_id));
+            $summit = $this->findSummit($this->parsePositiveInt($summit_id, 'summit_id'));

Also applies to: 109-125, 136-142, 151-156, 165-170, 179-184

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitDropboxSyncApiController.php`
around lines 94 - 100, The controller currently coerces route params using
intval (e.g. in the block calling findSummit, checkAdminPermission,
requireSyncEnabled and materializer_api->materialize) which accepts malformed
IDs like "1foo"; replace the coercion with strict validation: verify the
incoming $summit_id (and other route IDs in the other handlers) are pure
integers (e.g. ctype_digit or filter_var with FILTER_VALIDATE_INT) and return a
400/validation error if not, then pass the validated int to findSummit (and
similar methods) instead of intval; alternatively add numeric route constraints
so these handlers never receive malformed IDs.

});
}

/**
* POST /api/v1/summits/{id}/dropbox-sync/materialize/{location_id}/{room_id}
*/
public function materializeRoom($summit_id, $location_id, $room_id)
{
return $this->processRequest(function () use ($summit_id, $location_id, $room_id) {
$summit = $this->findSummit(intval($summit_id));
$this->checkAdminPermission($summit);
$this->requireSyncEnabled($summit);

$venue = $summit->getLocation(intval($location_id));
if (is_null($venue) || !$venue instanceof SummitVenue)
throw new EntityNotFoundException(sprintf("Venue %s not found", $location_id));

$room = $venue->getRoom(intval($room_id));
if (is_null($room))
throw new EntityNotFoundException(sprintf("Room %s not found", $room_id));

$result = $this->materializer_api->materializeRoom(
$summit->getId(),
$venue->getName(),
$room->getName()
);
return $this->ok($result);
});
}

/**
* POST /api/v1/summits/{id}/dropbox-sync/backfill
*/
public function backfill($summit_id)
{
return $this->processRequest(function () use ($summit_id) {
$summit = $this->findSummit(intval($summit_id));
$this->checkAdminPermission($summit);
$this->requireSyncEnabled($summit);

$result = $this->materializer_api->backfill($summit->getId());
return $this->ok($result);
});
}

/**
* POST /api/v1/summits/{id}/dropbox-sync/rebuild
*/
public function rebuild($summit_id)
{
return $this->processRequest(function () use ($summit_id) {
$summit = $this->findSummit(intval($summit_id));
$this->checkAdminPermission($summit);

$result = $this->materializer_api->rebuild($summit->getId());
return $this->ok($result);
});
}

/**
* GET /api/v1/summits/{id}/dropbox-sync/preflight
*/
public function preflight($summit_id)
{
return $this->processRequest(function () use ($summit_id) {
$summit = $this->findSummit(intval($summit_id));
$this->checkAdminPermission($summit);

$result = $this->materializer_api->preflight($summit->getId());
return $this->ok($result);
});
}

/**
* GET /api/v1/summits/{id}/dropbox-sync/status
*/
public function status($summit_id)
{
return $this->processRequest(function () use ($summit_id) {
$summit = $this->findSummit(intval($summit_id));
$this->checkAdminPermission($summit);

$result = $this->materializer_api->status($summit->getId());
return $this->ok($result);
});
}
}
2 changes: 2 additions & 0 deletions app/ModelSerializers/Summit/SummitSerializer.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ class SummitSerializer extends SilverStripeSerializer
'RegistrationAllowAutomaticReminderEmails' => 'registration_allow_automatic_reminder_emails:json_boolean',
'Modality' => 'modality:json_string',
'AllowUpdateAttendeeExtraQuestions' => 'allow_update_attendee_extra_questions:json_boolean',
'DropboxSyncEnabled' => 'dropbox_sync_enabled:json_boolean',
'TimeZoneLabel' => 'time_zone_label:json_string',
'RegistrationAllowedRefundRequestTillDate' => 'registration_allowed_refund_request_till_date:datetime_epoch',
'RegistrationSlugPrefix' => 'registration_slug_prefix:json_string',
Expand Down Expand Up @@ -162,6 +163,7 @@ class SummitSerializer extends SilverStripeSerializer
'registration_allow_automatic_reminder_emails',
'modality',
'allow_update_attendee_extra_questions',
'dropbox_sync_enabled',
'time_zone_label',
'registration_allowed_refund_request_till_date',
'registration_slug_prefix',
Expand Down
4 changes: 4 additions & 0 deletions app/Models/Foundation/Summit/Factories/SummitFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ public static function populate(Summit $summit, array $data){
$summit->setAllowUpdateAttendeeExtraQuestions(boolval($data['allow_update_attendee_extra_questions']));
}

if(isset($data['dropbox_sync_enabled'])){
$summit->setDropboxSyncEnabled(boolval($data['dropbox_sync_enabled']));
}

if(isset($data['dates_label']) ){
$summit->setDatesLabel(trim($data['dates_label']));
}
Expand Down
23 changes: 23 additions & 0 deletions app/Models/Foundation/Summit/Summit.php
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,12 @@ public function setMarketingSiteOauth2ClientScopes(string $marketing_site_oauth2
#[ORM\Column(name: 'RegistrationAllowAutomaticReminderEmails', type: 'boolean')]
private $registration_allow_automatic_reminder_emails;

/**
* @var bool
*/
#[ORM\Column(name: 'DropboxSyncEnabled', type: 'boolean')]
private $dropbox_sync_enabled;

#[ORM\OneToMany(targetEntity: \SummitEvent::class, mappedBy: 'summit', cascade: ['persist', 'remove'], orphanRemoval: true, fetch: 'EXTRA_LAZY')]
private $events;

Expand Down Expand Up @@ -1190,6 +1196,7 @@ public function __construct()
$this->registration_allow_automatic_reminder_emails = true;
$this->registration_send_order_email_automatically = true;
$this->allow_update_attendee_extra_questions = false;
$this->dropbox_sync_enabled = false;
$this->registration_companies = new ArrayCollection();
$this->external_registration_feed_last_ingest_date = null;
$this->speakers_announcement_emails = new ArrayCollection();
Expand Down Expand Up @@ -6362,6 +6369,22 @@ public function setAllowUpdateAttendeeExtraQuestions(bool $allow_update_attendee
$this->allow_update_attendee_extra_questions = $allow_update_attendee_extra_questions;
}

/**
* @return bool
*/
public function isDropboxSyncEnabled(): bool
{
return $this->dropbox_sync_enabled;
}

/**
* @param bool $dropbox_sync_enabled
*/
public function setDropboxSyncEnabled(bool $dropbox_sync_enabled): void
{
$this->dropbox_sync_enabled = $dropbox_sync_enabled;
}

/**
* @return bool
*/
Expand Down
Loading
Loading