Refactor file access

[Sabr1n4W]

Implements #1409

Type

• Bugfix
• Feature
• Documentation
• Refactoring (e.g. Style updates, Test implementation, etc.)
• Other (please describe):

Checklist

• Code updated to current develop branch head
• Passes CI checks
• Is a part of an issue
• Tests added for the bugfix or newly implemented feature, describe below why if not
• Changelog is updated
• Documentation of code and features exists

Changes

• Added room auth request that exchanges the provided access code or token for a room auth token
  • The room auth token is used in further API requests to authenticate the user against the room
• Removed the generation and usage of short live links to download files
  • Instead the room auth token is passed in the file access link

Summary by CodeRabbit

• New Features

  • Room authentication now supports session-scoped auth tokens and personalized room links.
  • File access now uses signed URLs and a terms-of-use flow for safer downloads.

• Bug Fixes

  • Fixed file downloads being blocked in strict-browser scenarios.

• UI / Documentation

  • “Tokens” renamed to “Personalized links” across the UI and translations.
  • Improved error pages and clearer authentication/error messages; settings labels updated.

[coderabbitai]

Actionable comments posted: 4

🤖 Fix all issues with AI agents

In `@tests/Backend/Feature/api/v1/Room/FileTest.php`:
- Around line 200-206: The test is passing the wrong identifier for room auth
tokens: replace uses of $roomAuthToken->id with the actual token value (e.g.,
$roomAuthToken->token or $roomAuthToken->getKey()) when populating the
'room_auth_token' route parameter (seen in the getJson call to
route('api.v1.rooms.files.get') and other occurrences), and update any related
assertions or ordering logic that assumed an integer id to use the token (and
room_id where applicable) since the room_tokens table uses a composite primary
key (token, room_id).

In `@tests/Backend/Feature/api/v1/Room/RoomTest.php`:
- Around line 3189-3207: The second POST to route('api.v1.rooms.start') is
missing the authenticated context and is testing a guest path; update the call
so it uses the same authenticated actor as the first request by chaining
actingAs($this->user) before postJson (keep the same payload and room_auth_token
parameters), ensuring the test exercises the authenticated flow involving
startNewSession, RoomAuthToken::factory(...) and RoomAuthTokenType::CODE.
- Around line 3360-3379: The failing assertions are using unauthenticated
requests for the "authorized user with invalid token" paths; update the two
postJson calls after the first to be authenticated by chaining
->actingAs($this->user) (same as the first request) so they run as the intended
user, keeping the route('api.v1.rooms.start', [...]) params and the body payload
unchanged and preserving the assertions that expect ROOM_INVALID_AUTH_TOKEN via
CustomErrorMessages and RoomAuthTokenType::CODE references.

In `@tests/Frontend/e2e/RoomsViewMeetings.cy.js`:
- Line 2271: Update the inline comment in the test "RoomsViewMeetings.cy.js"
that currently reads "// Check thet room auth token is reset" to correct the
typo to "// Check that room auth token is reset" so the comment is grammatically
correct and clear; locate the comment near the test assertion that verifies the
room auth token reset and replace "thet" with "that".

🧹 Nitpick comments (3)

tests/Backend/Feature/api/v1/RecordingTest.php (1)

711-743: Consider using a constant for the unauthorized message.

The error message 'This action is unauthorized.' is hardcoded while other error assertions in this file use the CustomErrorMessages enum (e.g., CustomErrorMessages::ROOM_REQUIRE_CODE->value).

If this is Laravel's default authorization message, consider either:

1. Extracting it to a constant for maintainability
2. Adding a comment explaining this is Laravel's default message

This improves consistency and makes future message changes easier to track.

♻️ Example refactor

-            ->assertJsonFragment(['message' => 'This action is unauthorized.']);
+            ->assertJsonFragment(['message' => __('This action is unauthorized.')]);

Or if a constant exists/should be added:

-            ->assertJsonFragment(['message' => 'This action is unauthorized.']);
+            ->assertJsonFragment(['message' => CustomErrorMessages::UNAUTHORIZED->value]);

tests/Frontend/e2e/RoomsViewRecordingsRecordingActions.cy.js (1)

131-134: Scope room GET intercepts to avoid overlapping recordings routes.

The wildcard pattern api/v1/rooms/abc-def-123* matches recording paths like /api/v1/rooms/abc-def-123/recordings/... because minimatch treats * as matching any character including /. In Cypress, when multiple intercepts match the same request, the most recently defined one runs first and stops propagation if it returns a response. If the room intercept is defined after a recordings intercept, it will intercept recording requests incorrectly. Constraining the room intercept to its exact pathname prevents this order-dependent brittleness.

♻️ Suggested pattern (apply similarly to all room GET intercepts)

- cy.intercept("GET", "api/v1/rooms/abc-def-123*", {
+ cy.intercept(
+   { method: "GET", pathname: "/api/v1/rooms/abc-def-123" },
+   {
      statusCode: 200,
      body: room,
- }).as("roomRequest");
+   },
+ ).as("roomRequest");

Also applies to: 232-235, 260-263, 299-302, 327-330, 370-373, 457-460

tests/Frontend/e2e/RoomsViewMeetings.cy.js (1)

822-825: Consider using a consistent assertion style for verifying tokens are cleared.

At lines 888-890, you use .to.be.undefined checks which explicitly verify the query parameters don't exist. Here you use .to.not.contain() which would also pass if the keys exist with different values. For consistency and precision, consider using the same approach throughout.

Suggested change for consistency

-    cy.wait("@roomRequest").then((interception) => {
-      expect(interception.request.query).to.not.contain({
-        room_auth_token: "roomAuthToken",
-        room_auth_token_type: "0",
-      });
-    });
+    cy.wait("@roomRequest").then((interception) => {
+      expect(interception.request.query.room_auth_token).to.be.undefined;
+      expect(interception.request.query.room_auth_token_type).to.be.undefined;
+    });

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

resources/js/components/RoomMembershipButton.vue (1)

120-134: ⚠️ Potential issue | 🟠 Major

Missing null check on error.response before accessing its properties.

If a network error occurs (e.g., timeout, DNS failure), error.response will be undefined, causing a TypeError at line 123. Consider guarding the access:

Proposed fix

     .catch((error) => {
-      // Access code invalid
-      if (
+      if (!error.response) {
+        api.error(error, { redirectOnUnauthenticated: false });
+        return;
+      }
+
+      // Room auth token invalid
+      if (
         error.response.status === env.HTTP_UNAUTHORIZED &&
         error.response.data.message === HTTP_ROOM_INVALID_AUTH_TOKEN
       ) {

resources/js/components/RoomJoinButton.vue (1)

364-364: ⚠️ Potential issue | 🟡 Minor

Stale comment: still references accessCode and token.

The comment on line 364 says "add accessCode and token if needed" but the code now uses roomAuthToken.

Proposed fix

-  // Build url, add accessCode and token if needed
+  // Build url, add room auth token if needed

🤖 Fix all issues with AI agents

In `@app/Http/Controllers/api/v1/RoomController.php`:
- Around line 493-496: Add a new enum case to CustomStatusCodes:
ROOM_GUESTS_ONLY = 420, then replace the raw literal 420 in RoomController (the
abort call inside the authenticated check) with
CustomStatusCodes::ROOM_GUESTS_ONLY->value; also update other occurrences that
expect the "Guests only" convention (e.g., RoomAuthenticate.php and related
tests) to use the new CustomStatusCodes::ROOM_GUESTS_ONLY->value for
consistency.

🧹 Nitpick comments (5)

resources/js/components/RoomMembershipButton.vue (1)

108-113: Auth token sent as query parameters on a POST request.

Passing room_auth_token and room_auth_token_type as query params means the token will appear in server access logs, browser history, and potentially Referer headers. For a POST request, consider sending these in the request body instead, which is the more conventional and safer approach.

resources/js/components/RoomJoinButton.vue (1)

270-275: Auth tokens passed as query parameters — same concern as RoomMembershipButton.

Both loadStartJoinRequirements (OPTIONS) and getJoinUrl (POST) send the room auth token as query params. For the OPTIONS request this is the only option, but for the POST at line 377–382, the token could be placed in the request body to avoid URL-based leakage.

Also applies to: 377-382

app/Http/Controllers/api/v1/RoomController.php (2)

442-442: Inconsistent Auth facade usage.

Line 442 uses \Illuminate\Support\Facades\Auth::user() while the rest of the method (and file) uses the Auth alias imported at Line 31. Use Auth::user() consistently.

Proposed fix

-        if (\Illuminate\Support\Facades\Auth::user() && ($room->owner->is(Auth::user()) || $room->members->contains(Auth::user()) || Auth::user()->can('viewAll', Room::class))) {
+        if (Auth::user() && ($room->owner->is(Auth::user()) || $room->members->contains(Auth::user()) || Auth::user()->can('viewAll', Room::class))) {

---

461-467: Rate limit key is not scoped to the room.

The key 'room_auth:'.($request->user()?->id ?: $request->ip()) is global across all rooms. A user who hits the rate limit on one room will be locked out from authenticating to any room. This may be intentional (stricter security), but if the intent is per-room limiting, include $room->id in the key.

app/Http/Controllers/api/v1/RoomPersonalizedLinkController.php (1)

104-118: Consider scoped route-model binding instead of manual ownership checks.

Lines 106–108 and 129–131 both manually verify $link->room->is($room). Laravel's scoped implicit binding (Room $room, RoomPersonalizedLink:room $link in route definition) can handle this automatically, reducing boilerplate and the risk of forgetting the check in future methods.