diff --git a/lib/blocs/drive_attach/drive_attach_cubit.dart b/lib/blocs/drive_attach/drive_attach_cubit.dart index eac15b4ec2..4dc25e00b1 100644 --- a/lib/blocs/drive_attach/drive_attach_cubit.dart +++ b/lib/blocs/drive_attach/drive_attach_cubit.dart @@ -90,8 +90,16 @@ class DriveAttachCubit extends Cubit { ); } - if (driveNameController.text.isNotEmpty && - driveKeyController.text.isNotEmpty) { + // The key is the whole precondition. A private share link no longer + // carries the drive's name - it is a secret, and `submit()` resolves + // the real one from the drive's own record on its way through. + // + // Gating this on the name as well used to mean that a link whose key + // was wrong, or whose drive could not be found, simply sat there: + // auto-attach was skipped and nothing said why. Letting `submit()` + // run surfaces those through the states it already emits - + // `DriveAttachInvalidDriveKey` and `DriveAttachDriveNotFound`. + if (driveKeyController.text.isNotEmpty) { submit(); } } @@ -105,7 +113,12 @@ class DriveAttachCubit extends Cubit { void submit() async { final driveId = driveIdController.text; - final driveName = driveNameController.text; + + // Read before the loaders below overwrite the field with the drive's own + // name. A name typed by hand is the one the drive is attached under; an + // empty one is not a preference, so it falls back to whatever the chain + // says once the loaders have run. + final typedDriveName = driveNameController.text; try { final previousState = state; @@ -155,6 +168,10 @@ class DriveAttachCubit extends Cubit { return; } + final driveName = typedDriveName.isNotEmpty + ? typedDriveName + : driveNameController.text; + await _driveDao.writeDriveEntity( name: driveName, entity: driveEntity, diff --git a/lib/blocs/drive_share/drive_share_cubit.dart b/lib/blocs/drive_share/drive_share_cubit.dart index 094f0bf90e..dc46647a68 100644 --- a/lib/blocs/drive_share/drive_share_cubit.dart +++ b/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 { 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 { loadDriveShareDetails(); } - Future 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 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 loadDriveShareDetails() => + _buildShareDetails(announceProgress: true); + + Future _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() 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; } } diff --git a/lib/blocs/drive_share/drive_share_state.dart b/lib/blocs/drive_share/drive_share_state.dart index b42a60dfda..4680a5a916 100644 --- a/lib/blocs/drive_share/drive_share_state.dart +++ b/lib/blocs/drive_share/drive_share_state.dart @@ -5,7 +5,7 @@ abstract class DriveShareState extends Equatable { const DriveShareState(); @override - List get props => []; + List 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 get props => [drive, driveShareLink]; + List 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' : ''})'; } /// [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 get props => [message]; + const DriveShareLoadFail(); } diff --git a/lib/blocs/file_download/shared_file_download_cubit.dart b/lib/blocs/file_download/shared_file_download_cubit.dart index 068aab7797..5e68a7af61 100644 --- a/lib/blocs/file_download/shared_file_download_cubit.dart +++ b/lib/blocs/file_download/shared_file_download_cubit.dart @@ -9,8 +9,19 @@ class SharedFileDownloadCubit extends FileDownloadCubit { final ArDriveDownloader _arDriveDownloader; final DownloadPolicy _downloadPolicy; + /// The cipher the share link named, when it named one. + /// + /// Supplied only for a transaction the caller knows is bundled - see + /// `_downloadFile` for why that qualification is load bearing. + final String? cipher; + + /// The cipher IV the share link named. Travels with [cipher] or not at all. + final String? cipherIv; + SharedFileDownloadCubit({ this.fileKey, + this.cipher, + this.cipherIv, required this.revision, required ArweaveService arweave, required ArDriveCrypto crypto, @@ -85,17 +96,33 @@ class SharedFileDownloadCubit extends FileDownloadCubit { } if (fileKey != null && !isPinFile) { - // Private/encrypted files need cipher/IV tags from the data transaction - final dataTx = await _arweave.getTransactionDetails(dataTxId); + if (cipher != null && cipherIv != null) { + // The link already said what this transaction is encrypted with, so + // the lookup that would have asked is skipped entirely. Carrying `c` + // and `iv` in a share link only pays for itself here - before this, + // every private download re-fetched tags the link had already + // delivered, which on a rate-limited connection is a call that can + // fail outright. + // + // `verifyDownload` stays false, and correctly so: it turns on the + // arweave client's chunk check for L1 transactions, and the caller + // only supplies these tags for a data item it knows is bundled - + // which is not an L1 transaction and has no chunks to check. + cipherTag = cipher; + cipherIvTag = cipherIv; + } else { + // Private/encrypted files need cipher/IV tags from the data transaction + final dataTx = await _arweave.getTransactionDetails(dataTxId); + + if (dataTx == null) { + throw StateError( + 'Data transaction not found for file ${revision.id} with txId $dataTxId from gateway ${_arweave.client.api.gatewayUrl.origin}'); + } - if (dataTx == null) { - throw StateError( - 'Data transaction not found for file ${revision.id} with txId $dataTxId from gateway ${_arweave.client.api.gatewayUrl.origin}'); + cipherTag = dataTx.getTag(EntityTag.cipher); + cipherIvTag = dataTx.getTag(EntityTag.cipherIv); + verifyDownload = dataTx.getTag(EntityTag.appName) == 'ArDrive-CLI'; } - - cipherTag = dataTx.getTag(EntityTag.cipher); - cipherIvTag = dataTx.getTag(EntityTag.cipherIv); - verifyDownload = dataTx.getTag(EntityTag.appName) == 'ArDrive-CLI'; } logger.d('File size: ${revision.size}'); diff --git a/lib/blocs/file_share/file_share_cubit.dart b/lib/blocs/file_share/file_share_cubit.dart index fb98153c8c..19f0ce0e4f 100644 --- a/lib/blocs/file_share/file_share_cubit.dart +++ b/lib/blocs/file_share/file_share_cubit.dart @@ -184,6 +184,10 @@ class FileShareCubit extends Cubit { _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 { /// link built before this schema does today (§1.2). Future _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; diff --git a/lib/blocs/shared_file/shared_file_cubit.dart b/lib/blocs/shared_file/shared_file_cubit.dart index 341ca92376..50948b6519 100644 --- a/lib/blocs/shared_file/shared_file_cubit.dart +++ b/lib/blocs/shared_file/shared_file_cubit.dart @@ -100,6 +100,55 @@ class SharedFileCubit extends Cubit { Future? _backgroundWork; + /// The longest a single network read here may take before it is abandoned. + /// + /// The data path has been bounded for a long time - [DataGatewayFallback] + /// gives every fetch a request timeout, a total timeout and a hedge. The + /// GraphQL reads that run *in front of* it had nothing: [GraphQLRetry] + /// retries a call that fails, but sets no timeout, so a connection that + /// errors is retried and a connection that simply hangs is not. This page + /// would sit on its skeleton forever. + /// + /// Sized well above a healthy read and well below a recipient's patience. + /// Anything that trips it lands in the load failure state, which already + /// offers Retry. + static const defaultReadTimeout = Duration(seconds: 15); + + /// The budget for the version history, which is a different kind of read. + /// + /// Every other read here fetches one thing and gates what the recipient sees; + /// this one walks the file's whole revision history, is asked for only when + /// the recipient opens the drawer, and blocks nothing while it runs. Holding + /// it to the first-paint budget made a file with real history - or a slow + /// gateway - report "version history unavailable" where it used to simply + /// take a while. + static const defaultHistoryTimeout = Duration(minutes: 2); + + final Duration _readTimeout; + + final Duration _historyTimeout; + + /// Bounds [future], naming [what] so a timeout is legible in the log. + /// + /// A [TimeoutException] is deliberately left to propagate: every caller on + /// the critical path already handles a failed read, either by degrading to + /// another resolution path or by emitting the failure state. + Future _bounded( + Future future, + String what, { + Duration? timeout, + }) { + final budget = timeout ?? _readTimeout; + + return future.timeout( + budget, + onTimeout: () => throw TimeoutException( + 'Timed out after ${budget.inSeconds}s while $what', + budget, + ), + ); + } + SharedFileCubit({ required this.fileId, this.fileKey, @@ -109,10 +158,14 @@ class SharedFileCubit extends Cubit { required licenseService, ArDriveCrypto? crypto, Duration propagationRetryDelay = const Duration(seconds: 3), + Duration readTimeout = defaultReadTimeout, + Duration historyTimeout = defaultHistoryTimeout, }) : _arweave = arweave, _licenseService = licenseService, _crypto = crypto ?? ArDriveCrypto(), _propagationRetryDelay = propagationRetryDelay, + _readTimeout = readTimeout, + _historyTimeout = historyTimeout, // A v2 link can paint its skeleton with the real name and size before // a single byte has been fetched. super(SharedFileLoadInProgress(payload: linkPayload)) { @@ -276,9 +329,10 @@ class SharedFileCubit extends Cubit { emit(current.copyWith(activityStatus: SharedFileActivityStatus.loading)); try { - final entities = await _arweave.getAllFileEntitiesWithId( - fileId, - fileKey, + final entities = await _bounded( + _arweave.getAllFileEntitiesWithId(fileId, fileKey), + 'reading the file\'s version history', + timeout: _historyTimeout, ); if (_isStale(resolution)) { @@ -357,7 +411,10 @@ class SharedFileCubit extends Cubit { FileEntity? latest; try { - latest = await _arweave.getLatestFileEntityWithId(fileId, fileKey); + latest = await _bounded( + _arweave.getLatestFileEntityWithId(fileId, fileKey), + 'checking for a newer revision', + ); } catch (e, stacktrace) { logger.e( 'Failed to load the newest revision of the shared file', @@ -485,15 +542,18 @@ class SharedFileCubit extends Cubit { // The link names the exact revision that was shared, so the key is // tried against it directly: one lookup by transaction id instead of an // owner probe followed by a latest-revision query. - final shared = await _fetchSharedRevision(metadataTxId, fileKey); + final shared = await _bounded( + _fetchSharedRevision(metadataTxId, fileKey), + 'trying the key against the revision the link names', + ); if (shared == null) { // The link's metadata transaction is not on the network. The key may // still be perfectly good, so fall back to resolving the file the // long way rather than blaming the key. - final file = await _arweave.getLatestFileEntityWithId( - fileId, - fileKey, + final file = await _bounded( + _arweave.getLatestFileEntityWithId(fileId, fileKey), + 'looking up the newest revision to try the key against', ); if (file == null) { @@ -596,7 +656,10 @@ class SharedFileCubit extends Cubit { _SharedRevision? shared; try { - shared = await _fetchSharedRevision(metadataTxId, fileKey); + shared = await _bounded( + _fetchSharedRevision(metadataTxId, fileKey), + 'reading the metadata transaction the link names', + ); } on EntityTransactionParseException { if (_isStale(resolution)) { return true; @@ -725,7 +788,10 @@ class SharedFileCubit extends Cubit { emit(SharedFileLoadInProgress(payload: linkPayload)); } - final privacy = await _arweave.getFilePrivacyForId(fileId); + final privacy = await _bounded( + _arweave.getFilePrivacyForId(fileId), + 'looking up whether the file is private', + ); if (_isStale(resolution)) { return; @@ -735,9 +801,9 @@ class SharedFileCubit extends Cubit { _emitLocked(linkPayload); return; } - final allEntities = await _arweave.getAllFileEntitiesWithId( - fileId, - fileKey, + final allEntities = await _bounded( + _arweave.getAllFileEntitiesWithId(fileId, fileKey), + 'reading the file\'s revisions', ); if (_isStale(resolution)) { @@ -764,9 +830,9 @@ class SharedFileCubit extends Cubit { // revisions are in reverse chronological order, so first is most recent final target = _targetRevision(revisions); final latestLicense = target.licenseTxId != null - ? await fetchLicenseForRevision( - target, - owner: ownerAddress, + ? await _bounded( + fetchLicenseForRevision(target, owner: ownerAddress), + 'reading the file\'s license', ) : null; diff --git a/lib/components/copyable_share_artifact.dart b/lib/components/copyable_share_artifact.dart new file mode 100644 index 0000000000..4bcf66afb4 --- /dev/null +++ b/lib/components/copyable_share_artifact.dart @@ -0,0 +1,169 @@ +import 'package:ardrive/components/copy_button.dart'; +import 'package:ardrive_ui/ardrive_ui.dart'; +import 'package:flutter/material.dart'; + +/// A read-only field holding one artifact of a share handover, with its own +/// copy affordance. +/// +/// The link and the key are copied one at a time, on purpose - a private file +/// is handed over as two artifacts meant to travel through different channels. +/// +/// ## Why [isSecret] masks by default +/// +/// The recipient typing an access key gets an obscured field +/// (`shared_file_locked_view.dart`). The sharer handing that key out used to +/// get it rendered in full, which is backwards: **typing a key is a private +/// act, but handing one over is the moment a screen is most likely to be +/// shared, recorded or screenshotted.** Anything that carries key material - +/// the key itself, or a link with the key embedded - is masked here until the +/// sharer deliberately reveals it. +/// +/// Masking never blocks the common path: Copy puts [text] on the clipboard, not +/// what the field displays, so it works while the field is showing dots. +/// +/// ## Why the controller lives here +/// +/// [text] is the whole input. An earlier version took a controller and left +/// each dialog to fill it from a bloc listener, which silently broke for any +/// cubit that reached its success state synchronously: a listener does not fire +/// for the state a bloc is already in, so the field rendered empty. Owning the +/// controller means the field always shows what it was given. +class CopyableShareArtifact extends StatefulWidget { + const CopyableShareArtifact({ + super.key, + required this.label, + required this.text, + required this.copyLabel, + required this.revealLabel, + this.isSecret = false, + }); + + final String label; + + /// The value displayed, and the value Copy puts on the clipboard. + final String text; + + final String copyLabel; + + /// The accessible name of the reveal control, which is icon-only. + final String revealLabel; + + /// Whether this artifact carries key material, and so starts masked behind + /// a reveal toggle. + final bool isSecret; + + @override + State createState() => _CopyableShareArtifactState(); +} + +class _CopyableShareArtifactState extends State { + /// The reveal control's footprint, reserved on every row. + /// + /// Only a secret has something to reveal, but the slot is held open either + /// way: without it, the field beside a reveal button is narrower than the one + /// without, so a dialog showing a link above a key rendered two boxes of + /// visibly different widths. + static const _revealSlotWidth = 36.0; + + late final TextEditingController _controller = + TextEditingController(text: widget.text); + + bool _isRevealed = false; + + @override + void didUpdateWidget(covariant CopyableShareArtifact oldWidget) { + super.didUpdateWidget(oldWidget); + + if (oldWidget.text != widget.text) { + _controller.text = widget.text; + + // A revealed secret stays revealed only for as long as it is the same + // secret. Ticking "include the key in the link" swaps a keyless link for + // one that carries the key, and that new value must not inherit the + // previous one's revealed state. + _isRevealed = false; + } + } + + @override + void dispose() { + _controller.dispose(); + super.dispose(); + } + + @override + Widget build(BuildContext context) { + final typography = ArDriveTypographyNew.of(context); + final colorTokens = ArDriveTheme.of(context).themeData.colorTokens; + final isMasked = widget.isSecret && !_isRevealed; + + return Row( + crossAxisAlignment: CrossAxisAlignment.center, + children: [ + Expanded( + child: ArDriveTextFieldNew( + // [ArDriveTextFieldNew] copies `obscureText` into its own state in + // `initState` and never syncs it again, so toggling the property + // on a mounted field does nothing. The key forces a fresh one - + // the same trick, for the same reason, that the file share dialog + // uses on its checkbox. Safe here because the field is read only + // and its value lives on the controller, so a remount loses + // nothing. + key: ValueKey(isMasked), + label: widget.label, + controller: _controller, + isEnabled: false, + obscureText: isMasked, + // Deliberately not the field's own `showObfuscationToggle`: it + // renders inside the decoration of a disabled field, which + // swallows the tap, so the mask could never be lifted. Asserted by + // `test/components/copyable_share_artifact_test.dart`. + showObfuscationToggle: false, + ), + ), + const SizedBox(width: 8), + SizedBox( + width: _revealSlotWidth, + child: widget.isSecret + // Material's `IconButton` rather than the `GestureDetector` + + // `ArDriveClickArea` pair used elsewhere: this control has no + // visible label, so it needs the keyboard focus and the announced + // name that a raw gesture detector does not provide. + ? IconButton( + icon: isMasked + ? ArDriveIcons.eyeClosed(color: colorTokens.textMid) + : ArDriveIcons.eyeOpen(color: colorTokens.textMid), + onPressed: () => setState(() => _isRevealed = !_isRevealed), + tooltip: widget.revealLabel, + splashRadius: 18, + padding: EdgeInsets.zero, + constraints: const BoxConstraints.tightFor( + width: _revealSlotWidth, + height: _revealSlotWidth, + ), + ) + : null, + ), + const SizedBox(width: 12), + CopyButton( + positionX: 4, + positionY: 40, + copyMessageColor: colorTokens.containerRed, + showCopyText: true, + text: widget.text, + child: Text( + widget.copyLabel, + style: typography + .paragraphNormal( + fontWeight: ArFontWeight.semiBold, + color: colorTokens.textMid, + ) + .copyWith( + decoration: TextDecoration.underline, + ), + ), + ), + ], + ); + } +} diff --git a/lib/components/details_panel.dart b/lib/components/details_panel.dart index 4f7b5ea743..2ec973ba54 100644 --- a/lib/components/details_panel.dart +++ b/lib/components/details_panel.dart @@ -1170,6 +1170,9 @@ class DetailsPanelToolbar extends StatelessWidget { @override Widget build(BuildContext context) { final drive = driveDetailLoadSuccess.currentDrive; + // A local so that type promotion works on it: a widget field cannot be + // promoted, and the share icon has to ask a folder whether it is a ghost. + final item = this.item; return Container( padding: const EdgeInsets.symmetric(vertical: 12), @@ -1190,7 +1193,12 @@ class DetailsPanelToolbar extends StatelessWidget { const SizedBox( width: 16, ), - if (item is FileDataTableItem || item is DriveDataItem) + if (item is FileDataTableItem || + item is DriveDataItem || + // Not a ghost folder: its metadata was never found, so a link + // naming it points the recipient at nothing. The two dropdown + // menus gate this the same way. + (item is FolderDataTableItem && !item.isGhostFolder)) _buildActionIcon( tooltip: _getShareTooltip(item, context), icon: ArDriveIcons.share(size: defaultIconSize), @@ -1201,6 +1209,16 @@ class DetailsPanelToolbar extends StatelessWidget { driveId: item.driveId, fileId: item.id, ); + } else if (item is FolderDataTableItem) { + // A folder link is a drive link that names a folder, and it + // carries the drive key for the same reason - a folder in a + // private drive cannot be read without it. + promptToShareDrive( + context: context, + drive: drive, + folderId: item.id, + folderName: item.name, + ); } else if (item is DriveDataItem) { promptToShareDrive( context: context, @@ -1216,15 +1234,15 @@ class DetailsPanelToolbar extends StatelessWidget { if (item is FileDataTableItem) { promptToDownloadProfileFile( context: context, - file: item as FileDataTableItem, + file: item, ); } else if (item is FolderDataTableItem) { promptToDownloadMultipleFiles(context, - selectedItems: [item as FolderDataTableItem], + selectedItems: [item], zipName: item.name); } else if (item is DriveDataItem) { promptToDownloadMultipleFiles(context, - selectedItems: [item as DriveDataItem], + selectedItems: [item], zipName: item.name); } }), @@ -1234,7 +1252,7 @@ class DetailsPanelToolbar extends StatelessWidget { icon: ArDriveIcons.newWindow(size: defaultIconSize), onTap: () { final bloc = context.read(); - bloc.launchPreview((item as FileDataTableItem).dataTxId); + bloc.launchPreview(item.dataTxId); }, ), if (isDriveOwner(context.read(), drive.ownerAddress)) @@ -1303,6 +1321,8 @@ class DetailsPanelToolbar extends StatelessWidget { String _getShareTooltip(ArDriveDataTableItem item, BuildContext context) { if (item is FileDataTableItem) { return appLocalizationsOf(context).shareFile; + } else if (item is FolderDataTableItem) { + return appLocalizationsOf(context).shareFolder; } else if (item is DriveDataItem) { return appLocalizationsOf(context).shareDrive; } else { diff --git a/lib/components/drive_attach_form.dart b/lib/components/drive_attach_form.dart index f1d8d0c046..81db655e42 100644 --- a/lib/components/drive_attach_form.dart +++ b/lib/components/drive_attach_form.dart @@ -155,7 +155,7 @@ class _DriveAttachFormState extends State { const LinearProgressIndicator(), const SizedBox(height: 4), Text( - 'Looking up drive...', + appLocalizationsOf(context).attachDriveLookingUp, style: ArDriveTypographyNew.of(context) .paragraphSmall( color: ArDriveTheme.of(context) @@ -169,6 +169,21 @@ class _DriveAttachFormState extends State { ); }, ), + if (state is DriveAttachPrivate) ...[ + const SizedBox(height: 8), + Align( + alignment: Alignment.centerLeft, + child: Text( + appLocalizationsOf(context).attachDrivePrivateFound, + style: ArDriveTypographyNew.of(context).paragraphSmall( + color: ArDriveTheme.of(context) + .themeData + .colorTokens + .textLow, + ), + ), + ), + ], const SizedBox(height: 16), if (state is DriveAttachPrivate) ArDriveTextFieldNew( diff --git a/lib/components/drive_share_dialog.dart b/lib/components/drive_share_dialog.dart index cadb25fdf4..959526da88 100644 --- a/lib/components/drive_share_dialog.dart +++ b/lib/components/drive_share_dialog.dart @@ -1,5 +1,5 @@ import 'package:ardrive/blocs/blocs.dart'; -import 'package:ardrive/components/copy_button.dart'; +import 'package:ardrive/components/copyable_share_artifact.dart'; import 'package:ardrive/models/models.dart'; import 'package:ardrive/theme/theme.dart'; import 'package:ardrive/utils/app_localizations_wrapper.dart'; @@ -8,15 +8,24 @@ import 'package:ardrive_ui/ardrive_ui.dart'; import 'package:flutter/material.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; +/// Shares [drive], or - when [folderId] is given - one folder inside it. +/// +/// [folderName] is what the dialog shows the sharer they are sharing. Without +/// it a folder share is described by its *drive's* name, which is the wrong +/// thing to confirm before copying a link. Future promptToShareDrive({ required BuildContext context, required Drive drive, + String? folderId, + String? folderName, }) => showArDriveDialog( context, content: BlocProvider( create: (_) => DriveShareCubit( drive: drive, + folderId: folderId, + folderName: folderName, driveDao: context.read(), profileCubit: context.read(), ), @@ -33,24 +42,19 @@ class DriveShareDialog extends StatefulWidget { } class DriveShareDialogState extends State { - final shareLinkController = TextEditingController(); - - @override - void initState() { - super.initState(); - } - @override Widget build(BuildContext context) => BlocBuilder( builder: (context, state) { final typography = ArDriveTypographyNew.of(context); + final colorTokens = ArDriveTheme.of(context).themeData.colorTokens; return ArDriveStandardModalNew( width: kLargeDialogWidth, - title: appLocalizationsOf(context).shareDriveWithOthers, - description: - state is DriveShareLoadSuccess ? state.drive.name : null, + title: state is DriveShareLoadSuccess && state.isFolder + ? appLocalizationsOf(context).shareFolderWithOthers + : appLocalizationsOf(context).shareDriveWithOthers, + scrollableContent: true, content: SizedBox( width: kLargeDialogWidth, child: Column( @@ -60,66 +64,115 @@ class DriveShareDialogState extends State { if (state is DriveShareLoadInProgress) const Center(child: CircularProgressIndicator()) else if (state is DriveShareLoadSuccess) ...{ - Row( - crossAxisAlignment: CrossAxisAlignment.center, - children: [ - Flexible( - child: Container( - padding: const EdgeInsets.fromLTRB(20, 14, 20, 14), - decoration: BoxDecoration( - color: ArDriveTheme.of(context) - .themeData - .colorTokens - .inputDisabled, - borderRadius: BorderRadius.circular(6), - border: Border.all( - color: ArDriveTheme.of(context) - .themeData - .colorTokens - .strokeMid, - ), - ), - child: Row( - children: [ - Expanded( - child: Text( - state.driveShareLink.toString(), - style: typography.paragraphNormal( - color: ArDriveTheme.of(context) - .themeData - .colorTokens - .textXLow, - fontWeight: ArFontWeight.semiBold, - ), - maxLines: 1, - overflow: TextOverflow.ellipsis, - ), - ), - const SizedBox(width: 8), - CopyButton( - text: state.driveShareLink.toString(), - ), - ], - ), - ), + // What is being shared, confirmed before the sharer copies + // anything. Not the modal's `description`, which it only + // renders when there is no `content` - so this dialog never + // showed one. + Padding( + padding: const EdgeInsets.only(bottom: 16), + child: Text( + state.folderName ?? state.drive.name, + style: typography.paragraphNormal( + fontWeight: ArFontWeight.semiBold, + color: colorTokens.textHigh, ), - ], + ), + ), + CopyableShareArtifact( + label: appLocalizationsOf(context).shareFileLinkLabel, + text: state.driveShareLink.toString(), + copyLabel: appLocalizationsOf(context).copyLink, + revealLabel: + appLocalizationsOf(context).shareDriveRevealLink, + // Only a link the sharer chose to embed the key in holds + // a secret. A keyless link - the default - is not worth + // hiding, and hiding it would suggest it is dangerous to + // share, which is the opposite of the point. + isSecret: state.keyIsInLink, ), - const SizedBox(height: 16), + if (state.hasSeparateKeyArtifact) ...{ + const SizedBox(height: 16), + CopyableShareArtifact( + label: appLocalizationsOf(context) + .shareDriveAccessKeyLabel, + text: state.driveKeyBase64!, + copyLabel: appLocalizationsOf(context).copyAccessKey, + revealLabel: + appLocalizationsOf(context).shareFileRevealKey, + isSecret: true, + ), + _HelperText( + appLocalizationsOf(context).shareDriveSendKeySeparately, + color: colorTokens.textLow, + ), + }, + if (state.drive.isPrivate) ...{ + const SizedBox(height: 20), + ArDriveCheckBox( + // The checkbox only reads `checked` when it is first + // built, so the key forces a fresh one whenever the + // cubit's answer changes. + key: ValueKey(state.keyIsInLink), + checked: state.keyIsInLink, + title: appLocalizationsOf(context) + .shareDriveIncludeKeyInLink, + titleStyle: typography.paragraphNormal( + color: colorTokens.textMid, + ), + onChange: (value) => + context.read().setKeyIsInLink( + value, + ), + ), + if (state.keyIsInLink) + _HelperText( + appLocalizationsOf(context) + .shareDriveKeyInLinkWarning, + color: colorTokens.strokeRed, + ), + }, + const SizedBox(height: 20), Text( + // A keyless private link does *not* grant access on its + // own, so it must not be described as though it does - + // that copy predates the key ever being optional. state.drive.isPublic ? appLocalizationsOf(context) .anyoneCanAccessThisDrivePublic - : appLocalizationsOf(context) - .anyoneCanAccessThisDrivePrivate, - style: typography.paragraphLarge(), + : state.keyIsInLink + ? appLocalizationsOf(context) + .anyoneCanAccessThisDrivePrivate + : appLocalizationsOf(context) + .shareDriveKeylessNotice, + // Guidance, not the headline. As `paragraphLarge` it was + // the largest text in a dialog whose actual subject is + // the two fields above it. + style: typography.paragraphNormal( + color: colorTokens.textMid, + ), ), } else if (state is DriveShareLoadFail) - Text(state.message) + Text( + appLocalizationsOf(context).shareDriveFailure, + style: typography.paragraphNormal( + color: colorTokens.textMid, + ), + ), ], ), ), actions: [ + if (state is DriveShareLoadFail) ...[ + ModalAction( + action: () => Navigator.pop(context), + title: appLocalizationsOf(context).cancel, + ), + ModalAction( + action: () => + context.read().loadDriveShareDetails(), + title: appLocalizationsOf(context).tryAgain, + ), + ], if (state is DriveShareLoadSuccess) ModalAction( action: () => Navigator.pop(context), @@ -130,3 +183,23 @@ class DriveShareDialogState extends State { }, ); } + +/// A line of guidance under the control it belongs to. +/// +/// Same treatment as the file share dialog's, so the two dialogs read as one +/// design rather than two. +class _HelperText extends StatelessWidget { + const _HelperText(this.text, {required this.color}); + + final String text; + final Color color; + + @override + Widget build(BuildContext context) => Padding( + padding: const EdgeInsets.only(top: 8), + child: Text( + text, + style: ArDriveTypographyNew.of(context).paragraphSmall(color: color), + ), + ); +} diff --git a/lib/components/file_download_dialog.dart b/lib/components/file_download_dialog.dart index 103ca20c65..b05933efe8 100644 --- a/lib/components/file_download_dialog.dart +++ b/lib/components/file_download_dialog.dart @@ -103,6 +103,8 @@ Future promptToDownloadSharedFile({ required BuildContext context, SecretKey? fileKey, required ARFSFileEntity revision, + String? cipher, + String? cipherIv, }) { final cubit = SharedFileDownloadCubit( arDriveDownloader: ArDriveDownloader( @@ -113,6 +115,8 @@ Future promptToDownloadSharedFile({ crypto: ArDriveCrypto(), revision: revision, fileKey: fileKey, + cipher: cipher, + cipherIv: cipherIv, arweave: context.read(), ); return showArDriveDialog( diff --git a/lib/components/file_share_dialog.dart b/lib/components/file_share_dialog.dart index b98a27c168..c314813fd0 100644 --- a/lib/components/file_share_dialog.dart +++ b/lib/components/file_share_dialog.dart @@ -1,5 +1,5 @@ import 'package:ardrive/blocs/blocs.dart'; -import 'package:ardrive/components/copy_button.dart'; +import 'package:ardrive/components/copyable_share_artifact.dart'; import 'package:ardrive/models/models.dart'; import 'package:ardrive/services/services.dart'; import 'package:ardrive/theme/theme.dart'; @@ -42,28 +42,13 @@ class FileShareDialog extends StatefulWidget { } class FileShareDialogState extends State { - final shareLinkController = TextEditingController(); - final fileKeyController = TextEditingController(); - - @override - void dispose() { - shareLinkController.dispose(); - fileKeyController.dispose(); - super.dispose(); - } @override Widget build(BuildContext context) { final typography = ArDriveTypographyNew.of(context); final colorTokens = ArDriveTheme.of(context).themeData.colorTokens; - return BlocConsumer( - listener: (context, state) { - if (state is FileShareLoadSuccess) { - shareLinkController.text = state.fileShareLink.toString(); - fileKeyController.text = state.fileKeyBase64 ?? ''; - } - }, + return BlocBuilder( builder: (context, state) => ArDriveStandardModalNew( width: kLargeDialogWidth, scrollableContent: true, @@ -131,11 +116,14 @@ class FileShareDialogState extends State { ], ), ), - _CopyableArtifact( + CopyableShareArtifact( label: appLocalizationsOf(context).shareFileLinkLabel, - controller: shareLinkController, text: state.fileShareLink.toString(), copyLabel: appLocalizationsOf(context).copyLink, + revealLabel: appLocalizationsOf(context).shareDriveRevealLink, + // A link only holds a secret when the sharer chose to embed + // the key in it. A keyless link is not worth hiding. + isSecret: state.keyIsInLink, ), if (state.isLoadingCipherDetails) _HelperText( @@ -149,11 +137,12 @@ class FileShareDialogState extends State { ), if (state.hasSeparateKeyArtifact) ...[ const SizedBox(height: 16), - _CopyableArtifact( + CopyableShareArtifact( label: appLocalizationsOf(context).shareFileAccessKeyLabel, - controller: fileKeyController, text: state.fileKeyBase64!, copyLabel: appLocalizationsOf(context).copyAccessKey, + revealLabel: appLocalizationsOf(context).shareFileRevealKey, + isSecret: true, ), _HelperText( appLocalizationsOf(context).shareFileSendKeySeparately, @@ -233,65 +222,6 @@ class FileShareDialogState extends State { } } -/// A read-only field holding one of the artifacts of the handover, with its -/// own copy affordance - the link and the key are copied one at a time, on -/// purpose. -class _CopyableArtifact extends StatelessWidget { - const _CopyableArtifact({ - required this.label, - required this.controller, - required this.text, - required this.copyLabel, - }); - - final String label; - final TextEditingController controller; - - /// What the copy button puts on the clipboard. Taken from the state rather - /// than from [controller], which is only how the value is displayed. - final String text; - - final String copyLabel; - - @override - Widget build(BuildContext context) { - final typography = ArDriveTypographyNew.of(context); - final colorTokens = ArDriveTheme.of(context).themeData.colorTokens; - - return Row( - crossAxisAlignment: CrossAxisAlignment.center, - children: [ - Expanded( - child: ArDriveTextFieldNew( - label: label, - controller: controller, - isEnabled: false, - ), - ), - const SizedBox(width: 16), - CopyButton( - positionX: 4, - positionY: 40, - copyMessageColor: colorTokens.containerRed, - showCopyText: true, - text: text, - child: Text( - copyLabel, - style: typography - .paragraphNormal( - fontWeight: ArFontWeight.semiBold, - color: colorTokens.textMid, - ) - .copyWith( - decoration: TextDecoration.underline, - ), - ), - ), - ], - ); - } -} - class _HelperText extends StatelessWidget { const _HelperText(this.text, {required this.color}); diff --git a/lib/l10n/app_en.arb b/lib/l10n/app_en.arb index e29d5cd92a..8e408e7ca7 100644 --- a/lib/l10n/app_en.arb +++ b/lib/l10n/app_en.arb @@ -2334,6 +2334,63 @@ "@shareDriveWithOthers": { "description": "The action of sharing a drive link" }, + "shareFolderWithOthers": "Share folder with others", + "@shareFolderWithOthers": { + "description": "Title of the dialog that shares a link to one folder" + }, + "shareFolder": "Share folder", + "@shareFolder": { + "description": "The action of sharing a link to a folder" + }, + "shareDriveKeylessNotice": "The link alone won’t open this drive. The person you send it to also needs the access key above.", + "@shareDriveKeylessNotice": { + "description": "Explains that a private drive link without the key embedded needs the key sent separately" + }, + "shareDriveAccessKeyLabel": "Drive access key", + "@shareDriveAccessKeyLabel": { + "description": "Label of the field holding the drive key handed over alongside a private drive link" + }, + "shareDriveIncludeKeyInLink": "Include the key in the link", + "@shareDriveIncludeKeyInLink": { + "description": "Checkbox that embeds the drive key in the share link instead of handing it over separately" + }, + "shareDriveKeyInLinkWarning": "Anyone with this link can open every file in the drive, including files added later. A drive key cannot be changed.", + "@shareDriveKeyInLinkWarning": { + "description": "Warning shown when the sharer chooses to embed the drive key in the link" + }, + "shareDriveSendKeySeparately": "Send this key separately from the link.", + "@shareDriveSendKeySeparately": { + "description": "Helper text under the drive key field explaining the two-artifact handover" + }, + "attachDriveLookingUp": "Looking up drive…", + "@attachDriveLookingUp": { + "description": "Shown while the entered drive ID is being resolved" + }, + "attachDrivePrivateFound": "This is a private drive. Enter its access key to continue.", + "@attachDrivePrivateFound": { + "description": "Shown once a drive ID resolves to a private drive, so the user knows it was found and what is still needed" + }, + "attachDriveNameResolved": "Drive found: {driveName}", + "@attachDriveNameResolved": { + "description": "Confirms the drive whose name has been read off the chain", + "placeholders": { + "driveName": { + "type": "String" + } + } + }, + "shareDriveFailure": "We couldn’t create a share link for this drive. Check your connection and try again.", + "@shareDriveFailure": { + "description": "Shown in the share drive dialog when the link could not be built, most often because the drive key could not be read" + }, + "shareDriveRevealLink": "Show link", + "@shareDriveRevealLink": { + "description": "Accessibility label for the control that reveals a masked share link" + }, + "shareFileRevealKey": "Show access key", + "@shareFileRevealKey": { + "description": "Accessibility label for the control that reveals a masked access key" + }, "shareFailedFile": "The file you are attempting to share is invalid and cannot be shared.", "@shareFailedFile": { "description": "Failed file share dialog text" diff --git a/lib/pages/app_route_information_parser.dart b/lib/pages/app_route_information_parser.dart index 19494d1399..22d471ab92 100644 --- a/lib/pages/app_route_information_parser.dart +++ b/lib/pages/app_route_information_parser.dart @@ -78,21 +78,27 @@ class AppRouteInformationParser extends RouteInformationParser { final driveId = uri.pathSegments[1]; final name = uri.queryParameters['name']; final driveKeyBase64 = uri.queryParameters[driveKeyQueryParamName]; + + DriveKey? sharedDriveKey; + String? sharedRawDriveKey; + + // Decoded once, before the route shape is decided, so that the key + // reaches whichever route matches. This used to sit inside the drive + // branch and return from there, which meant a *valid* key made the + // folder segments unreachable: `/drives/{id}/folders/{fid}?driveKey=` + // silently resolved to the drive root and lost the folder. Only a + // damaged key ever reached the folder route, which is the wrong way + // round. if (driveKeyBase64 != null && driveKeyBase64.isNotEmpty) { try { - final sharedDrivePkBytes = - utils.decodeBase64ToBytes(driveKeyBase64); - - return AppRoutePath.driveDetail( - driveId: driveId, - driveName: name, - sharedDrivePk: DriveKey(SecretKey(sharedDrivePkBytes), true), - sharedRawDriveKey: driveKeyBase64, + sharedDriveKey = DriveKey( + SecretKey(utils.decodeBase64ToBytes(driveKeyBase64)), + true, ); + sharedRawDriveKey = driveKeyBase64; } catch (e) { - // Same as the shared file link below: a damaged key must not - // throw while the route is being parsed. Drop it and carry on to - // the keyless drive route. + // A damaged key must not throw while the route is being parsed. + // Drop it and carry on to the keyless route. // // The reason is logged and never the exception object: the // `source` of the `FormatException` a base64 decoder throws *is* @@ -107,12 +113,22 @@ class AppRouteInformationParser extends RouteInformationParser { if (uri.pathSegments.length == 2) { // Handle '/drives/:driveId' - return AppRoutePath.driveDetail(driveId: driveId, driveName: name); + return AppRoutePath.driveDetail( + driveId: driveId, + driveName: name, + sharedDrivePk: sharedDriveKey, + sharedRawDriveKey: sharedRawDriveKey, + ); } else if (uri.pathSegments.length == 4 && uri.pathSegments[2] == 'folders') { // Handle /drives/:driveId/folders/:folderId return AppRoutePath.folderDetail( - driveId: driveId, driveFolderId: uri.pathSegments[3]); + driveId: driveId, + driveFolderId: uri.pathSegments[3], + driveName: name, + sharedDrivePk: sharedDriveKey, + sharedRawDriveKey: sharedRawDriveKey, + ); } } @@ -207,21 +223,26 @@ class AppRouteInformationParser extends RouteInformationParser { uri: Uri.parse('/get-started'), ); } else if (configuration.driveId != null) { - if (configuration.driveName != null && - configuration.sharedRawDriveKey != null) { - return RouteInformation( - uri: Uri.parse( - '/drives/${configuration.driveId}?name=${configuration.driveName}' - '&$driveKeyQueryParamName=${configuration.sharedRawDriveKey}'), - ); - } + final path = configuration.driveFolderId == null + ? '/drives/${configuration.driveId}' + : '/drives/${configuration.driveId}' + '/folders/${configuration.driveFolderId}'; - return configuration.driveFolderId == null - ? RouteInformation(uri: Uri.parse('/drives/${configuration.driveId}')) - : RouteInformation( - uri: Uri.parse( - '/drives/${configuration.driveId}/folders/${configuration.driveFolderId}'), - ); + // Each part is written when it is there, rather than only when both are. + // A private drive link no longer carries a name, and the old shape wrote + // the key back *only* alongside one - so the key silently fell out of the + // address bar and a refresh asked the recipient for a key their link + // already had. + final query = [ + if (configuration.driveName != null) + 'name=${Uri.encodeQueryComponent(configuration.driveName!)}', + if (configuration.sharedRawDriveKey != null) + '$driveKeyQueryParamName=${configuration.sharedRawDriveKey}', + ]; + + return RouteInformation( + uri: Uri.parse(query.isEmpty ? path : '$path?${query.join('&')}'), + ); } else if (configuration.rawTransactionId != null) { return RouteInformation( uri: Uri.parse( diff --git a/lib/pages/app_route_path.dart b/lib/pages/app_route_path.dart index 56efb75014..40554746ed 100644 --- a/lib/pages/app_route_path.dart +++ b/lib/pages/app_route_path.dart @@ -99,11 +99,24 @@ class AppRoutePath { ); /// Creates a route that points to a folder in a particular drive. + /// + /// Carries the drive key for the same reason [driveDetail] does: a folder + /// inside a private drive cannot be read without it, and a link that names + /// one has nowhere else to put it. factory AppRoutePath.folderDetail({ required String driveId, required String driveFolderId, + String? driveName, + DriveKey? sharedDrivePk, + String? sharedRawDriveKey, }) => - AppRoutePath(driveId: driveId, driveFolderId: driveFolderId); + AppRoutePath( + driveId: driveId, + driveFolderId: driveFolderId, + driveName: driveName, + sharedDriveKey: sharedDrivePk, + sharedRawDriveKey: sharedRawDriveKey, + ); /// Creates a route that points to a particular shared file. factory AppRoutePath.sharedFile({ diff --git a/lib/pages/app_router_delegate.dart b/lib/pages/app_router_delegate.dart index 7648337be8..04f7f9d223 100644 --- a/lib/pages/app_router_delegate.dart +++ b/lib/pages/app_router_delegate.dart @@ -44,6 +44,62 @@ class AppRouterDelegate extends RouterDelegate String? driveName; String? driveFolderId; + /// The drive a folder link named, held until that drive is the selected one. + /// + /// A folder link opened by someone who does not have the drive yet goes + /// through the attach flow, which ends by clearing [driveId] so the prompt + /// cannot re-fire. The drive is then selected fresh, [driveFolderId] reads as + /// belonging to a different drive, and the folder the link named is dropped - + /// so every recipient who was not already in the drive landed at its root, + /// which made a folder link a drive link with extra characters. + /// + /// One shot: cleared as soon as that drive is selected, so ordinary + /// navigation away from the drive still discards the folder as it always has. + String? _pendingFolderDriveId; + + /// The folder that link named, held with the drive it belongs to. + /// + /// Kept alongside [_pendingFolderDriveId] rather than read off + /// [driveFolderId] when the drive arrives: between the link opening and that + /// drive being selected the recipient may have been somewhere else entirely, + /// and [driveFolderId] would by then hold a folder from whatever drive they + /// were last in. Restoring *this* is the difference between opening the + /// folder the link named and opening another drive's folder under this + /// drive's name. + String? _pendingFolderId; + + /// Reconciles the folder in view with the drive that has just been selected. + /// + /// Selecting a different drive discards the folder, which is what ordinary + /// navigation wants - a folder from the drive you just left is stale. The one + /// exception is the drive a folder link named: it becomes selected *because* + /// of that link, so its folder is the whole point rather than a leftover. + /// + /// Extracted from the listener that calls it so it can be tested without + /// standing up the whole app shell. + @visibleForTesting + void onDriveSelected(String? selectedDriveId) { + final selectedDriveChanged = driveId != selectedDriveId; + + final isTheLinkedDrive = _pendingFolderDriveId != null && + _pendingFolderDriveId == selectedDriveId; + + if (isTheLinkedDrive) { + // The folder the link named, not the one in view - see [_pendingFolderId]. + driveFolderId = _pendingFolderId; + + // One shot. Released as soon as it is honored, so navigating away from + // the drive and back lands at its root rather than jumping to the old + // folder. + _pendingFolderDriveId = null; + _pendingFolderId = null; + } else if (selectedDriveChanged) { + driveFolderId = null; + } + + driveId = selectedDriveId; + } + DriveKey? sharedDriveKey; String? sharedRawDriveKey; @@ -196,13 +252,7 @@ class AppRouterDelegate extends RouterDelegate shell = BlocListener( listener: (context, state) { if (state is DrivesLoadSuccess) { - final selectedDriveChanged = - driveId != state.selectedDriveId; - if (selectedDriveChanged) { - driveFolderId = null; - } - - driveId = state.selectedDriveId; + onDriveSelected(state.selectedDriveId); notifyListeners(); } }, @@ -386,6 +436,9 @@ class AppRouterDelegate extends RouterDelegate driveId = configuration.driveId; driveName = configuration.driveName; driveFolderId = configuration.driveFolderId; + _pendingFolderDriveId = + configuration.driveFolderId == null ? null : configuration.driveId; + _pendingFolderId = configuration.driveFolderId; sharedDriveKey = configuration.sharedDriveKey; sharedRawDriveKey = configuration.sharedRawDriveKey; sharedFileId = configuration.sharedFileId; @@ -404,6 +457,8 @@ class AppRouterDelegate extends RouterDelegate driveId = null; driveName = null; driveFolderId = null; + _pendingFolderDriveId = null; + _pendingFolderId = null; sharedDriveKey = null; sharedRawDriveKey = null; sharedFileId = null; diff --git a/lib/pages/drive_detail/components/drive_explorer_item_tile.dart b/lib/pages/drive_detail/components/drive_explorer_item_tile.dart index 44583050dd..777132665c 100644 --- a/lib/pages/drive_detail/components/drive_explorer_item_tile.dart +++ b/lib/pages/drive_detail/components/drive_explorer_item_tile.dart @@ -489,6 +489,26 @@ class _DriveExplorerItemTileTrailingState ), if (isOwner) hideFileDropdownItem(context, item), ], + // Not offered for a ghost folder: its metadata was never found, so a + // link naming it points the recipient at nothing. + if (!item.isGhostFolder) + ArDriveDropdownItem( + onClick: () { + promptToShareDrive( + context: context, + drive: widget.drive, + folderId: item.id, + folderName: item.name, + ); + }, + content: _buildItem( + appLocalizationsOf(context).shareFolder, + ArDriveIcons.share( + size: defaultIconSize, + ), + height: height, + ), + ), ArDriveDropdownItem( onClick: () { final bloc = context.read(); @@ -895,6 +915,25 @@ class EntityActionsMenu extends StatelessWidget { ), hideFileDropdownItem(context, item), ], + // Guarded rather than assumed: this menu is also built in places that + // do not know the drive, and a folder link needs it for the key. + if (drive != null && !item.isGhostFolder) + ArDriveDropdownItem( + onClick: () { + promptToShareDrive( + context: context, + drive: drive!, + folderId: item.id, + folderName: item.name, + ); + }, + content: _buildItem( + appLocalizationsOf(context).shareFolder, + ArDriveIcons.share( + size: defaultIconSize, + ), + ), + ), if (withInfo) _buildInfoOption(context), ]; } else if (item is DriveDataItem) { diff --git a/lib/pages/shared_file/shared_file_ready_view.dart b/lib/pages/shared_file/shared_file_ready_view.dart index 9954aafa56..e73221a7ae 100644 --- a/lib/pages/shared_file/shared_file_ready_view.dart +++ b/lib/pages/shared_file/shared_file_ready_view.dart @@ -298,6 +298,7 @@ class _SharedFileReadyViewState extends State { revision: revision, ownerAddress: state.ownerAddress ?? payload?.ownerAddress, licenseName: state.latestLicense?.meta.nameWithShortName, + detailsAreResolved: state.detailsAreResolved, ), SharedFileVersionsDrawer( revisions: state.activityRevisions, @@ -499,11 +500,32 @@ class _SharedFileReadyViewState extends State { Future _download(BuildContext context, FileRevision revision) async { setState(() => _isDownloading = true); + final payload = widget.state.payload; + + // The link's own cipher, handed to the download so it does not re-fetch + // tags the link already delivered - the reason `c` and `iv` are in the + // schema at all. + // + // Only for a data item the link says is bundled. `verifyDownload` turns on + // the arweave client's chunk check for L1 transactions and is decided by a + // tag on that same lookup, so skipping the lookup for something that might + // be L1 would drop a check without saying so. A bundled item is not an L1 + // transaction, so for those there is nothing to drop. + // + // It must also be *this* revision's cipher: the recipient may have moved + // the page to a newer one, whose bytes the link never described. + final linkDescribesTarget = payload != null && + payload.hasCipherDetails && + payload.bundledInTxId != null && + payload.dataTxId == revision.dataTxId; + try { await promptToDownloadSharedFile( revision: ARFSFactory().getARFSFileFromFileRevision(revision), context: context, fileKey: widget.state.fileKey, + cipher: linkDescribesTarget ? payload.cipher : null, + cipherIv: linkDescribesTarget ? payload.cipherIv : null, ); } finally { if (mounted) { @@ -838,20 +860,56 @@ class SharedFileDetailsDrawer extends StatelessWidget { required this.revision, this.ownerAddress, this.licenseName, + this.detailsAreResolved = true, }); final FileRevision revision; final String? ownerAddress; final String? licenseName; + /// Whether [revision] holds the file's own record rather than what the link + /// claimed. + /// + /// A v2 link carries no timestamps, so until the metadata resolves the dates + /// on [revision] are the epoch placeholder. Showing those would put + /// "1970-01-01" in front of a recipient as though it were the upload date, + /// which is worse than showing nothing. + final bool detailsAreResolved; + @override Widget build(BuildContext context) { final ownerAddress = this.ownerAddress; final licenseName = this.licenseName; + final contentType = revision.dataContentType; + return _SharedFileDrawer( title: appLocalizationsOf(context).sharedFileDetailsDrawerTitle, children: [ + // Type and dates lead, ahead of the identifiers. A recipient who was + // sent a link by a stranger has almost no way to judge what they are + // looking at, and *when it was uploaded* is the most useful signal + // they have - it was previously nowhere on this page, and the only + // date anywhere sat inside the version history, which is collapsed and + // not fetched until it is opened. + if (contentType != null && contentType.isNotEmpty) + _SharedFileDetailRow( + label: appLocalizationsOf(context).fileType, + value: contentType, + canCopy: false, + ), + if (detailsAreResolved) ...[ + _SharedFileDetailRow( + label: appLocalizationsOf(context).dateCreated, + value: formatDateToUtcString(revision.dateCreated), + canCopy: false, + ), + _SharedFileDetailRow( + label: appLocalizationsOf(context).lastUpdated, + value: formatDateToUtcString(revision.lastModifiedDate), + canCopy: false, + ), + ], _SharedFileDetailRow( label: appLocalizationsOf(context).fileID, value: revision.fileId, diff --git a/lib/utils/link_generators.dart b/lib/utils/link_generators.dart index 9a35c70f9f..edbddb8b19 100644 --- a/lib/utils/link_generators.dart +++ b/lib/utils/link_generators.dart @@ -12,25 +12,70 @@ import 'package:flutter/foundation.dart'; /// build points back at that preview build. Everywhere else, app.ardrive.io. String shareLinkOrigin() => kIsWeb ? Uri.base.origin : linkOriginProduction; +/// A share link for a public drive, or for one folder inside it. +/// +/// The name rides along because a public drive's name is not a secret and it +/// saves the recipient's client a lookup. See [generatePrivateDriveShareLink] +/// for why the private variant carries none. Uri generatePublicDriveShareLink({ required final DriveID driveId, required final String driveName, + final FolderID? folderId, }) { - final driveShareLink = - '${shareLinkOrigin()}/#/drives/$driveId?name=${Uri.encodeQueryComponent(driveName)}'; + final driveShareLink = '${shareLinkOrigin()}' + '${_driveLocation(driveId: driveId, folderId: folderId)}' + '?name=${Uri.encodeQueryComponent(driveName)}'; + return Uri.parse(driveShareLink); } +/// The route location of a drive, or of a folder within it. +String _driveLocation({required DriveID driveId, FolderID? folderId}) => + folderId == null + ? '/#/drives/$driveId' + : '/#/drives/$driveId/folders/$folderId'; + +/// A share link for a private drive. +/// +/// Deliberately carries no `name`. A private drive's name is a secret of +/// exactly the kind the file link schema added its `hid` flag to protect - a +/// drive called "Q4 Layoffs" leaks whether or not the key sits beside it, and +/// a URL is the least private place a string can live: browser history, the +/// address bar, screenshots, and every unfurl preview. +/// +/// Nothing is lost by omitting it. The recipient's attach flow resolves the +/// real name from the drive's own record as soon as the key is in hand +/// (`DriveAttachCubit.driveNameLoader`), so the name in the link was only ever +/// a pre-fill that the chain immediately overwrote. +/// [includeKey] embeds the drive key in the link. +/// +/// Off by default, and off is what the share dialog uses unless the sharer +/// opts in. A drive key opens every file and every folder name in the drive +/// for the life of the drive, and - unlike a password - **it cannot be +/// rotated**. A link that carries one is a link whose key is in every forward, +/// screenshot and unfurl of the message it travelled in. The keyless link and +/// the key are handed over as two artifacts, meant for two channels, exactly +/// as a private file's are. +/// +/// A recipient who opens a keyless link is not stuck: the attach form asks for +/// the key, validates it, and reads the drive's real name off the chain once +/// it has one. Future generatePrivateDriveShareLink({ required final DriveID driveId, - required final String driveName, required final SecretKey driveKey, + final FolderID? folderId, + final bool includeKey = false, }) async { + final location = + '${shareLinkOrigin()}${_driveLocation(driveId: driveId, folderId: folderId)}'; + + if (!includeKey) { + return Uri.parse(location); + } + final driveKeyBase64 = encodeBytesToBase64(await driveKey.extractBytes()); - return Uri.parse( - '${generatePublicDriveShareLink(driveName: driveName, driveId: driveId)}&driveKey=$driveKeyBase64', - ); + return Uri.parse('$location?driveKey=$driveKeyBase64'); } Uri generatePublicFileShareLink({ diff --git a/test/blocs/drive_share_cubit_test.dart b/test/blocs/drive_share_cubit_test.dart new file mode 100644 index 0000000000..9d38d1bc35 --- /dev/null +++ b/test/blocs/drive_share_cubit_test.dart @@ -0,0 +1,330 @@ +import 'package:ardrive/blocs/blocs.dart'; +import 'package:ardrive/core/crypto/crypto.dart'; +import 'package:ardrive/entities/profile_types.dart'; +import 'package:ardrive/models/models.dart'; +import 'package:ardrive/user/user.dart'; +import 'package:ardrive_utils/ardrive_utils.dart'; +import 'package:cryptography/cryptography.dart'; +import 'package:drift/drift.dart' show Value; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mocktail/mocktail.dart'; + +import '../test_utils/utils.dart'; + +void main() { + const driveId = 'a2b7ba0a-3b2a-4c1b-8a2f-6d1a0b3c4d5e'; + const rootFolderId = 'b3c8cb1b-4c3b-5d2c-9b3f-7e2b1c4d5e6f'; + const ownerAddress = 'fOVzBRTBnyt4VrUUYadBH8yras_-jhgpmNgg-5b3vEw'; + + late Database db; + late DriveDao driveDao; + late MockProfileCubit profileCubit; + + final profileKey = SecretKey(List.filled(32, 1)); + final driveKey = DriveKey(SecretKey(List.filled(32, 2)), true); + + /// Writes the drive row itself, with no encrypted key attached - which is + /// what [DriveDao.getDriveKey] reads to decide there is no key. + Future insertDrive({required bool isPrivate}) => + db.into(db.drives).insert( + DrivesCompanion.insert( + id: driveId, + name: 'My Drive', + ownerAddress: ownerAddress, + rootFolderId: rootFolderId, + privacy: isPrivate + ? DrivePrivacyTag.private + : DrivePrivacyTag.public, + lastBlockHeight: const Value(1), + ), + ); + + Future drive() => driveDao.driveById(driveId: driveId).getSingle(); + + DriveShareCubit cubit(Drive d, {String? folderId, String? folderName}) => + DriveShareCubit( + drive: d, + folderId: folderId, + folderName: folderName, + driveDao: driveDao, + profileCubit: profileCubit, + ); + + /// The cubit's first settled state. + /// + /// Checks [Cubit.state] before the stream because the public path is + /// entirely synchronous - the link needs no await, so the success is emitted + /// from the constructor before any listener can attach, and a cubit stream + /// does not replay. + Future settled(DriveShareCubit c) async { + if (c.state is! DriveShareLoadInProgress) { + return c.state; + } + + return c.stream + .firstWhere((s) => s is! DriveShareLoadInProgress) + .timeout(const Duration(seconds: 5)); + } + + void signedIn() => when(() => profileCubit.state).thenReturn( + ProfileLoggedIn( + user: User( + password: 'password', + wallet: getTestWallet(), + walletAddress: ownerAddress, + walletBalance: BigInt.one, + cipherKey: profileKey, + profileType: ProfileType.json, + ioTokens: 'ioTokens', + errorFetchingIOTokens: false, + ), + useTurbo: false, + ), + ); + + setUp(() { + db = getTestDb(); + driveDao = db.driveDao; + profileCubit = MockProfileCubit(); + signedIn(); + }); + + tearDown(() async => db.close()); + + group('DriveShareCubit', () { + test('a public drive resolves to a link with no key in it', () async { + await insertDrive(isPrivate: false); + + final state = await settled(cubit(await drive())); + + expect(state, isA()); + + final link = (state as DriveShareLoadSuccess).driveShareLink.toString(); + + expect(link, contains('/#/drives/$driveId')); + expect(link, isNot(contains('driveKey'))); + }); + + test( + 'a private drive with no reachable key fails instead of hanging - ' + 'the dialog used to spin forever on an unhandled StateError', () async { + // The drive row exists but carries no encrypted key, so `getDriveKey` + // returns null and the cubit throws `StateError('Drive key not found')`. + // That throw happens inside a future started from the constructor and + // never awaited: before the guard nothing caught it, the cubit stayed in + // `DriveShareLoadInProgress`, and the dialog showed a spinner with no + // way out. The timeout above is what makes "hangs" a failure rather than + // a hung test run. + await insertDrive(isPrivate: true); + + expect(await settled(cubit(await drive())), isA()); + }); + + test('a failure from the database is caught too', () async { + // No drive row at all, so `getDriveKey` throws out of `getSingle()` + // rather than returning null. The guard has to cover the unexpected + // failure as well as the expected one. + await insertDrive(isPrivate: true); + + final d = await drive(); + + await db.delete(db.drives).go(); + + expect(await settled(cubit(d)), isA()); + }); + + test('a failed load can be retried', () async { + await insertDrive(isPrivate: true); + + final c = cubit(await drive()); + + await c.stream.firstWhere((s) => s is DriveShareLoadFail); + + // Retry is the point of the failure state: the key may become reachable + // once the profile has finished loading. The expectation is armed before + // the call so that neither emission can be missed. + final expectation = expectLater( + c.stream, + emitsInOrder([ + isA(), + isA(), + ]), + ); + + await c.loadDriveShareDetails(); + await expectation; + }); + + test('a private drive link is keyless by default', () async { + // A drive key opens every file in the drive, for the life of the drive, + // and cannot be rotated. It is handed over as its own artifact unless + // the sharer deliberately embeds it. + when(() => profileCubit.state).thenReturn(ProfilePromptAdd()); + + await insertDrive(isPrivate: true); + await driveDao.putDriveKeyInMemory(driveID: driveId, driveKey: driveKey); + + final state = await settled(cubit(await drive())); + + expect(state, isA()); + + final success = state as DriveShareLoadSuccess; + + expect(success.driveShareLink.toString(), isNot(contains('driveKey'))); + expect(success.keyIsInLink, isFalse); + // The key is still offered, just not inside the link. + expect(success.hasSeparateKeyArtifact, isTrue); + expect(success.driveKeyBase64, isNotNull); + }); + + test('opting in embeds the key and retires the separate artifact', + () async { + when(() => profileCubit.state).thenReturn(ProfilePromptAdd()); + + await insertDrive(isPrivate: true); + await driveDao.putDriveKeyInMemory(driveID: driveId, driveKey: driveKey); + + final c = cubit(await drive()); + + await settled(c); + await c.setKeyIsInLink(true); + + final success = c.state as DriveShareLoadSuccess; + + expect(success.driveShareLink.toString(), contains('driveKey=')); + expect(success.keyIsInLink, isTrue); + expect(success.hasSeparateKeyArtifact, isFalse); + }); + + test('a public drive has no key artifact to offer', () async { + await insertDrive(isPrivate: false); + + final success = await settled(cubit(await drive())) + as DriveShareLoadSuccess; + + expect(success.driveKeyBase64, isNull); + expect(success.hasSeparateKeyArtifact, isFalse); + }); + + test('a folder link names the folder inside the drive', () async { + const folderId = 'c4d9dc2c-5d4c-6e3d-ac4f-8f3c2d5e6f70'; + + await insertDrive(isPrivate: false); + + final success = + await settled(cubit(await drive(), folderId: folderId)) + as DriveShareLoadSuccess; + + expect( + success.driveShareLink.toString(), + contains('/#/drives/$driveId/folders/$folderId'), + ); + expect(success.isFolder, isTrue); + }); + + test('a private folder link can carry the drive key when opted in', + () async { + const folderId = 'c4d9dc2c-5d4c-6e3d-ac4f-8f3c2d5e6f70'; + + when(() => profileCubit.state).thenReturn(ProfilePromptAdd()); + + await insertDrive(isPrivate: true); + await driveDao.putDriveKeyInMemory(driveID: driveId, driveKey: driveKey); + + final c = cubit(await drive(), folderId: folderId); + + await settled(c); + await c.setKeyIsInLink(true); + + final link = (c.state as DriveShareLoadSuccess).driveShareLink.toString(); + + // Both halves have to survive together: the parser used to return the + // drive route as soon as it saw a key, which lost the folder. + expect(link, contains('/folders/$folderId')); + expect(link, contains('driveKey=')); + }); + + test('toggling the key rebuilds the link without a loading state', + ( + ) async { + // The dialog renders a centred spinner for DriveShareLoadInProgress, so + // re-announcing it here would blank the dialog and take the checkbox the + // sharer just clicked off the screen. The file share dialog rebuilds its + // link with no loading state; this one matches. + when(() => profileCubit.state).thenReturn(ProfilePromptAdd()); + + await insertDrive(isPrivate: true); + await driveDao.putDriveKeyInMemory(driveID: driveId, driveKey: driveKey); + + final c = cubit(await drive()); + + await settled(c); + + final states = []; + final subscription = c.stream.listen(states.add); + + await c.setKeyIsInLink(true); + await Future.delayed(Duration.zero); + await subscription.cancel(); + + expect(states, isNot(contains(isA()))); + expect(states.last, isA()); + expect( + (states.last as DriveShareLoadSuccess).driveShareLink.toString(), + contains('driveKey='), + ); + }); + + test('a folder share carries the folder\'s own name', () async { + // The dialog confirms what is being shared. Described by the drive's + // name, a folder share would confirm the wrong thing. + const folderId = 'c4d9dc2c-5d4c-6e3d-ac4f-8f3c2d5e6f70'; + + await insertDrive(isPrivate: false); + + final success = await settled( + cubit(await drive(), folderId: folderId, folderName: 'Q4 Photos'), + ) as DriveShareLoadSuccess; + + expect(success.folderName, 'Q4 Photos'); + expect(success.isFolder, isTrue); + }); + + test('the drive key never reaches the state\'s stringification', () async { + // Equatable stringifies props in debug builds, and one of them is a key. + when(() => profileCubit.state).thenReturn(ProfilePromptAdd()); + + await insertDrive(isPrivate: true); + await driveDao.putDriveKeyInMemory(driveID: driveId, driveKey: driveKey); + + final success = + await settled(cubit(await drive())) as DriveShareLoadSuccess; + + expect(success.driveKeyBase64, isNotNull); + expect(success.toString(), contains('')); + expect(success.toString(), isNot(contains(success.driveKeyBase64!))); + }); + + test('a private drive resolves to a link carrying its key', () async { + // Signed out, so the key comes from the in-memory vault - the path a + // drive attached this session but never persisted takes. + when(() => profileCubit.state).thenReturn(ProfilePromptAdd()); + + await insertDrive(isPrivate: true); + await driveDao.putDriveKeyInMemory( + driveID: driveId, + driveKey: driveKey, + ); + + final c = cubit(await drive()); + + await settled(c); + await c.setKeyIsInLink(true); + + expect( + (c.state as DriveShareLoadSuccess).driveShareLink.toString(), + contains('driveKey='), + ); + }); + }); +} diff --git a/test/blocs/shared_file/shared_file_cubit_test.dart b/test/blocs/shared_file/shared_file_cubit_test.dart index 4c096f2540..17f8e5cbb6 100644 --- a/test/blocs/shared_file/shared_file_cubit_test.dart +++ b/test/blocs/shared_file/shared_file_cubit_test.dart @@ -155,6 +155,7 @@ void main() { bool linkKeyIsDamaged = false, SharedFileLinkPayload? payload, ArDriveCrypto? crypto, + Duration readTimeout = SharedFileCubit.defaultReadTimeout, }) => SharedFileCubit( fileId: id, @@ -166,6 +167,7 @@ void main() { crypto: crypto, // The propagation retries are the behavior under test, not the wait. propagationRetryDelay: Duration.zero, + readTimeout: readTimeout, ); setUpAll(() { @@ -191,6 +193,41 @@ void main() { expect: () => [isA()], ); + blocTest( + 'gives up on a read that hangs rather than waiting forever', + build: () { + // Not a failure - silence. `GraphQLRetry` retries a call that throws, + // but sets no timeout, so a connection that never answers was never + // retried and never abandoned either: the page sat on its skeleton + // for as long as the tab stayed open. A completer that is never + // completed is exactly that gateway. + when(() => arweave.getFilePrivacyForId(any())) + .thenAnswer((_) => Completer().future); + + return createCubit( + readTimeout: const Duration(milliseconds: 50), + ); + }, + wait: const Duration(milliseconds: 200), + expect: () => [isA()], + ); + + blocTest( + 'gives up on a hanging revision read too', + build: () { + when(() => arweave.getFilePrivacyForId(any())) + .thenAnswer((_) async => DrivePrivacyTag.public); + when(() => arweave.getAllFileEntitiesWithId(any(), any())) + .thenAnswer((_) => Completer>().future); + + return createCubit( + readTimeout: const Duration(milliseconds: 50), + ); + }, + wait: const Duration(milliseconds: 200), + expect: () => [isA()], + ); + blocTest( 'emits SharedFileLoadFailure when fetching the file entities fails', build: () { diff --git a/test/blocs/shared_file_download_cubit_test.dart b/test/blocs/shared_file_download_cubit_test.dart index 8bcee49394..37d41c563b 100644 --- a/test/blocs/shared_file_download_cubit_test.dart +++ b/test/blocs/shared_file_download_cubit_test.dart @@ -10,8 +10,10 @@ import 'package:cryptography/cryptography.dart'; // lib/download/limits.dart. // ignore: depend_on_referenced_packages import 'package:fake_async/fake_async.dart'; +import 'package:ardrive/services/services.dart'; import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:mocktail/mocktail.dart'; import '../test_utils/mocks.dart'; @@ -152,16 +154,49 @@ class _RecordingDownloader implements ArDriveDownloader { /// The recipient's download. void main() { - SharedFileDownloadCubit cubitFor(_RecordingDownloader downloader) { + SharedFileDownloadCubit cubitFor( + _RecordingDownloader downloader, { + ArweaveService? arweave, + SecretKey? fileKey, + String? cipher, + String? cipherIv, + DownloadPolicy policy = const _UnavailablePolicy(), + }) { return SharedFileDownloadCubit( revision: _Revision(), - arweave: MockArweaveService(), + arweave: arweave ?? MockArweaveService(), crypto: MockArDriveCrypto(), arDriveDownloader: downloader, - downloadPolicy: const _UnavailablePolicy(), + fileKey: fileKey, + cipher: cipher, + cipherIv: cipherIv, + downloadPolicy: policy, ); } + test( + 'a private download uses the cipher the link named, without asking for ' + 'it again', () async { + // `c` and `iv` are in the link schema precisely so this lookup does not + // have to happen. Before this the download re-fetched tags the link had + // already delivered - wasted on a good connection, and on a rate limited + // one a call that can fail outright. + final arweave = MockArweaveService(); + final downloader = _RecordingDownloader(); + + final cubit = cubitFor( + downloader, + arweave: arweave, + fileKey: SecretKey(List.filled(32, 1)), + cipher: 'AES256-GCM', + cipherIv: 'an-iv', + ); + + await cubit.stream.firstWhere((s) => s is! FileDownloadStarting); + + verifyNever(() => arweave.getTransactionDetails(any())); + }); + test('a size check that throws costs the check, not the download', () async { final downloader = _RecordingDownloader(); diff --git a/test/components/copyable_share_artifact_test.dart b/test/components/copyable_share_artifact_test.dart new file mode 100644 index 0000000000..6425a0527d --- /dev/null +++ b/test/components/copyable_share_artifact_test.dart @@ -0,0 +1,133 @@ +import 'package:ardrive/components/copyable_share_artifact.dart'; +import 'package:ardrive_ui/ardrive_ui.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + const secret = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopQ'; + + Widget wrap(Widget child) => ArDriveTheme( + themeData: lightTheme(), + child: MaterialApp(home: Scaffold(body: child)), + ); + + Widget artifact({required bool isSecret, String text = secret}) => + CopyableShareArtifact( + label: 'Access key', + text: text, + copyLabel: 'Copy', + revealLabel: 'Show access key', + isSecret: isSecret, + ); + + /// The obscured state lives on the design system's own field, so read it + /// from there rather than from anything this component owns. + bool isObscured(WidgetTester tester) => + tester.widget(find.byType(EditableText)).obscureText; + + group('CopyableShareArtifact', () { + testWidgets('a secret artifact starts masked', (tester) async { + await tester.pumpWidget(wrap(artifact(isSecret: true))); + + expect(isObscured(tester), isTrue); + }); + + testWidgets('a non-secret artifact is never masked', (tester) async { + await tester.pumpWidget(wrap(artifact(isSecret: false))); + + expect(isObscured(tester), isFalse); + }); + + testWidgets('a non-secret artifact offers no reveal control', + (tester) async { + // A public link has nothing to hide, and a toggle that does nothing is + // one more thing to explain. + await tester.pumpWidget(wrap(artifact(isSecret: false))); + + expect(find.byTooltip('Show access key'), findsNothing); + }); + + testWidgets( + 'the reveal toggle works even though the field is read only - ' + 'a mask the sharer cannot lift would be a dead end', (tester) async { + // The field is `isEnabled: false` so the value cannot be edited, and a + // disabled field swallows taps on its own decoration - which is why the + // design system's built-in `showObfuscationToggle` cannot be used here. + // This asserts the replacement control is actually reachable. + await tester.pumpWidget(wrap(artifact(isSecret: true))); + + expect(isObscured(tester), isTrue); + + await tester.tap(find.byTooltip('Show access key')); + await tester.pump(); + + expect(isObscured(tester), isFalse); + + // And back, so the sharer can re-hide it without closing the dialog. + await tester.tap(find.byTooltip('Show access key')); + await tester.pump(); + + expect(isObscured(tester), isTrue); + }); + + testWidgets('a synchronously available value is shown, not swallowed', + (tester) async { + // The component owns its controller precisely so this holds. When each + // dialog filled a controller from a bloc listener instead, a cubit that + // reached success synchronously - which the public drive path does - + // never fired the listener, and the field rendered empty. + await tester.pumpWidget(wrap(artifact(isSecret: false, text: 'a-link'))); + + expect( + tester.widget(find.byType(EditableText)).controller.text, + 'a-link', + ); + }); + + testWidgets('revealing one secret does not reveal the next', + (tester) async { + // Ticking "include the key in the link" swaps the value underneath a + // revealed field. The new value must start masked again. + await tester.pumpWidget(wrap(artifact(isSecret: true))); + + await tester.tap(find.byTooltip('Show access key')); + await tester.pump(); + + expect(isObscured(tester), isFalse); + + await tester.pumpWidget( + wrap(artifact(isSecret: true, text: 'a-different-secret')), + ); + await tester.pump(); + + expect(isObscured(tester), isTrue); + }); + + testWidgets('a secret and a plain field are the same width', + (tester) async { + // A dialog stacks a link over a key. The reveal control only exists on + // the secret one, so unless its slot is held open on both, the two boxes + // render at visibly different widths. + double fieldWidth(WidgetTester tester) => + tester.getSize(find.byType(ArDriveTextFieldNew)).width; + + await tester.pumpWidget(wrap(artifact(isSecret: false))); + final plain = fieldWidth(tester); + + await tester.pumpWidget(wrap(artifact(isSecret: true))); + final secret = fieldWidth(tester); + + expect(secret, plain); + }); + + testWidgets('the value stays intact underneath the mask', (tester) async { + // Masking is a display concern. If it ever reached the controller the + // sharer would hand out a string of dots. + await tester.pumpWidget(wrap(artifact(isSecret: true))); + + final field = tester.widget(find.byType(EditableText)); + + expect(field.controller.text, secret); + }); + }); +} diff --git a/test/components/drive_share_dialog_test.dart b/test/components/drive_share_dialog_test.dart new file mode 100644 index 0000000000..2cdfc93cc8 --- /dev/null +++ b/test/components/drive_share_dialog_test.dart @@ -0,0 +1,180 @@ +import 'package:ardrive/blocs/blocs.dart'; +import 'package:ardrive/components/drive_share_dialog.dart'; +import 'package:ardrive/models/models.dart'; +import 'package:ardrive_utils/ardrive_utils.dart'; +import 'package:ardrive_ui/ardrive_ui.dart'; +import 'package:bloc_test/bloc_test.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_bloc/flutter_bloc.dart'; +import 'package:flutter_gen/gen_l10n/app_localizations.dart'; +import 'package:flutter_localizations/flutter_localizations.dart'; +import 'package:flutter_test/flutter_test.dart'; + +class MockDriveShareCubit extends MockCubit + implements DriveShareCubit {} + +/// What the share drive dialog actually puts on screen. +/// +/// These exist because the dialog had no test at all, and the gap showed: it +/// filled its fields from a bloc listener, which never fires for the state a +/// bloc is already in, so a public drive - whose link is built synchronously - +/// rendered an empty link field. +void main() { + const driveId = 'a2b7ba0a-3b2a-4c1b-8a2f-6d1a0b3c4d5e'; + const driveKeyBase64 = 'X123YZAB-CD4e5fgHIjKlmN6O7pqrStuVwxYzaBcd8E'; + + late MockDriveShareCubit cubit; + + Drive drive({required bool isPrivate}) => Drive( + id: driveId, + name: 'My Drive', + ownerAddress: 'owner', + rootFolderId: 'root', + privacy: + isPrivate ? DrivePrivacyTag.private : DrivePrivacyTag.public, + isHidden: false, + dateCreated: DateTime.utc(2026, 1, 1), + lastUpdated: DateTime.utc(2026, 1, 1), + ); + + Widget wrap(Widget child) => ArDriveTheme( + themeData: lightTheme(), + child: MaterialApp( + localizationsDelegates: const [ + AppLocalizations.delegate, + GlobalMaterialLocalizations.delegate, + GlobalWidgetsLocalizations.delegate, + GlobalCupertinoLocalizations.delegate, + ], + supportedLocales: const [Locale('en', '')], + home: Scaffold(body: child), + ), + ); + + Future pumpState(WidgetTester tester, DriveShareState state) async { + tester.view.physicalSize = const Size(1200, 1600); + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.reset); + + whenListen(cubit, const Stream.empty(), + initialState: state); + + await tester.pumpWidget( + wrap( + BlocProvider.value( + value: cubit, + child: const DriveShareDialog(), + ), + ), + ); + await tester.pump(); + } + + /// The value shown in a field, whether or not it is masked. + String fieldText(WidgetTester tester, int index) => + tester.widgetList(find.byType(EditableText)).toList()[index] + .controller + .text; + + setUp(() => cubit = MockDriveShareCubit()); + + group('DriveShareDialog', () { + testWidgets('a public drive shows its link', (tester) async { + // The regression that started this file: the public path builds its link + // synchronously, so the dialog was already in the success state before + // it could listen for one, and the field came up blank. + await pumpState( + tester, + DriveShareLoadSuccess( + drive: drive(isPrivate: false), + driveShareLink: + Uri.parse('https://app.ardrive.io/#/drives/$driveId?name=My+Drive'), + ), + ); + + expect( + fieldText(tester, 0), + 'https://app.ardrive.io/#/drives/$driveId?name=My+Drive', + ); + }); + + testWidgets('a keyless private drive does not claim the link is enough', + (tester) async { + // The old copy - "Anyone can access this private drive using the link + // above" - was written when the key was always embedded. For the keyless + // default it is simply false, and it is the sharer's only cue about what + // they still have to send. + await pumpState( + tester, + DriveShareLoadSuccess( + drive: drive(isPrivate: true), + driveShareLink: + Uri.parse('https://app.ardrive.io/#/drives/$driveId'), + driveKeyBase64: driveKeyBase64, + ), + ); + + expect( + find.textContaining('Anyone can access this private drive'), + findsNothing, + ); + expect(find.textContaining('The link alone'), findsOneWidget); + + // And the key is offered as its own artifact to send separately. + expect(find.text('Drive access key'), findsOneWidget); + expect(fieldText(tester, 1), driveKeyBase64); + }); + + testWidgets('embedding the key retires the separate artifact', + (tester) async { + await pumpState( + tester, + DriveShareLoadSuccess( + drive: drive(isPrivate: true), + driveShareLink: Uri.parse( + 'https://app.ardrive.io/#/drives/$driveId?driveKey=$driveKeyBase64', + ), + driveKeyBase64: driveKeyBase64, + keyIsInLink: true, + ), + ); + + expect(find.text('Drive access key'), findsNothing); + expect( + find.textContaining('Anyone can access this private drive'), + findsOneWidget, + ); + }); + + testWidgets('a folder share is named after the folder, not the drive', + (tester) async { + await pumpState( + tester, + DriveShareLoadSuccess( + drive: drive(isPrivate: false), + driveShareLink: Uri.parse( + 'https://app.ardrive.io/#/drives/$driveId/folders/folder-id', + ), + isFolder: true, + folderName: 'Q4 Photos', + ), + ); + + expect(find.text('Share folder with others'), findsOneWidget); + expect(find.text('Q4 Photos'), findsOneWidget); + expect(find.text('My Drive'), findsNothing); + }); + + testWidgets('a failure is shown, with a way out of it', (tester) async { + // The state that used to be unreachable: the dialog spun forever instead. + await pumpState(tester, const DriveShareLoadFail()); + + expect(find.byType(CircularProgressIndicator), findsNothing); + expect( + find.textContaining('couldn’t create a share link'), + findsOneWidget, + ); + expect(find.text('Try Again'), findsOneWidget); + }); + }); +} diff --git a/test/pages/app_route_information_parser_test.dart b/test/pages/app_route_information_parser_test.dart index 9233814a6e..7e826cbba6 100644 --- a/test/pages/app_route_information_parser_test.dart +++ b/test/pages/app_route_information_parser_test.dart @@ -306,12 +306,12 @@ void main() { }); }); - group('restoreRouteInformation', () { - Future roundTrip(AppRoutePath routePath) async { - final restored = parser.restoreRouteInformation(routePath); + Future roundTripOf(AppRoutePath routePath) => + parser.parseRouteInformation(parser.restoreRouteInformation(routePath)); - return parser.parseRouteInformation(restored); - } + group('restoreRouteInformation', () { + Future roundTrip(AppRoutePath routePath) => + roundTripOf(routePath); test('a v1 link keeps the shape it has always had', () async { final routePath = await parse( @@ -390,6 +390,21 @@ void main() { expect(restored.sharedRawDriveKey, validDriveKey); }); + test('a nameless private drive link keeps its key', () async { + // A private drive link no longer carries a name - the name is a secret + // and the recipient's attach flow reads the real one off the chain. The + // key must survive the round trip regardless, or a refresh would drop it + // and the recipient would be asked to paste a key the link already had. + final routePath = await parse( + '/drives/$driveId?$driveKeyQueryParamName=$validDriveKey', + ); + + final restored = await roundTrip(routePath); + + expect(restored.driveId, driveId); + expect(restored.sharedRawDriveKey, validDriveKey); + }); + test('a folder link still round trips', () async { final routePath = await parse('/drives/$driveId/folders/$folderId'); @@ -463,6 +478,35 @@ void main() { expect(routePath.sharedDriveKey, isNull); }); + test('a folder link keeps its drive key instead of losing the folder', + () async { + // The key used to be decoded inside the drive branch, which returned + // from there - so a *valid* key made the folder segments unreachable and + // the link quietly resolved to the drive root. Only a damaged key ever + // reached the folder route, which is exactly backwards. + final routePath = await parse( + '/drives/$driveId/folders/$folderId' + '?$driveKeyQueryParamName=$validDriveKey', + ); + + expect(routePath.driveId, driveId); + expect(routePath.driveFolderId, folderId); + expect(routePath.sharedRawDriveKey, validDriveKey); + expect(routePath.sharedDriveKey, isNotNull); + }); + + test('a private folder link round trips with both', () async { + final routePath = await parse( + '/drives/$driveId/folders/$folderId' + '?$driveKeyQueryParamName=$validDriveKey', + ); + + final restored = await roundTripOf(routePath); + + expect(restored.driveFolderId, folderId); + expect(restored.sharedRawDriveKey, validDriveKey); + }); + test('parses a folder link', () async { final routePath = await parse('/drives/$driveId/folders/$folderId'); diff --git a/test/pages/app_router_delegate_test.dart b/test/pages/app_router_delegate_test.dart new file mode 100644 index 0000000000..78d50ee94b --- /dev/null +++ b/test/pages/app_router_delegate_test.dart @@ -0,0 +1,144 @@ +import 'package:ardrive/pages/app_route_path.dart'; +import 'package:ardrive/pages/app_router_delegate.dart'; +import 'package:flutter_test/flutter_test.dart'; + +/// How a folder link survives the drive being attached under it. +/// +/// A folder link only ever opened the folder for someone who already had the +/// drive. Everyone else - which is every stranger a public folder link is sent +/// to - went through the attach flow, which ends by clearing the drive id so +/// the prompt cannot re-fire. The drive was then selected fresh, the folder +/// read as belonging to a different drive, and it was dropped. A folder link +/// was a drive link with extra characters. +void main() { + const driveId = 'a2b7ba0a-3b2a-4c1b-8a2f-6d1a0b3c4d5e'; + const otherDriveId = 'b3c8cb1b-4c3b-5d2c-9b3f-7e2b1c4d5e6f'; + const folderId = 'c4d9dc2c-5d4c-6e3d-ac4f-8f3c2d5e6f70'; + + late AppRouterDelegate delegate; + + setUp(() => delegate = AppRouterDelegate()); + + /// What the attach flow does when it finishes: the drive id is cleared so the + /// attach prompt cannot fire again, and the drive is then selected fresh. + void attachCompletes() => delegate.driveId = null; + + group('a folder link', () { + test('opens its folder once the drive it names is selected', () async { + await delegate.setNewRoutePath( + AppRoutePath.folderDetail(driveId: driveId, driveFolderId: folderId), + ); + + attachCompletes(); + delegate.onDriveSelected(driveId); + + expect(delegate.driveFolderId, folderId); + expect(delegate.driveId, driveId); + }); + + test('opens its folder for someone who already had the drive', () async { + // No attach in between: the drive is already selected. + await delegate.setNewRoutePath( + AppRoutePath.folderDetail(driveId: driveId, driveFolderId: folderId), + ); + + delegate.onDriveSelected(driveId); + + expect(delegate.driveFolderId, folderId); + }); + + test('is released after it has been honored', () async { + // Otherwise every later visit to that drive would jump back into the + // folder the link named, long after the link was opened. + await delegate.setNewRoutePath( + AppRoutePath.folderDetail(driveId: driveId, driveFolderId: folderId), + ); + + attachCompletes(); + delegate.onDriveSelected(driveId); + + // Away to another drive, and back. + delegate.onDriveSelected(otherDriveId); + delegate.onDriveSelected(driveId); + + expect(delegate.driveFolderId, isNull); + }); + + test('never carries another drive\'s folder in under its name', () async { + // The pending marker used to preserve whatever folder was in view when + // its drive arrived, rather than restoring the one the link named. A + // recipient who wandered off before the drive was selected therefore + // landed on drive A showing a folder that belongs to drive B. + const otherFolderId = 'd5eaed3d-6e5d-7f4e-bd5f-9f4d3e6f7a81'; + + await delegate.setNewRoutePath( + AppRoutePath.folderDetail(driveId: driveId, driveFolderId: folderId), + ); + + // Away to another drive, and into a folder there. + delegate.onDriveSelected(otherDriveId); + delegate.driveFolderId = otherFolderId; + + // Now the linked drive finally arrives. + delegate.onDriveSelected(driveId); + + expect(delegate.driveFolderId, folderId); + expect(delegate.driveFolderId, isNot(otherFolderId)); + }); + + test('does not follow the user to a different drive', () async { + await delegate.setNewRoutePath( + AppRoutePath.folderDetail(driveId: driveId, driveFolderId: folderId), + ); + + delegate.onDriveSelected(otherDriveId); + + expect(delegate.driveFolderId, isNull); + expect(delegate.driveId, otherDriveId); + }); + }); + + group('a drive link', () { + test('holds nothing back, since it names no folder', () async { + await delegate.setNewRoutePath( + AppRoutePath.driveDetail(driveId: driveId), + ); + + // The folder in view at the time belongs to wherever the user was. + delegate.driveFolderId = folderId; + + attachCompletes(); + delegate.onDriveSelected(driveId); + + expect(delegate.driveFolderId, isNull); + }); + }); + + group('ordinary navigation', () { + test('still discards the folder when the drive changes', () async { + await delegate.setNewRoutePath( + AppRoutePath.driveDetail(driveId: driveId), + ); + + delegate.onDriveSelected(driveId); + delegate.driveFolderId = folderId; + + delegate.onDriveSelected(otherDriveId); + + expect(delegate.driveFolderId, isNull); + }); + + test('keeps the folder while the drive stays the same', () async { + await delegate.setNewRoutePath( + AppRoutePath.driveDetail(driveId: driveId), + ); + + delegate.onDriveSelected(driveId); + delegate.driveFolderId = folderId; + + delegate.onDriveSelected(driveId); + + expect(delegate.driveFolderId, folderId); + }); + }); +} diff --git a/test/pages/shared_file/shared_file_page_test.dart b/test/pages/shared_file/shared_file_page_test.dart index 64090f1b59..a95b977c77 100644 --- a/test/pages/shared_file/shared_file_page_test.dart +++ b/test/pages/shared_file/shared_file_page_test.dart @@ -59,11 +59,22 @@ void main() { late MockSharedFileCubit cubit; late StreamController states; + /// The value rendered beside [label] in the details drawer, scoped to that + /// label's own row. + Finder detailRowValue(String label, String value) => find.descendant( + of: find + .ancestor(of: find.text(label), matching: find.byType(Row)) + .first, + matching: find.text(value), + ); + FileRevision fileRevision({ String name = 'Q3 Report.pdf', String dataTxId = 'data-tx-newest', int size = 4821133, String? dataContentType = 'application/pdf', + DateTime? lastModifiedDate, + DateTime? dateCreated, }) { return FileRevision( fileId: fileId, @@ -71,11 +82,11 @@ void main() { name: name, parentFolderId: 'parent-folder-id', size: size, - lastModifiedDate: DateTime.utc(2024, 3, 3), + lastModifiedDate: lastModifiedDate ?? DateTime.utc(2023, 11, 9), dataContentType: dataContentType, metadataTxId: 'metadata-tx', dataTxId: dataTxId, - dateCreated: DateTime.utc(2024, 3, 3), + dateCreated: dateCreated ?? DateTime.utc(2024, 3, 3), action: RevisionAction.create, isHidden: false, ); @@ -490,6 +501,35 @@ void main() { expect(find.text('metadata-tx'), findsOneWidget); }); + testWidgets( + 'the details drawer states the type and when the file was uploaded', + (tester) async { + // A recipient sent a link by a stranger has almost nothing to judge the + // file by, and the upload date is the strongest signal available. It + // used to appear nowhere on this page: the drawer listed only ids, and + // the sole date on the page sat inside the version history, which is + // collapsed and not fetched until opened. + await pumpPage(tester, success()); + + await tester.tap(find.byType(ExpansionTile).first); + await tester.pumpAndSettle(); + + // Each value is scoped to the row its label is in, so two rows cannot + // satisfy each other's assertion - a swap between created and modified + // would otherwise still pass. The expected strings are literal rather + // than run through `formatDateToUtcString`, which would let a formatter + // change rewrite the production output and the expectation together. + expect(detailRowValue('File type', 'application/pdf'), findsOneWidget); + expect( + detailRowValue('Date created', '2024-03-03 00:00:00 GMT+0'), + findsOneWidget, + ); + expect( + detailRowValue('Last updated', '2023-11-09 00:00:00 GMT+0'), + findsOneWidget, + ); + }); + testWidgets('asks for the version history only when it is opened', (tester) async { await pumpPage(tester, success()); @@ -569,6 +609,38 @@ void main() { expect(find.text('Preview'), findsNothing); }); + testWidgets('shows no dates until the file\'s own record has them', + (tester) async { + // A v2 link carries no timestamps, so the revision it paints from holds + // the epoch placeholder until the metadata resolves. Rendering that puts + // "1970-01-01" in front of a recipient as though it were the upload + // date - worse than showing nothing, and worse than the placeholder was + // ever meant to be. + await pumpPage( + tester, + SharedFileLoadSuccess( + fileRevisions: [ + fileRevision( + lastModifiedDate: DateTime.fromMillisecondsSinceEpoch(0), + dateCreated: DateTime.fromMillisecondsSinceEpoch(0), + ), + ], + verification: LinkVerification.pending, + detailsAreResolved: false, + ), + ); + + await tester.tap(find.byType(ExpansionTile).first); + await tester.pumpAndSettle(); + + expect(find.text('Date created'), findsNothing); + expect(find.text('Last updated'), findsNothing); + expect(find.textContaining('1970'), findsNothing); + + // The type still shows: the link really did carry it. + expect(find.text('File type'), findsOneWidget); + }); + testWidgets('shows the verification badge the link earned', (tester) async { await pumpPage(tester, success(verification: LinkVerification.verified)); diff --git a/test/utils/link_generators_test.dart b/test/utils/link_generators_test.dart index 4f43f6d1c5..3f12b5abfa 100644 --- a/test/utils/link_generators_test.dart +++ b/test/utils/link_generators_test.dart @@ -46,8 +46,8 @@ void main() { () async { final webShareUri = await generatePrivateDriveShareLink( driveId: testPrivateDrive.id, - driveName: testPrivateDrive.name, driveKey: testPrivateDriveKey, + includeKey: true, ); // Remove # delimiter as it messes with Uri parsing outside of app route // information parser @@ -59,8 +59,9 @@ void main() { final driveKey = driveShareLink.queryParameters['driveKey']; expect(driveId, equals(testPrivateDrive.id)); - expect(driveName, equals(testPrivateDrive.name)); expect(driveKey, equals(testPrivateDriveKeyBase64)); + // The name is the drive's secret, not part of the handover. + expect(driveName, isNull); }); test( 'generatePublicDriveShareLink generates the correct link for a public drive', @@ -206,17 +207,78 @@ void main() { ); }); - test('a private drive link still carries its key in the query', () async { + test('a private drive link carries its key when the sharer opts in', + () async { const driveKeyBase64 = 'X123YZAB-CD4e5fgHIjKlmN6O7pqrStuVwxYzaBcd8E'; final link = await generatePrivateDriveShareLink( driveId: 'driveId', - driveName: 'My Drive', driveKey: SecretKey(decodeBase64ToBytes(driveKeyBase64)), + includeKey: true, ); expect(link.toString(), startsWith('https://app.ardrive.io/#/drives/')); - expect(link.toString(), endsWith('&driveKey=$driveKeyBase64')); + expect(link.toString(), endsWith('?driveKey=$driveKeyBase64')); + }); + + test('a private drive link is keyless unless asked otherwise', () async { + // The default matters more than the capability: a drive key opens + // every file in the drive for the life of the drive and cannot be + // rotated, so embedding one is a deliberate act. + const driveKeyBase64 = 'X123YZAB-CD4e5fgHIjKlmN6O7pqrStuVwxYzaBcd8E'; + + final link = await generatePrivateDriveShareLink( + driveId: 'driveId', + driveKey: SecretKey(decodeBase64ToBytes(driveKeyBase64)), + ); + + expect(link.toString(), 'https://app.ardrive.io/#/drives/driveId'); + expect(link.toString(), isNot(contains(driveKeyBase64))); + }); + + test('a folder link names the folder under its drive', () async { + expect( + generatePublicDriveShareLink( + driveId: 'driveId', + driveName: 'My Drive', + folderId: 'folderId', + ).toString(), + 'https://app.ardrive.io/#/drives/driveId/folders/folderId' + '?name=My+Drive', + ); + }); + + test('a private folder link keeps the folder and the key together', + () async { + const driveKeyBase64 = 'X123YZAB-CD4e5fgHIjKlmN6O7pqrStuVwxYzaBcd8E'; + + final link = await generatePrivateDriveShareLink( + driveId: 'driveId', + driveKey: SecretKey(decodeBase64ToBytes(driveKeyBase64)), + folderId: 'folderId', + includeKey: true, + ); + + expect( + link.toString(), + 'https://app.ardrive.io/#/drives/driveId/folders/folderId' + '?driveKey=$driveKeyBase64', + ); + }); + + test('a private drive link does not carry the drive name', () async { + // The name of a private drive is as sensitive as the names of the + // files inside it, and the recipient's attach flow reads the real one + // off the chain as soon as the key is in hand. + const driveKeyBase64 = 'X123YZAB-CD4e5fgHIjKlmN6O7pqrStuVwxYzaBcd8E'; + + final link = await generatePrivateDriveShareLink( + driveId: 'driveId', + driveKey: SecretKey(decodeBase64ToBytes(driveKeyBase64)), + ); + + expect(link.toString(), isNot(contains('name='))); + expect(link.toString(), isNot(contains('Layoffs'))); }); });