Skip to content

Commit

Permalink
fix: reset password checks with languages (#787)
Browse files Browse the repository at this point in the history
Impacted files:
* `open_food_api_client.dart`: relying on html tags for error detection instead of English localizations
* `user_management_test_prod.dart`: additional tests with / without language/country and on existing / not existing users
  • Loading branch information
monsieurtanuki authored Aug 17, 2023
1 parent 5dabb08 commit 0fee449
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 36 deletions.
53 changes: 32 additions & 21 deletions lib/src/open_food_api_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1186,8 +1186,10 @@ class OpenFoodAPIClient {
}

/// Uses reset_password.pl to send a password reset Email
/// needs only
/// Returns [Status.status] 200 = complete; 400 = wrong inputs or other error + [Status.error]; 500 = server error;
///
/// Returns a [Status]
/// * if [Status.status] == 200, that's OK
/// * if [Status.status] == 400, wrong inputs or other error
///
/// By default the email will be sent in English, please provide a [language]
/// and/or a [country] to have a localized content
Expand All @@ -1197,7 +1199,7 @@ class OpenFoodAPIClient {
final OpenFoodFactsLanguage? language,
final OpenFoodFactsCountry? country,
}) async {
var passwordResetUri = UriHelper.getUri(
Uri passwordResetUri = UriHelper.getUri(
path: '/cgi/reset_password.pl',
queryType: queryType,
addUserAgentParameters: false,
Expand All @@ -1211,14 +1213,14 @@ class OpenFoodAPIClient {
);
}

Map<String, String> data = <String, String>{
final Map<String, String> data = <String, String>{
'userid_or_email': emailOrUserID,
'action': 'process',
'type': 'send_email',
'.submit': 'Submit',
};

Status status = await HttpHelper().doMultipartRequest(
final Status status = await HttpHelper().doMultipartRequest(
passwordResetUri,
data,
queryType: queryType,
Expand All @@ -1229,23 +1231,32 @@ class OpenFoodAPIClient {
error:
'No response, open an issue here: https://github.com/openfoodfacts/openfoodfacts-dart/issues/new',
);
} else if (status.body!.contains('There is no account with this email')) {
return Status(
status: 400,
body: 'There is no account with this email',
);
} else if (status.body!.contains('has been sent to the e-mail address')) {
return Status(
status: 200,
body:
'An email with a link to reset your password has been sent to the e-mail address associated with your account.',
);
} else if (status.status is int && status.status < 500) {
return status.copyWith(status: 400);
} else {
/// Trigger real 5xx errors
return status;
}
// Possible strings found in the resulting html.
// Basically, if we see explicit errors or an html form, it's not good.
const List<String> errors = <String>[
// display of single errors
'<li class="error">',
// display of errors: start
'<!-- start templates/web/common/includes/error_list.tt.html -->',
// display of errors: end
'<!-- end templates/web/common/includes/error_list.tt.html -->',
// html label for user form field
'<label for="userid_or_email">',
// html form
'<form method="post" action="/cgi/reset_password.pl" enctype="multipart/form-data">',
];
for (final String error in errors) {
if (status.body!.contains(error)) {
return Status(
status: 400,
// I know, that's a bit bold to say so.
body: 'There is no account with this email',
);
}
}
// if we're lucky, we'll have a 200 status.
return status;
}

/// Returns the nutrient hierarchy specific to a country, localized.
Expand Down
68 changes: 53 additions & 15 deletions test/user_management_test_prod.dart
Original file line number Diff line number Diff line change
Expand Up @@ -124,24 +124,62 @@ void main() {
expect(status.userId, TestConstants.PROD_USER.userId);
});

test('Reset password', () async {
Status status =
await OpenFoodAPIClient.resetPassword(TestConstants.TEST_USER.userId);
group('reset password', () {
final String existingUser = TestConstants.TEST_USER.userId;
final String notExistingUser =
'${TestConstants.TEST_USER.userId}...@comfjdklf';
const int okStatus = 200;
const int koStatus = 400;

test('Reset password for existing user (no country, no language)',
() async {
final Status status = await OpenFoodAPIClient.resetPassword(
existingUser,
);
expect(status.status, okStatus);
});

expect(status.status, 200);
});
test('Reset password for not existing user (no country, no language)',
() async {
final Status status = await OpenFoodAPIClient.resetPassword(
notExistingUser,
);
expect(status.status, koStatus);
});

test('Reset password in French', () async {
Status status = await OpenFoodAPIClient.resetPassword(
TestConstants.TEST_USER.userId,
language: OpenFoodFactsLanguage.FRENCH,
);
test('Reset password in French for existing user', () async {
final Status status = await OpenFoodAPIClient.resetPassword(
existingUser,
language: OpenFoodFactsLanguage.FRENCH,
);
expect(status.status, okStatus);
});

expect(
status.body!,
contains(
'Un e-mail avec un lien pour vous permettre de changer le mot de passe a été envoyé'),
);
test('Reset password in French for not existing user', () async {
final Status status = await OpenFoodAPIClient.resetPassword(
notExistingUser,
language: OpenFoodFactsLanguage.FRENCH,
);
expect(status.status, koStatus);
});

test('Reset password in BE_nl for existing user', () async {
final Status status = await OpenFoodAPIClient.resetPassword(
existingUser,
language: OpenFoodFactsLanguage.DUTCH,
country: OpenFoodFactsCountry.BELGIUM,
);
expect(status.status, okStatus);
});

test('Reset password in BE_nl for not existing user', () async {
final Status status = await OpenFoodAPIClient.resetPassword(
notExistingUser,
language: OpenFoodFactsLanguage.DUTCH,
country: OpenFoodFactsCountry.BELGIUM,
);
expect(status.status, koStatus);
});
});
}

Expand Down

0 comments on commit 0fee449

Please sign in to comment.