From 57ba141930ac6a7146bc1d94f15a2f46eae6ab18 Mon Sep 17 00:00:00 2001 From: MarkG Date: Wed, 19 May 2021 15:27:48 +0700 Subject: [PATCH] fix: token doesn't refesh after expires (#77) --- lib/gen/assets.gen.dart | 7 +-- lib/modules/landing/landing_interactor.dart | 6 +++ lib/modules/landing/landing_module.dart | 3 +- lib/modules/landing/landing_presenter.dart | 10 +++- lib/modules/login/login_module.dart | 2 +- .../auth/auth_refresh_token_interceptor.dart | 25 +++++++++ .../{ => auth}/auth_repository.dart | 20 ++++++++ lib/services/api/api_exception.dart | 14 ++++- lib/services/api/api_service.dart | 14 +++++ lib/services/api/auth/auth_api_service.dart | 19 +++++-- .../params/auth_refresh_token_params.dart | 36 +++++++++++++ lib/services/http/http_exception.dart | 2 +- lib/services/http/http_interceptor.dart | 51 +++++++++++++++++++ lib/services/http/http_service.dart | 10 ++++ lib/services/locator/locator_service.dart | 2 +- pubspec.lock | 7 +++ pubspec.yaml | 1 + .../landing/landing_interactor_test.dart | 14 ++++- .../landing_interactor_test.mocks.dart | 7 ++- .../landing/landing_module_test.mocks.dart | 3 ++ .../landing/landing_presenter_test.dart | 26 ++++++++-- .../landing/landing_presenter_test.mocks.dart | 3 ++ test/modules/login/login_interactor_test.dart | 2 +- .../login/login_interactor_test.mocks.dart | 7 ++- 24 files changed, 267 insertions(+), 24 deletions(-) create mode 100644 lib/repositories/auth/auth_refresh_token_interceptor.dart rename lib/repositories/{ => auth}/auth_repository.dart (79%) create mode 100644 lib/services/api/auth/params/auth_refresh_token_params.dart create mode 100644 lib/services/http/http_interceptor.dart diff --git a/lib/gen/assets.gen.dart b/lib/gen/assets.gen.dart index face551..2d07bd4 100644 --- a/lib/gen/assets.gen.dart +++ b/lib/gen/assets.gen.dart @@ -27,10 +27,7 @@ class Assets { } class AssetGenImage extends AssetImage { - const AssetGenImage(String assetName) - : _assetName = assetName, - super(assetName); - final String _assetName; + const AssetGenImage(String assetName) : super(assetName); Image image({ Key? key, @@ -75,7 +72,7 @@ class AssetGenImage extends AssetImage { ); } - String get path => _assetName; + String get path => assetName; } class SvgGenImage { diff --git a/lib/modules/landing/landing_interactor.dart b/lib/modules/landing/landing_interactor.dart index 45a5ba5..9090486 100644 --- a/lib/modules/landing/landing_interactor.dart +++ b/lib/modules/landing/landing_interactor.dart @@ -2,6 +2,7 @@ part of 'landing_module.dart'; abstract class LandingInteractor extends Interactor { void validateAuthentication(); + void logout(); } abstract class LandingInteractorDelegate { @@ -21,4 +22,9 @@ class LandingInteractorImpl extends LandingInteractor { delegate?.authenticationDidFailToValidate.add(exception); }); } + + @override + void logout() { + _authRepository.logout().then((value) => null); + } } diff --git a/lib/modules/landing/landing_module.dart b/lib/modules/landing/landing_module.dart index 4772568..aa3a4f9 100644 --- a/lib/modules/landing/landing_module.dart +++ b/lib/modules/landing/landing_module.dart @@ -8,7 +8,8 @@ import 'package:survey/modules/home/home_module.dart'; import 'package:survey/modules/login/login_module.dart'; import 'package:survey/modules/screen.dart'; import 'package:survey/core/extensions/build_context.dart'; -import 'package:survey/repositories/auth_repository.dart'; +import 'package:survey/repositories/auth/auth_repository.dart'; +import 'package:survey/services/api/api_service.dart'; import 'package:survey/services/locator/locator_service.dart'; part 'landing_presenter.dart'; diff --git a/lib/modules/landing/landing_presenter.dart b/lib/modules/landing/landing_presenter.dart index aa6951a..df2c9e9 100644 --- a/lib/modules/landing/landing_presenter.dart +++ b/lib/modules/landing/landing_presenter.dart @@ -42,8 +42,14 @@ class LandingPresenterImpl extends LandingPresenter interactor.validateAuthentication(); } - void _authenticationDidFailToValidate(Object error) { - view.alert(error); + void _authenticationDidFailToValidate(Exception exception) { + if (exception == ApiException.invalidToken) { + interactor.logout(); + router.replaceToLoginScreen(context: view.context); + return; + } + + view.alert(exception); } void _didAllFinish(bool isAuthenticated) { diff --git a/lib/modules/login/login_module.dart b/lib/modules/login/login_module.dart index 3ebb23d..c94ad74 100644 --- a/lib/modules/login/login_module.dart +++ b/lib/modules/login/login_module.dart @@ -13,7 +13,7 @@ import 'package:survey/gen/assets.gen.dart'; import 'package:survey/modules/forgot_password/forgot_password_module.dart'; import 'package:survey/modules/home/home_module.dart'; import 'package:survey/modules/screen.dart'; -import 'package:survey/repositories/auth_repository.dart'; +import 'package:survey/repositories/auth/auth_repository.dart'; import 'package:survey/services/locator/locator_service.dart'; import 'package:flutter_gen/gen_l10n/app_localizations.dart'; diff --git a/lib/repositories/auth/auth_refresh_token_interceptor.dart b/lib/repositories/auth/auth_refresh_token_interceptor.dart new file mode 100644 index 0000000..a40f14b --- /dev/null +++ b/lib/repositories/auth/auth_refresh_token_interceptor.dart @@ -0,0 +1,25 @@ +part of 'auth_repository.dart'; + +class AuthRefreshTokenInterceptor extends HttpInterceptor { + final AuthRepository _authRepository = locator.get(); + + @override + final identifier = "auth_refresh_token"; + + @override + Future onException( + HttpException exception, HttpExceptionInterceptorHandler handler) async { + final apiException = ApiException.fromHttpException(exception); + if (apiException == null || apiException != ApiException.invalidToken) { + return handler.next(exception); + } + + try { + await _authRepository.refreshToken(); + } on Exception { + return handler.next(exception); + } + + return handler.retry(exception); + } +} diff --git a/lib/repositories/auth_repository.dart b/lib/repositories/auth/auth_repository.dart similarity index 79% rename from lib/repositories/auth_repository.dart rename to lib/repositories/auth/auth_repository.dart index 00b5e43..8cb7bd9 100644 --- a/lib/repositories/auth_repository.dart +++ b/lib/repositories/auth/auth_repository.dart @@ -2,10 +2,13 @@ import 'package:survey/models/auth_token_info.dart'; import 'package:survey/services/api/api_service.dart'; import 'package:survey/services/api/auth/auth_api_service.dart'; import 'package:survey/services/api/user/user_api_service.dart'; +import 'package:survey/services/http/http_service.dart'; import 'package:survey/services/local_storage/local_storage_service.dart'; import 'package:survey/services/locator/locator_service.dart'; import 'package:survey/models/user_info.dart'; +part 'auth_refresh_token_interceptor.dart'; + abstract class AuthRepository { static const tokenLocalStorageKey = "auth_repository_token"; @@ -27,6 +30,8 @@ abstract class AuthRepository { Future fetchUser(); Future attemptAndFetchUser(); + + Future refreshToken(); } class AuthRepositoryImpl implements AuthRepository { @@ -83,6 +88,7 @@ class AuthRepositoryImpl implements AuthRepository { } _accessToken = token.accessToken; + _apiService.addGlobalInterceptors([AuthRefreshTokenInterceptor()]); _apiService.configureGlobalToken(_accessToken, token.tokenType); } @@ -99,4 +105,18 @@ class AuthRepositoryImpl implements AuthRepository { await attempt(); await fetchUser(); } + + @override + Future refreshToken() async { + final oldToken = await _localStorageService + .getObject(AuthRepository.tokenLocalStorageKey); + + final params = + AuthRefreshTokenParams(refreshToken: oldToken!.refreshToken!); + final token = await _authApiService.refreshToken(params: params); + + await _localStorageService.setObject(token, + key: AuthRepository.tokenLocalStorageKey); + await attemptAndFetchUser(); + } } diff --git a/lib/services/api/api_exception.dart b/lib/services/api/api_exception.dart index b2b7c14..04ece69 100644 --- a/lib/services/api/api_exception.dart +++ b/lib/services/api/api_exception.dart @@ -1,6 +1,6 @@ part of 'api_service.dart'; -class ApiException implements LocalizedException { +class ApiException extends Equatable implements LocalizedException { const ApiException({ required this.source, required this.message, @@ -26,13 +26,23 @@ class ApiException implements LocalizedException { } final String? source; + final String code; + @override final String message; - final String code; + + @override + List get props => [source, code]; static const invalidResponseStructure = ApiException( source: "local", message: "Wrong response structure", code: "wrong_response_structure", ); + + static const invalidToken = ApiException( + source: "unauthorized", + message: "The access token is invalid", + code: "invalid_token", + ); } diff --git a/lib/services/api/api_service.dart b/lib/services/api/api_service.dart index ca369a2..a3c5bc7 100644 --- a/lib/services/api/api_service.dart +++ b/lib/services/api/api_service.dart @@ -1,3 +1,4 @@ +import 'package:equatable/equatable.dart'; import 'package:survey/core/classes/localized_exception.dart'; import 'package:survey/gen/configs.gen.dart'; import 'package:survey/services/http/http_service.dart'; @@ -36,12 +37,16 @@ abstract class ApiService { void configureGlobalBaseUrl(String? baseUrl); void configureGlobalToken(String? token, String? tokenType); + + void addGlobalInterceptors(List interceptor); } class ApiServiceImpl implements ApiService { static String? _baseUrl; static String? _token; static String _tokenType = "Bearer"; + static final List _interceptor = []; + final HttpService _httpService = locator.get(); @override @@ -111,6 +116,14 @@ class ApiServiceImpl implements ApiService { } } + @override + void addGlobalInterceptors(List interceptor) { + final identifiers = interceptor.map((e) => e.identifier); + _interceptor + .removeWhere((element) => identifiers.contains(element.identifier)); + _interceptor.addAll(interceptor); + } + Future> _request({ required HttpMethod method, String? baseUrl, @@ -139,6 +152,7 @@ class ApiServiceImpl implements ApiService { data: params?.toJson(), url: url, headers: headers, + interceptors: _interceptor, ) as Map; } on HttpException catch (e) { throw ApiException.fromHttpException(e) ?? e; diff --git a/lib/services/api/auth/auth_api_service.dart b/lib/services/api/auth/auth_api_service.dart index af5b605..12eeb28 100644 --- a/lib/services/api/auth/auth_api_service.dart +++ b/lib/services/api/auth/auth_api_service.dart @@ -7,16 +7,20 @@ import 'package:survey/services/http/http_service.dart'; part 'params/auth_login_params.dart'; +part 'params/auth_refresh_token_params.dart'; + abstract class AuthApiService { static const loginEndpoint = "/oauth/token"; static const logoutEndpoint = "/oauth/revoke"; + static const refreshTokenEndpoint = loginEndpoint; + static const preferenceTokenKey = "auth_service_preference_token"; - Future login({ - required AuthLoginParams params, - }); + Future login({required AuthLoginParams params}); Future logout(); + + Future refreshToken({required AuthRefreshTokenParams params}); } class AuthApiServiceImpl implements AuthApiService { @@ -40,4 +44,13 @@ class AuthApiServiceImpl implements AuthApiService { endpoint: AuthApiService.logoutEndpoint, ); } + + @override + Future refreshToken({required AuthRefreshTokenParams params}) { + return _apiService.call( + method: HttpMethod.post, + endpoint: AuthApiService.refreshTokenEndpoint, + params: params, + ); + } } diff --git a/lib/services/api/auth/params/auth_refresh_token_params.dart b/lib/services/api/auth/params/auth_refresh_token_params.dart new file mode 100644 index 0000000..d4bdb06 --- /dev/null +++ b/lib/services/api/auth/params/auth_refresh_token_params.dart @@ -0,0 +1,36 @@ +part of '../auth_api_service.dart'; + +class AuthRefreshTokenParams extends ApiParams { + factory AuthRefreshTokenParams({ + required String refreshToken, + String? clientId, + String? clientSecret, + }) { + return AuthRefreshTokenParams._( + refreshToken: refreshToken, + clientId: clientId ?? Configs.app.api.clientId, + clientSecret: clientSecret ?? Configs.app.api.clientSecret, + ); + } + + AuthRefreshTokenParams._({ + required this.refreshToken, + required this.clientId, + required this.clientSecret, + }); + + String refreshToken; + String clientId; + String clientSecret; + String grantType = "refresh_token"; + + @override + void mapping(Mapper map) { + map( + "refresh_token", refreshToken, (v) => refreshToken = v as String); + map("client_id", clientId, (v) => clientId = v as String); + map( + "client_secret", clientSecret, (v) => clientSecret = v as String); + map("grant_type", grantType, (v) => grantType = v as String); + } +} diff --git a/lib/services/http/http_exception.dart b/lib/services/http/http_exception.dart index c2931c9..cc1cd61 100644 --- a/lib/services/http/http_exception.dart +++ b/lib/services/http/http_exception.dart @@ -58,7 +58,7 @@ class HttpException implements Exception { return HttpException( response: response, type: type, - error: error.error, + error: error, ); } diff --git a/lib/services/http/http_interceptor.dart b/lib/services/http/http_interceptor.dart new file mode 100644 index 0000000..651435c --- /dev/null +++ b/lib/services/http/http_interceptor.dart @@ -0,0 +1,51 @@ +part of 'http_service.dart'; + +abstract class HttpInterceptor { + String get identifier; + + void onException( + HttpException exception, + HttpExceptionInterceptorHandler handler, + ) => + handler.next(exception); + + Interceptor toInterceptor(Dio dio) { + return InterceptorsWrapper(onError: (e, handler) { + onException( + HttpException.fromDioError(e), + HttpExceptionInterceptorHandler._(dio: dio, handler: handler), + ); + }); + } +} + +class HttpExceptionInterceptorHandler { + const HttpExceptionInterceptorHandler._({ + required Dio dio, + required ErrorInterceptorHandler handler, + }) : _dio = dio, + _handler = handler; + + final ErrorInterceptorHandler _handler; + final Dio _dio; + + void next(HttpException exception) { + _handler.next(exception.error as DioError); + } + + void reject(HttpException exception) { + _handler.reject(exception.error as DioError); + } + + Future retry(HttpException exception) { + final requestOptions = (exception.error as DioError).requestOptions; + + return _dio.request(requestOptions.path, + cancelToken: requestOptions.cancelToken, + data: requestOptions.data, + onReceiveProgress: requestOptions.onReceiveProgress, + onSendProgress: requestOptions.onSendProgress, + queryParameters: requestOptions.queryParameters, + options: requestOptions as Options); + } +} diff --git a/lib/services/http/http_service.dart b/lib/services/http/http_service.dart index e1f1450..8ea5e73 100644 --- a/lib/services/http/http_service.dart +++ b/lib/services/http/http_service.dart @@ -3,15 +3,20 @@ import 'package:enumerated_class/enumerated_class.dart'; import 'package:flutter/foundation.dart'; part 'http_method.dart'; + part 'http_exception.dart'; + part 'http_response.dart'; +part 'http_interceptor.dart'; + abstract class HttpService { Future request({ required HttpMethod method, dynamic data, required String url, Map? headers, + List interceptors = const [], }); } @@ -33,7 +38,12 @@ class HttpServiceImpl implements HttpService { dynamic data, required String url, Map? headers, + List interceptors = const [], }) async { + _dio.interceptors.addAll(interceptors.map( + (e) => e.toInterceptor(_dio), + )); + final options = Options(method: method.rawValue, headers: headers); try { final response = await _dio.request(url, diff --git a/lib/services/locator/locator_service.dart b/lib/services/locator/locator_service.dart index 7dbb13b..5cbae91 100644 --- a/lib/services/locator/locator_service.dart +++ b/lib/services/locator/locator_service.dart @@ -4,7 +4,7 @@ import 'package:survey/modules/landing/landing_module.dart'; import 'package:survey/modules/login/login_module.dart'; import 'package:survey/services/api/api_service.dart'; import 'package:survey/services/api/auth/auth_api_service.dart'; -import 'package:survey/repositories/auth_repository.dart'; +import 'package:survey/repositories/auth/auth_repository.dart'; import 'package:survey/services/local_storage/local_storage_service.dart'; import 'package:survey/services/http/http_service.dart'; import 'package:survey/services/api/user/user_api_service.dart'; diff --git a/pubspec.lock b/pubspec.lock index 7ed8a7c..6dec3b4 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -218,6 +218,13 @@ packages: url: "https://pub.dartlang.org" source: hosted version: "0.0.1+1" + equatable: + dependency: "direct main" + description: + name: equatable + url: "https://pub.dartlang.org" + source: hosted + version: "2.0.0" fake_async: dependency: transitive description: diff --git a/pubspec.yaml b/pubspec.yaml index e233774..f687039 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -40,6 +40,7 @@ dependencies: shared_preferences: ^2.0.5 modal_progress_hud_nsn: ^0.1.0-nullsafety-1 adaptive_dialog: ^0.10.0+5 + equatable: ^2.0.0 dev_dependencies: flutter_test: diff --git a/test/modules/landing/landing_interactor_test.dart b/test/modules/landing/landing_interactor_test.dart index b712a55..200ec60 100644 --- a/test/modules/landing/landing_interactor_test.dart +++ b/test/modules/landing/landing_interactor_test.dart @@ -3,7 +3,7 @@ import 'package:mockito/annotations.dart'; import 'package:mockito/mockito.dart'; import 'package:quick_test/quick_test.dart'; import 'package:survey/modules/landing/landing_module.dart'; -import 'package:survey/repositories/auth_repository.dart'; +import 'package:survey/repositories/auth/auth_repository.dart'; import 'package:survey/services/locator/locator_service.dart'; import '../../helpers/behavior_subject_generator.dart'; @@ -62,5 +62,17 @@ void main() { }); }); }); + + describe("its logout()", () { + beforeEach(() { + when(authRepository.logout()) + .thenAnswer((realInvocation) => Future.value(null)); + interactor.logout(); + }); + + it("triggers authRepository call logout()", () { + verify(authRepository.logout()).called(1); + }); + }); }); } diff --git a/test/modules/landing/landing_interactor_test.mocks.dart b/test/modules/landing/landing_interactor_test.mocks.dart index cd1e993..cb48a65 100644 --- a/test/modules/landing/landing_interactor_test.mocks.dart +++ b/test/modules/landing/landing_interactor_test.mocks.dart @@ -7,7 +7,7 @@ import 'dart:async' as _i4; import 'package:mockito/mockito.dart' as _i1; import 'package:rxdart/src/subjects/behavior_subject.dart' as _i2; import 'package:survey/modules/landing/landing_module.dart' as _i5; -import 'package:survey/repositories/auth_repository.dart' as _i3; +import 'package:survey/repositories/auth/auth_repository.dart' as _i3; // ignore_for_file: comment_references // ignore_for_file: unnecessary_parenthesis @@ -57,6 +57,11 @@ class MockAuthRepository extends _i1.Mock implements _i3.AuthRepository { (super.noSuchMethod(Invocation.method(#attemptAndFetchUser, []), returnValue: Future.value(null), returnValueForMissingStub: Future.value()) as _i4.Future); + @override + _i4.Future refreshToken() => + (super.noSuchMethod(Invocation.method(#refreshToken, []), + returnValue: Future.value(null), + returnValueForMissingStub: Future.value()) as _i4.Future); } /// A class which mocks [LandingInteractorDelegate]. diff --git a/test/modules/landing/landing_module_test.mocks.dart b/test/modules/landing/landing_module_test.mocks.dart index 9786c77..ad29b0a 100644 --- a/test/modules/landing/landing_module_test.mocks.dart +++ b/test/modules/landing/landing_module_test.mocks.dart @@ -47,4 +47,7 @@ class MockLandingInteractor extends _i1.Mock implements _i2.LandingInteractor { void validateAuthentication() => super.noSuchMethod(Invocation.method(#validateAuthentication, []), returnValueForMissingStub: null); + @override + void logout() => super.noSuchMethod(Invocation.method(#logout, []), + returnValueForMissingStub: null); } diff --git a/test/modules/landing/landing_presenter_test.dart b/test/modules/landing/landing_presenter_test.dart index cfbc5ff..dd11d4b 100644 --- a/test/modules/landing/landing_presenter_test.dart +++ b/test/modules/landing/landing_presenter_test.dart @@ -2,6 +2,7 @@ import 'package:mockito/annotations.dart'; import 'package:mockito/mockito.dart'; import 'package:quick_test/quick_test.dart'; import 'package:survey/modules/landing/landing_module.dart'; +import 'package:survey/services/api/api_service.dart'; import '../../mocks/build_context.dart'; import 'landing_presenter_test.mocks.dart'; @@ -54,12 +55,29 @@ void main() { }); describe("it receives didFailToValidateAuthentication event", () { - beforeEach(() { - presenter.authenticationDidFailToValidate.add(Exception()); + context("when exception is not InvalidToken", () { + beforeEach(() { + presenter.authenticationDidFailToValidate.add(Exception()); + }); + + it("triggers view to show alert dialog", () { + verify(view.alert(any)).called(1); + }); }); - it("triggers view to show alert dialog", () { - verify(view.alert(any)).called(1); + context("when exception is InvalidToken", () { + beforeEach(() { + presenter.authenticationDidFailToValidate + .add(ApiException.invalidToken); + }); + + it("triggers interactor to logout", () { + verify(interactor.logout()).called(1); + }); + + it("triggers router to replace to Login screen", () { + verify(router.replaceToLoginScreen(context: buildContext)).called(1); + }); }); }); diff --git a/test/modules/landing/landing_presenter_test.mocks.dart b/test/modules/landing/landing_presenter_test.mocks.dart index e857426..f1c550c 100644 --- a/test/modules/landing/landing_presenter_test.mocks.dart +++ b/test/modules/landing/landing_presenter_test.mocks.dart @@ -75,4 +75,7 @@ class MockLandingInteractor extends _i1.Mock implements _i3.LandingInteractor { void validateAuthentication() => super.noSuchMethod(Invocation.method(#validateAuthentication, []), returnValueForMissingStub: null); + @override + void logout() => super.noSuchMethod(Invocation.method(#logout, []), + returnValueForMissingStub: null); } diff --git a/test/modules/login/login_interactor_test.dart b/test/modules/login/login_interactor_test.dart index 3bdc42d..99c8382 100644 --- a/test/modules/login/login_interactor_test.dart +++ b/test/modules/login/login_interactor_test.dart @@ -3,7 +3,7 @@ import 'package:mockito/annotations.dart'; import 'package:mockito/mockito.dart'; import 'package:quick_test/quick_test.dart'; import 'package:survey/modules/login/login_module.dart'; -import 'package:survey/repositories/auth_repository.dart'; +import 'package:survey/repositories/auth/auth_repository.dart'; import 'package:survey/services/locator/locator_service.dart'; import '../../helpers/behavior_subject_generator.dart'; diff --git a/test/modules/login/login_interactor_test.mocks.dart b/test/modules/login/login_interactor_test.mocks.dart index 9418549..4fc5979 100644 --- a/test/modules/login/login_interactor_test.mocks.dart +++ b/test/modules/login/login_interactor_test.mocks.dart @@ -7,7 +7,7 @@ import 'dart:async' as _i4; import 'package:mockito/mockito.dart' as _i1; import 'package:rxdart/src/subjects/behavior_subject.dart' as _i2; import 'package:survey/modules/login/login_module.dart' as _i5; -import 'package:survey/repositories/auth_repository.dart' as _i3; +import 'package:survey/repositories/auth/auth_repository.dart' as _i3; // ignore_for_file: comment_references // ignore_for_file: unnecessary_parenthesis @@ -57,6 +57,11 @@ class MockAuthRepository extends _i1.Mock implements _i3.AuthRepository { (super.noSuchMethod(Invocation.method(#attemptAndFetchUser, []), returnValue: Future.value(null), returnValueForMissingStub: Future.value()) as _i4.Future); + @override + _i4.Future refreshToken() => + (super.noSuchMethod(Invocation.method(#refreshToken, []), + returnValue: Future.value(null), + returnValueForMissingStub: Future.value()) as _i4.Future); } /// A class which mocks [LoginInteractorDelegate].