Skip to content

Commit

Permalink
fix(core): Require current password on password change (#285)
Browse files Browse the repository at this point in the history
Increase security by requiring the current password when changing the
password. This increases the security for cases such as XSS, or just a
forgotten browser window left open.

Fixes #4140
  • Loading branch information
the-nic committed Jul 27, 2020
1 parent 03d8ed5 commit 2300fe8
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 17 deletions.
22 changes: 12 additions & 10 deletions UI/MainUI/SOGoRootPage.m
Expand Up @@ -133,7 +133,7 @@ - (WOCookie *) _cookieWithUsername: (NSString *) username
}

[loginCookie setPath: [NSString stringWithFormat: @"/%@/", appName]];

return loginCookie;
}

Expand Down Expand Up @@ -188,14 +188,14 @@ - (WOResponse *) connectAction
NSDictionary *params;
NSString *username, *password, *language, *domain, *remoteHost, *verificationCode;
NSArray *supportedLanguages, *creds;

SOGoPasswordPolicyError err;
int expire, grace;
BOOL rememberLogin, b;

err = PolicyNoError;
expire = grace = -1;

auth = [[WOApplication application] authenticatorInContext: context];
request = [context request];
params = [[request contentAsString] objectFromJSONString];
Expand All @@ -209,10 +209,10 @@ - (WOResponse *) connectAction
/* this will always be set to something more or less useful by
* [WOHttpTransaction applyAdaptorHeadersWithHttpRequest] */
remoteHost = [request headerForKey:@"x-webobjects-remote-host"];

if ((b = [auth checkLogin: username password: password domain: &domain
perr: &err expire: &expire grace: &grace useCache: NO])
&& (err == PolicyNoError)
&& (err == PolicyNoError)
// no password policy
&& ((expire < 0 && grace < 0) // no password policy or everything is alright
|| (expire < 0 && grace > 0) // password expired, grace still permits login
Expand All @@ -221,7 +221,7 @@ - (WOResponse *) connectAction
NSDictionary *json;

[self logWithFormat: @"successful login from '%@' for user '%@' - expire = %d grace = %d", remoteHost, username, expire, grace];

// We get the proper username for cookie creation. If we are using a multidomain
// environment with SOGoEnableDomainBasedUID, we could have to append the domain
// to the username. Also when SOGoEnableDomainBasedUID is enabled, we could be in
Expand Down Expand Up @@ -648,7 +648,7 @@ - (WOResponse *) changePasswordAction

request = [context request];
message = [[request contentAsString] objectFromJSONString];

auth = [[WOApplication application]
authenticatorInContext: context];
value = [[context request]
Expand All @@ -662,6 +662,8 @@ - (WOResponse *) changePasswordAction
password: &password];

newPassword = [message objectForKey: @"newPassword"];
// overwrite the value from the session to compare the actual input
password = [message objectForKey: @"oldPassword"];

um = [SOGoUserManager sharedUserManager];

Expand All @@ -673,7 +675,7 @@ - (WOResponse *) changePasswordAction
perr: &error])
{
// We delete the previous session
[SOGoSession deleteValueForSessionKey: [creds objectAtIndex: 1]];
[SOGoSession deleteValueForSessionKey: [creds objectAtIndex: 1]];

if ([domain isNotNull])
{
Expand All @@ -682,7 +684,7 @@ - (WOResponse *) changePasswordAction
[username rangeOfString: @"@"].location == NSNotFound)
username = [NSString stringWithFormat: @"%@@%@", username, domain];
}

response = [self responseWith204];
authCookie = [auth cookieWithUsername: username
andPassword: newPassword
Expand Down
1 change: 1 addition & 0 deletions UI/PreferencesUI/English.lproj/Localizable.strings
Expand Up @@ -238,6 +238,7 @@
"Additional Parameters" = "Additional Parameters";

/* password */
"Current password" = "Current password";
"New password" = "New password";
"Confirmation" = "Confirmation";
"Change" = "Change";
Expand Down
1 change: 1 addition & 0 deletions UI/PreferencesUI/German.lproj/Localizable.strings
Expand Up @@ -233,6 +233,7 @@
"Additional Parameters" = "Zusätzliche Einstellungen";

/* password */
"Current password" = "Aktuelles Passwort";
"New password" = "Neues Passwort";
"Confirmation" = "Bestätigung";
"Change" = "Ändern";
Expand Down
9 changes: 7 additions & 2 deletions UI/Templates/PreferencesUI/UIxPreferences.wox
Expand Up @@ -262,16 +262,21 @@
label:label="Password">
<md-content id="passwordView" class="md-padding">
<div layout="row">
<md-input-container class="md-block" flex="50">
<label><var:string label:value="Current password"/>
</label>
<input type="password" sg-no-dirty-check="true" ng-model="app.passwords.oldPassword"/>
</md-input-container>
<md-input-container class="md-block" flex="50">
<label><var:string label:value="New password"/>
</label>
<input type="password" sg-no-dirty-check="true" ng-model="app.passwords.newPassword"/>
<input type="password" sg-no-dirty-check="true" ng-model="app.passwords.newPassword"/>
</md-input-container>

<md-input-container class="md-block" flex="50">
<label><var:string label:value="Confirmation"/>
</label>
<input type="password" sg-no-dirty-check="true" ng-model="app.passwords.newPasswordConfirmation"/>
<input type="password" sg-no-dirty-check="true" ng-model="app.passwords.newPasswordConfirmation"/>
</md-input-container>
</div>
<div layout="row" layout-align="end center">
Expand Down
4 changes: 2 additions & 2 deletions UI/WebServerResources/js/Common/Authentication.service.js
Expand Up @@ -139,7 +139,7 @@
return d.promise;
}, // login: function(data) { ...

changePassword: function(newPassword) {
changePassword: function(newPassword, oldPassword) {
var d = $q.defer(),
xsrfCookie = $cookies.get('XSRF-TOKEN');

Expand All @@ -151,7 +151,7 @@
headers: {
'X-XSRF-TOKEN' : xsrfCookie
},
data: { newPassword: newPassword }
data: { newPassword: newPassword, oldPassword: oldPassword }
}).then(d.resolve, function(response) {
var error,
data = response.data,
Expand Down
7 changes: 4 additions & 3 deletions UI/WebServerResources/js/Preferences/PreferencesController.js
Expand Up @@ -13,7 +13,7 @@

this.$onInit = function() {
this.preferences = Preferences;
this.passwords = { newPassword: null, newPasswordConfirmation: null };
this.passwords = { newPassword: null, newPasswordConfirmation: null, oldPassword: null };
this.timeZonesList = $window.timeZonesList;
this.timeZonesSearchText = '';
this.sieveVariablesCapability = ($window.sieveCapabilities.indexOf('variables') >= 0);
Expand Down Expand Up @@ -465,14 +465,15 @@
this.canChangePassword = function() {
if (this.passwords.newPassword && this.passwords.newPassword.length > 0 &&
this.passwords.newPasswordConfirmation && this.passwords.newPasswordConfirmation.length &&
this.passwords.newPassword == this.passwords.newPasswordConfirmation)
this.passwords.newPassword == this.passwords.newPasswordConfirmation &&
this.passwords.oldPassword && this.passwords.oldPassword.length > 0)
return true;

return false;
};

this.changePassword = function() {
Authentication.changePassword(this.passwords.newPassword).then(function() {
Authentication.changePassword(this.passwords.newPassword, this.passwords.oldPassword).then(function() {
var alert = $mdDialog.alert({
title: l('Password'),
content: l('The password was changed successfully.'),
Expand Down

0 comments on commit 2300fe8

Please sign in to comment.