Skip to content

Commit 66e2fad

Browse files
frankmerfrankmer
authored
Merge pull request #479 from privacyidea/478-redirect-status-codes-should-not-be-treated-as-errors
feat: implement HttpStatusChecker for consistent error handling across API responses
2 parents b462e3a + 9a6a68d commit 66e2fad

File tree

8 files changed

+110
-12
lines changed

8 files changed

+110
-12
lines changed

‎.github/workflows/flutter_build.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ jobs:
2626
- uses: subosito/flutter-action@v2
2727
with:
2828
channel: "stable"
29-
flutter-version: "3.32.8" # Use the latest stable Flutter version
29+
flutter-version: "3.35.3" # Use the latest stable Flutter version as of Sep 03 2025
3030

3131
- run: flutter clean
3232
- run: flutter --version
@@ -53,7 +53,7 @@ jobs:
5353
uses: subosito/flutter-action@v2
5454
with:
5555
channel: "stable"
56-
flutter-version: "3.32.8" # Current latest as of July 25, 2025
56+
flutter-version: "3.35.3" # Current latest as of Sep 03 2025
5757

5858
- name: Clean Flutter project
5959
run: flutter clean

‎lib/api/impl/privacy_idea_container_api.dart‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import '../../model/riverpod_states/token_state.dart';
4141
import '../../model/token_container.dart';
4242
import '../../model/token_template.dart';
4343
import '../../model/tokens/token.dart';
44+
import '../../utils/http_status_checker.dart';
4445
import '../../utils/logger.dart';
4546
import '../../widgets/dialog_widgets/container_dialogs/initial_token_assignment_dialog.dart';
4647
import '../../widgets/dialog_widgets/enter_passphrase_dialog.dart';
@@ -237,7 +238,7 @@ class PiContainerApi implements TokenContainerApi {
237238
};
238239

239240
final response = await _ioClient.doPost(url: requestUrl, body: body, sslVerify: container.sslVerify);
240-
if (response.statusCode != 200) {
241+
if (HttpStatusChecker.isError(response.statusCode)) {
241242
final errorResponse = response.asPiErrorResponse();
242243
if (errorResponse != null) throw errorResponse.piServerResultError;
243244
throw ResponseError(response);
@@ -284,7 +285,7 @@ class PiContainerApi implements TokenContainerApi {
284285
if (errorResponse != null) {
285286
throw errorResponse.piServerResultError;
286287
}
287-
if (response.statusCode != 200 || piResponse == null) throw ResponseError(response);
288+
if (HttpStatusChecker.isError(response.statusCode) || piResponse == null) throw ResponseError(response);
288289

289290
return piResponse.asSuccess!.resultValue;
290291
}
@@ -299,7 +300,7 @@ class PiContainerApi implements TokenContainerApi {
299300
TokenContainer.SCOPE: requestUrl.toString(),
300301
};
301302
final challengeResponse = await _ioClient.doPost(url: container.challengeUrl, body: body, sslVerify: container.sslVerify);
302-
if (challengeResponse.statusCode != 200) {
303+
if (HttpStatusChecker.isError(challengeResponse.statusCode)) {
303304
final errorResponse = challengeResponse.asPiErrorResponse();
304305
if (errorResponse != null) throw errorResponse.piServerResultError;
305306
throw ResponseError(challengeResponse);
@@ -338,7 +339,7 @@ class PiContainerApi implements TokenContainerApi {
338339
};
339340

340341
final response = await _ioClient.doPost(url: container.syncUrl, body: body, sslVerify: container.sslVerify);
341-
if (response.statusCode != 200) {
342+
if (HttpStatusChecker.isError(response.statusCode)) {
342343
final piErrorResponse = response.asPiErrorResponse();
343344
if (piErrorResponse != null) throw piErrorResponse.piServerResultError;
344345
throw ResponseError(response);

‎lib/model/exception_errors/response_error.dart‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
*/
2020
import 'package:http/http.dart';
2121

22+
import '../../utils/http_status_checker.dart';
23+
2224
class ResponseError {
2325
final int _statusCode;
2426
int get statusCode => _statusCode;
@@ -32,7 +34,7 @@ class ResponseError {
3234
//<title>405 Method Not Allowed</title>
3335
//<title>Method Not Allowed</title>
3436
factory ResponseError(Response response) {
35-
assert(response.statusCode != 200, 'Status code of an response error should not be 200');
37+
assert(HttpStatusChecker.isError(response.statusCode), 'Status code of an response error should not be 200');
3638
final regexpCode = RegExp(r'<title>(\d{3})');
3739
final regexpMessage = RegExp(r'(?<=(<title>\d* ?))[A-Za-z][A-Za-z\s]*(?=</title>)');
3840
final message = regexpMessage.firstMatch(response.body)?.group(0) ?? response.body;

‎lib/utils/http_status_checker.dart‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
abstract class HttpStatusChecker {
2+
static bool isInformational(int statusCode) {
3+
return statusCode >= 100 && statusCode < 200;
4+
}
5+
6+
static bool isSuccessful(int statusCode) {
7+
return statusCode >= 200 && statusCode < 300;
8+
}
9+
10+
static bool isRedirect(int statusCode) {
11+
return statusCode >= 300 && statusCode < 400;
12+
}
13+
14+
static bool isClientError(int statusCode) {
15+
return statusCode >= 400 && statusCode < 500;
16+
}
17+
18+
static bool isServerError(int statusCode) {
19+
return statusCode >= 500 && statusCode < 600;
20+
}
21+
22+
static bool isInvalidStatus(int statusCode) {
23+
return statusCode < 100 || statusCode >= 600;
24+
}
25+
26+
static bool isError(int statusCode) {
27+
return isClientError(statusCode) || isServerError(statusCode) || isInvalidStatus(statusCode);
28+
}
29+
}

‎lib/utils/privacyidea_io_client.dart‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import '../../../../../../../model/pi_server_response.dart';
3030
import '../model/api_results/pi_server_results/pi_server_result_value.dart';
3131
import '../utils/logger.dart';
3232
import '../utils/view_utils.dart';
33+
import 'http_status_checker.dart';
3334

3435
class PrivacyideaIOClient {
3536
const PrivacyideaIOClient();
@@ -138,7 +139,7 @@ class PrivacyideaIOClient {
138139
Logger.info('Post request finished');
139140
}
140141

141-
if (response.statusCode != 200) {
142+
if (HttpStatusChecker.isError(response.statusCode)) {
142143
Logger.warning(
143144
'Received unexpected response',
144145
error: 'Status code: ${response.statusCode}' '\nPosted body: $body' '\nResponse: ${response.body}\n',
@@ -202,7 +203,7 @@ class PrivacyideaIOClient {
202203
Logger.info('Post request finished');
203204
}
204205

205-
if (response.statusCode != 200) {
206+
if (HttpStatusChecker.isError(response.statusCode)) {
206207
Logger.warning('Received unexpected response: ${response.statusCode}');
207208
}
208209

‎lib/utils/riverpod/riverpod_providers/generated_providers/push_request_provider.dart‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import '../../../../model/push_request.dart';
2929
import '../../../../model/riverpod_states/push_request_state.dart';
3030
import '../../../../model/tokens/push_token.dart';
3131
import '../../../../repo/secure_push_request_repository.dart';
32+
import '../../../http_status_checker.dart';
3233
import '../../../logger.dart';
3334
import '../../../privacyidea_io_client.dart';
3435
import '../../../push_provider.dart';
@@ -395,7 +396,7 @@ class PushRequestNotifier extends _$PushRequestNotifier {
395396
return false;
396397
}
397398
}
398-
if (response.statusCode != 200) {
399+
if (HttpStatusChecker.isError(response.statusCode)) {
399400
ref.read(statusMessageProvider.notifier).state = StatusMessage(
400401
message: (l) => '${l.sendPushRequestResponseFailed}\n${l.statusCode(response.statusCode)}',
401402
details: (_) => (tryJsonDecode(response.body)?['result']?['error']?['message']) ?? '',

‎lib/utils/riverpod/riverpod_providers/generated_providers/token_notifier.dart‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import '../../../../repo/secure_token_repository.dart';
5050
import '../../../../views/import_tokens_view/pages/import_plain_tokens_page.dart';
5151
import '../../../firebase_utils.dart';
5252
import '../../../globals.dart';
53+
import '../../../http_status_checker.dart';
5354
import '../../../lock_auth.dart';
5455
import '../../../logger.dart';
5556
import '../../../privacyidea_io_client.dart';
@@ -626,7 +627,7 @@ class TokenNotifier extends _$TokenNotifier with ResultHandler {
626627
return false;
627628
}
628629

629-
if (response.statusCode != 200) {
630+
if (HttpStatusChecker.isError(response.statusCode)) {
630631
Logger.warning(
631632
'Post request on roll out failed.',
632633
error: 'Token: ${pushToken.serial}\nStatus code: ${response.statusCode},\nURL:${response.request?.url}\nBody: ${response.body}',
@@ -770,7 +771,7 @@ class TokenNotifier extends _$TokenNotifier with ResultHandler {
770771
body: {'new_fb_token': firebaseToken, 'serial': token.serial, 'timestamp': timestamp, 'signature': signature},
771772
sslVerify: token.sslVerify,
772773
);
773-
if (response.statusCode != 200) {
774+
if (HttpStatusChecker.isError(response.statusCode)) {
774775
Logger.warning('Updating firebase token for push token failed!');
775776
return false;
776777
}

‎test/unit_test/utils/http_status_checker_test.dart‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import 'package:flutter_test/flutter_test.dart';
2+
import 'package:privacyidea_authenticator/utils/http_status_checker.dart';
3+
4+
void main() {
5+
group('HttpStatusChecker', () {
6+
test('isInformational returns true for 1xx codes', () {
7+
expect(HttpStatusChecker.isInformational(100), isTrue);
8+
expect(HttpStatusChecker.isInformational(150), isTrue);
9+
expect(HttpStatusChecker.isInformational(199), isTrue);
10+
expect(HttpStatusChecker.isInformational(200), isFalse);
11+
expect(HttpStatusChecker.isInformational(99), isFalse);
12+
});
13+
14+
test('isSuccessful returns true for 2xx codes', () {
15+
expect(HttpStatusChecker.isSuccessful(200), isTrue);
16+
expect(HttpStatusChecker.isSuccessful(250), isTrue);
17+
expect(HttpStatusChecker.isSuccessful(299), isTrue);
18+
expect(HttpStatusChecker.isSuccessful(199), isFalse);
19+
expect(HttpStatusChecker.isSuccessful(300), isFalse);
20+
});
21+
22+
test('isRedirect returns true for 3xx codes', () {
23+
expect(HttpStatusChecker.isRedirect(300), isTrue);
24+
expect(HttpStatusChecker.isRedirect(350), isTrue);
25+
expect(HttpStatusChecker.isRedirect(399), isTrue);
26+
expect(HttpStatusChecker.isRedirect(299), isFalse);
27+
expect(HttpStatusChecker.isRedirect(400), isFalse);
28+
});
29+
30+
test('isClientError returns true for 4xx codes', () {
31+
expect(HttpStatusChecker.isClientError(400), isTrue);
32+
expect(HttpStatusChecker.isClientError(450), isTrue);
33+
expect(HttpStatusChecker.isClientError(499), isTrue);
34+
expect(HttpStatusChecker.isClientError(399), isFalse);
35+
expect(HttpStatusChecker.isClientError(500), isFalse);
36+
});
37+
38+
test('isServerError returns true for 5xx codes', () {
39+
expect(HttpStatusChecker.isServerError(500), isTrue);
40+
expect(HttpStatusChecker.isServerError(550), isTrue);
41+
expect(HttpStatusChecker.isServerError(599), isTrue);
42+
expect(HttpStatusChecker.isServerError(499), isFalse);
43+
expect(HttpStatusChecker.isServerError(600), isFalse);
44+
});
45+
46+
test('isInvalidStatus returns true for codes < 100 or >= 600', () {
47+
expect(HttpStatusChecker.isInvalidStatus(99), isTrue);
48+
expect(HttpStatusChecker.isInvalidStatus(600), isTrue);
49+
expect(HttpStatusChecker.isInvalidStatus(700), isTrue);
50+
expect(HttpStatusChecker.isInvalidStatus(100), isFalse);
51+
expect(HttpStatusChecker.isInvalidStatus(599), isFalse);
52+
});
53+
54+
test('isError returns true for 4xx, 5xx, and invalid codes', () {
55+
expect(HttpStatusChecker.isError(404), isTrue); // client error
56+
expect(HttpStatusChecker.isError(500), isTrue); // server error
57+
expect(HttpStatusChecker.isError(99), isTrue); // invalid
58+
expect(HttpStatusChecker.isError(600), isTrue); // invalid
59+
expect(HttpStatusChecker.isError(200), isFalse); // success
60+
expect(HttpStatusChecker.isError(301), isFalse); // redirect
61+
});
62+
});
63+
}

0 commit comments

Comments
 (0)