Skip to content
Open
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
13 changes: 13 additions & 0 deletions Explorer/Assets/DCL/Communities/CommunitiesFeatureAccess.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,19 @@ public async UniTask<bool> IsUserAllowedToUseTheFeatureAsync(CancellationToken c
return result;
}

/// <summary>
/// Synchronous counterpart of <see cref="IsUserAllowedToUseTheFeatureAsync" />: returns the cached
/// result of the last completed check, or false while no check has resolved for the current identity.
/// </summary>
public bool IsUserAllowedCached()
{
#if UNITY_EDITOR && !COMMUNITIES_FORCE_USER_WHITELIST
return true;
#else
return storedResult ?? false;
#endif
}

public bool TryGetCommunityIdFromAppArgs(out string? communityId) =>
appArgs.TryGetValue(AppArgsFlags.COMMUNITY, out communityId) && !string.IsNullOrEmpty(communityId);

Expand Down
6 changes: 0 additions & 6 deletions Explorer/Assets/DCL/ExplorePanel/ExplorePanelController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ public class ExplorePanelController : ControllerBase<ExplorePanelView, ExplorePa
private readonly bool includeDiscover;
private readonly HttpEventsApiService eventsApiService;
private readonly JoinedCommunitiesVoiceLiveTracker communitiesLiveTracker;
private readonly ICreditsPanelController creditsPanelController;
private bool includeCommunities;

private ReactivePropertyExtensions.DisposableSubscription<bool>? communitiesLiveBadgeSubscription;
Expand All @@ -61,7 +60,6 @@ public class ExplorePanelController : ControllerBase<ExplorePanelView, ExplorePa
private ExploreSections lastShownSection;
private bool isControlClosing;

private CommunitiesBrowserController communitiesBrowserController { get; }
public NavmapController NavmapController { get; }
public CameraReelController CameraReelController { get; }
public SettingsController SettingsController { get; }
Expand All @@ -71,8 +69,6 @@ public class ExplorePanelController : ControllerBase<ExplorePanelView, ExplorePa

public override CanvasOrdering.SortingLayer Layer => CanvasOrdering.SortingLayer.FULLSCREEN;

public bool CanBeClosedByEscape => State != ControllerState.ViewShowing;

public event Action? PlacesOpenedFromStartMenu;
public event Action? EventsOpenedFromStartMenu;

Expand Down Expand Up @@ -103,11 +99,9 @@ public ExplorePanelController(ViewFactoryMethod viewFactory,
this.inputBlock = inputBlock;
this.includeCameraReel = FeaturesRegistry.Instance.IsEnabled(FeatureId.CAMERA_REEL);
this.mvcManager = mvcManager;
this.communitiesBrowserController = communitiesBrowserController;
this.includeDiscover = FeaturesRegistry.Instance.IsEnabled(FeatureId.DISCOVER);
this.eventsApiService = eventsApiService;
this.communitiesLiveTracker = communitiesLiveTracker;
this.creditsPanelController = creditsPanelController;
CommunitiesBrowserController = communitiesBrowserController;
PlacesController = placesController;

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
using Cysharp.Threading.Tasks;
using DCL.Communities;
using DCL.CrdtEcsBridge.JsModulesImplementation;
using DCL.Diagnostics;
using DCL.ExplorePanel;
using DCL.UI;
using Decentraland.Kernel.Apis;
using MVC;
using System;

namespace DCL.Infrastructure.CrdtEcsBridge.JsModulesImplementation.RestrictedActions
{
/// <summary>
/// Implementation of <see cref="IExplorerUiActions" />. The sibling asmref compiles it into
/// DCL.Social (not into SceneRuntime like the rest of this folder) because it references
/// <see cref="ExplorePanelController" />, and DCL.Social already depends on SceneRuntime.
/// </summary>
public class ExplorerUiActions : IExplorerUiActions
{
private readonly IMVCManager mvcManager;

private bool isExplorePanelOpen;

public ExplorerUiActions(IMVCManager mvcManager)
{
this.mvcManager = mvcManager;
mvcManager.OnViewShowed += OnViewShowed;
mvcManager.OnViewClosed += OnViewClosed;
}

public void Dispose()
{
mvcManager.OnViewShowed -= OnViewShowed;
mvcManager.OnViewClosed -= OnViewClosed;
}

public OpenExplorerUiResult OpenSection(ExploreSections section)
{
// Communities availability depends on the user identity (feature flag + wallets allowlist),
// so it cannot be gated through FeaturesRegistry like the other sections.
if (section == ExploreSections.Communities && !CommunitiesFeatureAccess.Instance.IsUserAllowedCached())
{
ReportHub.Log(ReportCategory.RESTRICTED_ACTIONS, "OpenSection: the Communities feature is not available for this user");
return OpenExplorerUiResult.RejectedFeatureDisabled;
}

if (isExplorePanelOpen)
return OpenExplorerUiResult.WasAlreadyOpen;

OpenSectionAsync(section).Forget();
return OpenExplorerUiResult.Opened;
Comment on lines +47 to +51

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Race in isExplorePanelOpen guard. Between the isExplorePanelOpen check (line 47) and OnViewShowed firing, a rapid second call passes the guard and queues a duplicate ShowAsync. Both return Opened to the scene. The MVC manager handles double-show gracefully (TC6 documents both verdicts as acceptable), so this isn't a functional bug, but setting the flag eagerly would make the WasAlreadyOpen verdict more accurate.

Suggested change
if (isExplorePanelOpen)
return OpenExplorerUiResult.WasAlreadyOpen;
OpenSectionAsync(section).Forget();
return OpenExplorerUiResult.Opened;
if (isExplorePanelOpen)
return OpenExplorerUiResult.WasAlreadyOpen;
isExplorePanelOpen = true;
OpenSectionAsync(section).Forget();
return OpenExplorerUiResult.Opened;

If ShowAsync fails before OnViewShowed fires, reset the flag in the catch blocks of OpenSectionAsync:

catch (OperationCanceledException) { isExplorePanelOpen = false; }
catch (Exception e)
{
    isExplorePanelOpen = false;
    ReportHub.LogException(e, ReportCategory.RESTRICTED_ACTIONS);
}

}

private async UniTask OpenSectionAsync(ExploreSections section)
{
try
{
await UniTask.SwitchToMainThread();
await mvcManager.ShowAsync(ExplorePanelController.IssueCommand(new ExplorePanelParameter(section)));
}
catch (OperationCanceledException) { }
catch (Exception e) { ReportHub.LogException(e, ReportCategory.RESTRICTED_ACTIONS); }
}

private void OnViewShowed(IController controller)
{
if (controller is ExplorePanelController)
isExplorePanelOpen = true;
}

private void OnViewClosed(IController controller)
{
if (controller is ExplorePanelController)
isExplorePanelOpen = false;
}
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"reference": "GUID:f56000518aba31544aa96f4739e50a64"
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
using DCL.UI;
using Decentraland.Kernel.Apis;
using System;

namespace DCL.CrdtEcsBridge.JsModulesImplementation
{
public interface IExplorerUiActions : IDisposable
{
OpenExplorerUiResult OpenSection(ExploreSections section);
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,20 @@
using DCL.AvatarRendering.Emotes;
using DCL.ChangeRealmPrompt;
using DCL.Clipboard;
using DCL.CrdtEcsBridge.JsModulesImplementation;
using DCL.Diagnostics;
using DCL.ECSComponents;
using DCL.FeatureFlags;
using DCL.UI;
using Utility.Arch;
using DCL.ExternalUrlPrompt;
using DCL.NftPrompt;
using DCL.SceneRuntime.Apis.RestrictedActionsApi;
using DCL.TeleportPrompt;
using DCL.Utilities;
using Decentraland.Kernel.Apis;
using MVC;
using SceneRunner.Scene;
using SceneRuntime.Apis.Modules.RestrictedActionsApi;
using SceneRuntime.ScenePermissions;
using System;
using System.Threading;
Expand All @@ -22,12 +26,15 @@ namespace CrdtEcsBridge.RestrictedActions
{
public class RestrictedActionsAPIImplementation : IRestrictedActionsAPI
{
private const uint USER_GESTURE_WINDOW_TICKS = 1;

private readonly IMVCManager mvcManager;
private readonly ISceneStateProvider sceneStateProvider;
private readonly IGlobalWorldActions globalWorldActions;
private readonly ISceneData sceneData;
private readonly IJsApiPermissionsProvider permissionsProvider;
private readonly ISystemClipboard systemClipboard;
private readonly IExplorerUiActions explorerUiActions;
private readonly Arch.Core.World sceneWorld;
private readonly Arch.Core.Entity scenePlayerEntity;

Expand All @@ -39,7 +46,8 @@ public RestrictedActionsAPIImplementation(
IJsApiPermissionsProvider permissionsProvider,
ISystemClipboard systemClipboard,
Arch.Core.World sceneWorld,
Arch.Core.Entity scenePlayerEntity)
Arch.Core.Entity scenePlayerEntity,
IExplorerUiActions explorerUiActions)
{
this.mvcManager = mvcManager;
this.sceneStateProvider = sceneStateProvider;
Expand All @@ -49,6 +57,7 @@ public RestrictedActionsAPIImplementation(
this.systemClipboard = systemClipboard;
this.sceneWorld = sceneWorld;
this.scenePlayerEntity = scenePlayerEntity;
this.explorerUiActions = explorerUiActions;
}

public bool TryOpenExternalUrl(string url)
Expand Down Expand Up @@ -200,6 +209,41 @@ public bool TryOpenNftDialog(string urn)
return true;
}

public int TryOpenExplorerUi(int ui)
{
if (!sceneStateProvider.IsCurrent)
return (int)OpenExplorerUiResult.RejectedNotCurrentScene;

// Underflow-safe recent-gesture check: never subtract unsigned ticks. The "== 0" guard is
// load-bearing — a scene where no user input has ever been recorded must be rejected.
uint lastUserInputTick = sceneStateProvider.LastUserInputTick;

if (lastUserInputTick == 0 || lastUserInputTick + USER_GESTURE_WINDOW_TICKS < sceneStateProvider.TickNumber)
{
ReportHub.Log(ReportCategory.RESTRICTED_ACTIONS, "OpenExplorerUi: rejected, call did not originate from a recent user gesture");
return (int)OpenExplorerUiResult.RejectedNoUserGesture;
}

if (!TryMapExplorerUi((ExplorerUi)ui, out ExploreSections section, out FeatureId? gatingFeature))
{
ReportHub.LogWarning(ReportCategory.RESTRICTED_ACTIONS, $"OpenExplorerUi: unsupported ui value '{ui}'");
return (int)OpenExplorerUiResult.RejectedFeatureDisabled;
}

if (gatingFeature.HasValue && !FeaturesRegistry.Instance.IsEnabled(gatingFeature.Value))
{
ReportHub.Log(ReportCategory.RESTRICTED_ACTIONS, $"OpenExplorerUi: feature '{gatingFeature.Value}' is disabled");
return (int)OpenExplorerUiResult.RejectedFeatureDisabled;
}

return (int)explorerUiActions.OpenSection(section);
}

public void Dispose()
{
explorerUiActions.Dispose();
}

public void TryCopyToClipboard(string text)
{
if (!sceneStateProvider.IsCurrent)
Expand Down Expand Up @@ -253,5 +297,52 @@ private async UniTask CopyToClipboardAsync(string text)
await UniTask.SwitchToMainThread();
systemClipboard.Set(text);
}

/// <summary>
/// Maps a protocol <see cref="ExplorerUi" /> value to its explore panel section and, when the
/// section is behind a synchronous feature flag, the <see cref="FeatureId" /> that gates it.
/// Returns false for unknown values so the caller can reject the request.
/// </summary>
private static bool TryMapExplorerUi(ExplorerUi ui, out ExploreSections section, out FeatureId? gatingFeature)
{
switch (ui)
{
case ExplorerUi.EuMap:
section = ExploreSections.Navmap;
gatingFeature = null;
return true;
case ExplorerUi.EuSettings:
section = ExploreSections.Settings;
gatingFeature = null;
return true;
case ExplorerUi.EuBackpack:
section = ExploreSections.Backpack;
gatingFeature = null;
return true;
case ExplorerUi.EuCameraReel:
section = ExploreSections.CameraReel;
gatingFeature = FeatureId.CAMERA_REEL;
return true;
case ExplorerUi.EuCommunities:
section = ExploreSections.Communities;
// FeatureId.COMMUNITIES is never populated in FeaturesRegistry (its state is
// identity-dependent), so the gate lives in the IExplorerUiActions implementation,
// which can reach CommunitiesFeatureAccess.
gatingFeature = null;
return true;
case ExplorerUi.EuPlaces:
section = ExploreSections.Places;
gatingFeature = FeatureId.DISCOVER;
return true;
case ExplorerUi.EuEvents:
section = ExploreSections.Events;
gatingFeature = FeatureId.DISCOVER;
return true;
default:
section = default(ExploreSections);
gatingFeature = null;
return false;
}
}
}
}
Loading
Loading