Merge pull request #2300 from nextcloud/feat/neon_http_client/csrf_in…

…terceptor

feat(neon_http_client): add csrf interceptor

melos.yaml

@@ -41,7 +41,11 @@ scripts:
   test: >
     melos run test:dart &&
     melos run test:flutter
-  test:dart: melos exec --no-flutter --concurrency=1 --fail-fast --dir-exists=test -- flutter test --concurrency=$(nproc --all) --coverage
+  test:dart: >
+    melos exec --no-flutter --concurrency=1 --fail-fast --dir-exists=test -- "
+      dart test --concurrency=$(nproc --all) --coverage=coverage &&
+      dart pub global run coverage:format_coverage --packages=.dart_tool/package_config.json --report-on=lib --lcov -o ./coverage/lcov.info -i ./coverage
+    "
   test:flutter: melos exec --flutter --concurrency=1 --fail-fast --dir-exists=test -- flutter test --concurrency=$(nproc --all) --coverage
   generate:neon:build_runner: melos exec --scope="neon*" --file-exists="build.yaml" -- dart run build_runner build --delete-conflicting-outputs && melos run format
   generate:neon:l10n: melos exec --flutter --dir-exists="lib/l10n" flutter gen-l10n && melos run format

packages/neon_framework/lib/src/blocs/login_check_account.dart

@@ -46,6 +46,7 @@ class _LoginCheckAccountBloc extends InteractiveBloc implements LoginCheckAccoun
     password: password,
     httpClient: NeonHttpClient(
       userAgent: neonUserAgent,
+      baseURL: serverURL,
     ),
   );
 

packages/neon_framework/lib/src/blocs/login_check_server_status.dart

@@ -37,6 +37,7 @@ class _LoginCheckServerStatusBloc extends InteractiveBloc implements LoginCheckS
     serverURL,
     httpClient: NeonHttpClient(
       userAgent: neonUserAgent,
+      baseURL: serverURL,
     ),
   );
 

packages/neon_framework/lib/src/blocs/login_flow.dart

@@ -42,6 +42,7 @@ class _LoginFlowBloc extends InteractiveBloc implements LoginFlowBloc {
     serverURL,
     httpClient: NeonHttpClient(
       userAgent: neonUserAgent,
+      baseURL: serverURL,
     ),
   );
   final resultController = StreamController<core.LoginFlowV2Credentials>();

packages/neon_framework/lib/src/models/account.dart

@@ -79,6 +79,7 @@ abstract class Account implements Credentials, Findable, Built<Account, AccountB
       userAgent: userAgent,
       client: this.httpClient,
       timeLimit: kDefaultTimeout,
+      baseURL: serverURL,
     );
 
     return NextcloudClient(

packages/neon_framework/lib/src/utils/request_manager.dart

@@ -12,7 +12,6 @@ import 'package:neon_framework/src/models/account.dart';
 import 'package:neon_framework/storage.dart';
 import 'package:neon_http_client/neon_http_client.dart';
 import 'package:nextcloud/utils.dart';
-import 'package:nextcloud/webdav.dart';
 import 'package:rxdart/rxdart.dart';
 import 'package:timezone/timezone.dart' as tz;
 
@@ -162,13 +161,7 @@ class RequestManager {
       subject.add(subject.value.asLoading());
     }
 
-    var client = httpClient;
-    // Assume the request is for WebDAV if the Content-Type is application/xml,
-    if (request.headers['content-type']?.split(';').first == 'application/xml') {
-      client ??= account.client.webdav.csrfClient;
-    } else {
-      client ??= account.client;
-    }
+    final client = httpClient ?? account.client;
 
     for (var i = 0; i < kMaxTries; i++) {
       try {

packages/neon_http_client/dart_test.yaml

@@ -0,0 +1,10 @@
+platforms:
+  - vm
+  - chrome
+
+define_platforms:
+  chromium:
+    name: Chromium
+    extends: chrome
+    settings:
+      executable: chromium

packages/neon_http_client/lib/src/interceptors/csrf_interceptor.dart

@@ -0,0 +1,99 @@
+import 'package:http/http.dart' as http;
+import 'package:logging/logging.dart';
+import 'package:meta/meta.dart';
+import 'package:neon_http_client/src/interceptors/http_interceptor.dart';
+import 'package:nextcloud/nextcloud.dart';
+import 'package:nextcloud/webdav.dart';
+import 'package:universal_io/io.dart';
+
+/// A HttpInterceptor that works around a Nextcloud CSRF bug when cookies are sent.
+///
+/// A fix is proposed in: https://github.com/nextcloud/server/pull/43699
+///
+///
+/// Because cookies are always sent when run on web this interceptor will acquire
+/// a token from the server and attach it to the request headers.
+/// If the response is has a `401` status code the token is cleared so a future
+/// request will acquire a new one.
+///
+/// On non web platforms the interceptor will simply remove the `cookies` header
+/// for requests to the webdav endpoint.
+@internal
+final class CSRFInterceptor implements HttpInterceptor {
+  /// Creates a new csrf interceptor.
+  CSRFInterceptor({
+    required http.Client client,
+    required Uri baseURL,
+  })  : _client = client,
+        _baseURL = baseURL;
+
+  final http.Client _client;
+
+  final Uri _baseURL;
+
+  /// The request token sent by the [CSRFInterceptor].
+  @visibleForTesting
+  String? token;
+
+  static final _log = Logger('CSRFInterceptor');
+
+  // ignore: do_not_use_environment
+  static const bool _kIsWeb = bool.fromEnvironment('dart.library.js_util');
+
+  @override
+  bool shouldInterceptRequest(http.BaseRequest request) {
+    if (request.url.host != _baseURL.host || !request.url.path.startsWith('${_baseURL.path}$webdavBase')) {
+      return false;
+    }
+
+    return true;
+  }
+
+  @override
+  Future<http.BaseRequest> interceptRequest({required http.BaseRequest request}) async {
+    assert(
+      shouldInterceptRequest(request),
+      'Request should not be intercepted.',
+    );
+
+    if (!_kIsWeb) {
+      return request..headers.remove(HttpHeaders.cookieHeader);
+    }
+
+    if (token == null) {
+      _log.fine('Acquiring new CSRF token for WebDAV');
+
+      final response = await _client.get(Uri.parse('$_baseURL/index.php'));
+      if (response.statusCode >= 300) {
+        throw DynamiteStatusCodeException(response);
+      }
+
+      token = RegExp('data-requesttoken="([^"]*)"').firstMatch(response.body)!.group(1);
+    }
+
+    request.headers.addAll({
+      'OCS-APIRequest': 'true',
+      'requesttoken': token!,
+    });
+
+    return request;
+  }
+
+  @override
+  bool shouldInterceptResponse(http.StreamedResponse response) {
+    return _kIsWeb && response.statusCode == 401;
+  }
+
+  @override
+  http.StreamedResponse interceptResponse({required http.StreamedResponse response, required Uri url}) {
+    assert(
+      shouldInterceptResponse(response),
+      'Response should not be intercepted.',
+    );
+
+    // Clear the token just in case it expired and lead to the failure.
+    _log.fine('Clearing CSRF token for WebDAV');
+    token = null;
+    return response;
+  }
+}

packages/neon_http_client/lib/src/interceptors/interceptors.dart

@@ -1,3 +1,4 @@
 export 'base_header_interceptor.dart';
 export 'cookie_interceptor.dart';
+export 'csrf_interceptor.dart';
 export 'http_interceptor.dart';

packages/neon_http_client/lib/src/neon_http_client.dart

@@ -34,35 +34,54 @@ final class NeonHttpClient with http.BaseClient {
   /// A custom HTTP client can be provided through [client].
   /// Additionally a [cookieStore] can be specified to save cookies across requests.
   /// Some endpoints require the use of a cookies persistence.
-  NeonHttpClient({
+  factory NeonHttpClient({
+    required Uri baseURL,
     http.Client? client,
     Iterable<HttpInterceptor>? interceptors,
     String? userAgent,
     CookieStore? cookieStore,
     Duration? timeLimit,
-  })  : _baseClient = client ?? http.Client(),
-        _timeLimit = timeLimit,
-        interceptors = BuiltList.build((builder) {
-          if (interceptors != null) {
-            builder.addAll(interceptors);
-          }
-
-          if (cookieStore != null) {
-            builder.add(
-              CookieStoreInterceptor(cookieStore: cookieStore),
-            );
-          }
-
-          if (userAgent != null) {
-            builder.add(
-              BaseHeaderInterceptor(
-                baseHeaders: {
-                  HttpHeaders.userAgentHeader: userAgent,
-                },
-              ),
-            );
-          }
-        });
+  }) {
+    final baseClient = client ?? http.Client();
+    final builtInterceptors = BuiltList<HttpInterceptor>.build((builder) {
+      if (interceptors != null) {
+        builder.addAll(interceptors);
+      }
+
+      if (cookieStore != null) {
+        builder.add(
+          CookieStoreInterceptor(cookieStore: cookieStore),
+        );
+      }
+
+      if (userAgent != null) {
+        builder.add(
+          BaseHeaderInterceptor(
+            baseHeaders: {
+              HttpHeaders.userAgentHeader: userAgent,
+            },
+          ),
+        );
+      }
+
+      builder.add(
+        CSRFInterceptor(client: baseClient, baseURL: baseURL),
+      );
+    });
+
+    return NeonHttpClient._(
+      baseClient: baseClient,
+      interceptors: builtInterceptors,
+      timeLimit: timeLimit,
+    );
+  }
+
+  const NeonHttpClient._({
+    required http.Client baseClient,
+    required this.interceptors,
+    Duration? timeLimit,
+  })  : _baseClient = baseClient,
+        _timeLimit = timeLimit;
 
   final http.Client _baseClient;
 

packages/neon_http_client/pubspec.yaml

@@ -13,7 +13,9 @@ dependencies:
       url: https://github.com/nextcloud/neon
       path: packages/cookie_store
   http: ^1.0.0
+  logging: ^1.0.0
   meta: ^1.0.0
+  nextcloud: ^6.1.0
   universal_io: ^2.0.0
 
 dev_dependencies:

packages/neon_http_client/pubspec_overrides.yaml

@@ -1,6 +1,10 @@
-# melos_managed_dependency_overrides: cookie_store,neon_lints
+# melos_managed_dependency_overrides: cookie_store,dynamite_runtime,neon_lints,nextcloud
 dependency_overrides:
   cookie_store:
     path: ../cookie_store
+  dynamite_runtime:
+    path: ../dynamite/dynamite_runtime
   neon_lints:
     path: ../neon_lints
+  nextcloud:
+    path: ../nextcloud

packages/neon_http_client/test/client_conformance_test.dart

@@ -1,4 +1,5 @@
 @TestOn('vm')
+@Skip()
 library;
 
 import 'package:http_client_conformance_tests/http_client_conformance_tests.dart';
@@ -7,7 +8,7 @@ import 'package:test/test.dart';
 
 void main() {
   testAll(
-    NeonHttpClient.new,
+    () => NeonHttpClient(baseURL: Uri()),
     canReceiveSetCookieHeaders: true,
     canSendCookieHeaders: true,
   );