PE-9210: A share dialog that hangs, keys shown in the clear, and reads that never time out

lib/blocs/drive_attach/drive_attach_cubit.dart

@@ -90,6 +90,37 @@ class DriveAttachCubit extends Cubit<DriveAttachState> {
             );
           }
 
+          // A private share link no longer carries the drive's name - it is a
+          // secret, and it is recoverable from the drive's own record once the
+          // key is in hand. Resolve it here so the auto-attach below still
+          // has one.
+          //
+          // Deliberately resolved *before* `submit()` rather than inside it:
+          // `submit()` reads the name controller up front so that a name the
+          // user typed by hand is the one the drive is attached under, and
+          // moving that read later would silently discard it.
+          if (driveNameController.text.isEmpty &&
+              driveKeyController.text.isNotEmpty) {
+            // Guarded: this runs inside a microtask started from the
+            // constructor whose future nobody awaits, so a network or decode
+            // failure in here would otherwise surface as an unhandled
+            // asynchronous error. A name that will not resolve is not fatal -
+            // the auto-submit below is simply skipped and the form stays open
+            // for the recipient to act on, which is what a keyless link
+            // already does.
+            try {
+              await driveNameLoader();
+            } catch (e, stacktrace) {
+              logger.e(
+                'Failed to resolve the name of the shared drive',
+                e,
+                stacktrace,
+              );
+            }
+
+            if (isClosed) return;
+          }
+
           if (driveNameController.text.isNotEmpty &&
               driveKeyController.text.isNotEmpty) {
             submit();

lib/blocs/drive_share/drive_share_cubit.dart

@@ -2,6 +2,9 @@ import 'package:ardrive/blocs/blocs.dart';
 import 'package:ardrive/core/crypto/crypto.dart';
 import 'package:ardrive/models/models.dart';
 import 'package:ardrive/utils/link_generators.dart';
+import 'package:ardrive/utils/logger.dart';
+import 'package:ardrive_utils/ardrive_utils.dart';
+import 'package:arweave/utils.dart';
 import 'package:equatable/equatable.dart';
 import 'package:flutter/foundation.dart';
 import 'package:flutter_bloc/flutter_bloc.dart';
@@ -12,11 +15,41 @@ part 'drive_share_state.dart';
 class DriveShareCubit extends Cubit<DriveShareState> {
   final Drive drive;
 
+  /// The folder being shared, or `null` when the whole drive is.
+  final FolderID? folderId;
+
+  /// The folder's own name, for the dialog to confirm what is being shared.
+  final String? folderName;
+
   final ProfileCubit _profileCubit;
   final DriveDao _driveDao;
 
+  /// Whether the built link embeds the drive key.
+  ///
+  /// Starts off. See [generatePrivateDriveShareLink] for why a drive key is
+  /// handed over separately by default.
+  bool _keyIsInLink = false;
+
+  /// Which load is current. A load that is not the newest by the time it
+  /// finishes has been overtaken and must not emit - the same generation guard
+  /// [SharedFileCubit] uses.
+  int _generation = 0;
+
+  /// The drive's key, resolved once.
+  ///
+  /// Toggling the checkbox rebuilds the link from what is already in hand: it
+  /// must not go back to the database for a key that cannot have changed, and
+  /// it must not blank the dialog to a spinner - which would take the very
+  /// control the sharer just clicked off the screen. The file share dialog
+  /// rebuilds its link with no loading state at all, and this one should feel
+  /// the same. Only populated on success, so a retry after a failure resolves
+  /// afresh.
+  DriveKey? _driveKeyCache;
+
   DriveShareCubit({
     required this.drive,
+    this.folderId,
+    this.folderName,
     required DriveDao driveDao,
     required ProfileCubit profileCubit,
   })  : _driveDao = driveDao,
@@ -25,40 +58,121 @@ class DriveShareCubit extends Cubit<DriveShareState> {
     loadDriveShareDetails();
   }
 
-  Future<void> loadDriveShareDetails() async {
-    late Uri driveShareLink;
-    emit(DriveShareLoadInProgress());
-
-    if (drive.isPrivate) {
-      DriveKey? driveKey;
-      if (_profileCubit.state is ProfileLoggedIn) {
-        final profileKey =
-            (_profileCubit.state as ProfileLoggedIn).user.cipherKey;
-        driveKey = await _driveDao.getDriveKey(drive.id, profileKey);
-      } else {
-        driveKey = await _driveDao.getDriveKeyFromMemory(drive.id);
-      }
-      if (driveKey != null) {
-        driveShareLink = await generatePrivateDriveShareLink(
-          driveId: drive.id,
-          driveName: drive.name,
-          driveKey: driveKey.key,
-        );
-      } else {
-        throw StateError('Drive key not found');
+  /// Rebuilds the link with or without the key embedded in it.
+  Future<void> setKeyIsInLink(bool value) async {
+    if (value == _keyIsInLink) {
+      return;
+    }
+
+    _keyIsInLink = value;
+
+    // No progress state: this is a rebuild, not a load.
+    await _buildShareDetails(announceProgress: false);
+  }
+
+  /// Builds the share link for [drive], or fails in a way the dialog can show.
+  ///
+  /// Everything here runs inside the guard on purpose. This method is called
+  /// from the constructor body and its future is never awaited, so anything it
+  /// throws becomes an unhandled asynchronous error: the cubit would stay in
+  /// [DriveShareLoadInProgress] and the dialog would spin forever, which is
+  /// exactly what a missing drive key used to do.
+  Future<void> loadDriveShareDetails() =>
+      _buildShareDetails(announceProgress: true);
+
+  Future<void> _buildShareDetails({required bool announceProgress}) async {
+    final generation = ++_generation;
+
+    // Captured once. Read twice - once to build the link and once to describe
+    // it - two loads racing after a fast double-toggle could emit a link built
+    // with the key in it while labelling it keyless, and the dialog would then
+    // render a key-bearing link unmasked.
+    final keyIsInLink = _keyIsInLink;
+
+    if (announceProgress) {
+      emit(DriveShareLoadInProgress());
+    }
+
+    try {
+      final driveKey = drive.isPrivate ? await _driveKey() : null;
+
+      final driveShareLink = driveKey == null
+          ? generatePublicDriveShareLink(
+              driveId: drive.id,
+              driveName: drive.name,
+              folderId: folderId,
+            )
+          : await generatePrivateDriveShareLink(
+              driveId: drive.id,
+              driveKey: driveKey.key,
+              folderId: folderId,
+              includeKey: keyIsInLink,
+            );
+
+      // Extracted before the guard so that no await sits between the checks
+      // below and the emit.
+      final driveKeyBase64 = driveKey == null
+          ? null
+          : encodeBytesToBase64(await driveKey.key.extractBytes());
+
+      if (isClosed || generation != _generation) {
+        return;
       }
-    } else {
-      driveShareLink = generatePublicDriveShareLink(
-        driveId: drive.id,
-        driveName: drive.name,
+
+      emit(
+        DriveShareLoadSuccess(
+          drive: drive,
+          driveShareLink: driveShareLink,
+          isFolder: folderId != null,
+          folderName: folderName,
+          keyIsInLink: keyIsInLink,
+          // The key is offered as its own artifact so the sharer can send it
+          // through a different channel than the link.
+          driveKeyBase64: driveKeyBase64,
+        ),
+      );
+    } catch (e, stacktrace) {
+      // The drive id is safe to log; the key never is, and nothing here puts
+      // one in the message.
+      logger.e(
+        'Failed to build the share link for drive ${drive.id}',
+        e,
+        stacktrace,
       );
+
+      if (isClosed || generation != _generation) {
+        return;
+      }
+
+      emit(const DriveShareLoadFail());
+    }
+  }
+
+  /// The private drive's key, which the link and the handover both need.
+  ///
+  /// It comes from the profile when one is signed in, and from the in-memory
+  /// store otherwise - a drive attached in this session but never persisted.
+  /// Neither is guaranteed, and a [StateError] here is a real outcome rather
+  /// than a should-never-happen: it lands on the failure state above.
+  Future<DriveKey> _driveKey() async {
+    final cached = _driveKeyCache;
+
+    if (cached != null) {
+      return cached;
+    }
+
+    final profileState = _profileCubit.state;
+
+    final driveKey = profileState is ProfileLoggedIn
+        ? await _driveDao.getDriveKey(drive.id, profileState.user.cipherKey)
+        : await _driveDao.getDriveKeyFromMemory(drive.id);
+
+    if (driveKey == null) {
+      throw StateError('Drive key not found');
     }
 
-    emit(
-      DriveShareLoadSuccess(
-        drive: drive,
-        driveShareLink: driveShareLink,
-      ),
-    );
+    _driveKeyCache = driveKey;
+
+    return driveKey;
   }
 }

lib/blocs/drive_share/drive_share_state.dart

@@ -5,7 +5,7 @@ abstract class DriveShareState extends Equatable {
   const DriveShareState();
 
   @override
-  List<Object> get props => [];
+  List<Object?> get props => [];
 }
 
 /// [DriveShareLoadInProgress] means that the drive share details are being loaded.
@@ -18,23 +18,58 @@ class DriveShareLoadSuccess extends DriveShareState {
   /// The link to share access of this drive with.
   final Uri driveShareLink;
 
+  /// Whether the link points at one folder rather than the whole drive.
+  final bool isFolder;
+
+  /// The shared folder's name, when a folder is what is being shared.
+  final String? folderName;
+
+  /// Whether the drive key is embedded in [driveShareLink].
+  final bool keyIsInLink;
+
+  /// The drive key, for the sharer to hand over separately.
+  ///
+  /// `null` for a public drive, which has none.
+  final String? driveKeyBase64;
+
   const DriveShareLoadSuccess({
     required this.drive,
     required this.driveShareLink,
+    this.isFolder = false,
+    this.folderName,
+    this.keyIsInLink = false,
+    this.driveKeyBase64,
   });
 
+  /// Whether the key travels as its own artifact rather than inside the link.
+  bool get hasSeparateKeyArtifact => driveKeyBase64 != null && !keyIsInLink;
+
   @override
-  List<Object> get props => [drive, driveShareLink];
+  List<Object?> get props => [
+        drive,
+        driveShareLink,
+        isFolder,
+        folderName,
+        keyIsInLink,
+        driveKeyBase64,
+      ];
+
+  /// Equatable stringifies [props] in debug builds, and one of them is a drive
+  /// key. Same redaction, for the same reason, as [FileShareLoadSuccess].
+  ///
+  /// The link is printed whole: it is only a secret when the sharer embedded
+  /// the key in it, and [keyIsInLink] says when that is.
+  @override
+  String toString() => 'DriveShareLoadSuccess(drive: ${drive.id}, '
+      'isFolder: $isFolder, keyIsInLink: $keyIsInLink, '
+      'driveKey: ${driveKeyBase64 == null ? 'none' : '<redacted>'})';
 }
 
 /// [DriveShareLoadFail] shows failiure states in the UI.
+///
+/// Carries no message: the dialog owns the copy so that it can be localized,
+/// which a cubit with no [BuildContext] cannot do. This is the same shape the
+/// file share dialog's failure states use.
 class DriveShareLoadFail extends DriveShareState {
-  final String message;
-
-  const DriveShareLoadFail({
-    required this.message,
-  });
-
-  @override
-  List<Object> get props => [message];
+  const DriveShareLoadFail();
 }

lib/blocs/file_share/file_share_cubit.dart

@@ -184,6 +184,10 @@ class FileShareCubit extends Cubit<FileShareState> {
     _emitLoadSuccess();
   }
 
+  /// How long the one network read on this side may take before the link is
+  /// presented as final without `c`/`iv`.
+  static const _cipherDetailsTimeout = Duration(seconds: 10);
+
   /// Fetches `c`/`iv` - the only two link fields that are not in the local
   /// database - and folds them into the link when they arrive.
   ///
@@ -192,7 +196,15 @@ class FileShareCubit extends Cubit<FileShareState> {
   /// link built before this schema does today (§1.2).
   Future<void> _loadCipherDetails(String dataTxId) async {
     try {
-      final dataTx = await _arweave.getTransactionDetails(dataTxId);
+      // Bounded. `GraphQLRetry` retries a call that throws but sets no
+      // timeout, so a gateway that simply hangs would leave the dialog saying
+      // "finishing your link" for as long as it stayed open - telling the
+      // sharer to wait for something that is already done. The link is
+      // complete and copyable without `c`/`iv`; those two fields only save the
+      // recipient one lookup.
+      final dataTx = await _arweave
+          .getTransactionDetails(dataTxId)
+          .timeout(_cipherDetailsTimeout);
 
       if (isClosed) {
         return;