modxcms / revolution

MODX Revolution - Content Management Framework
https://modx.com/
GNU General Public License v2.0
1.36k stars 529 forks source link

Invalid Password Specified : Inability to set Strong passwords #9086

Closed modxbot closed 6 years ago

modxbot commented 11 years ago

everettg_99 created Redmine issue ID 9086

The manager is not allowing certain passwords, e.g.

juju59)sails

Seems the closing parenthesis is the offender.

It's important that users be able to set strong passwords. Not sure why this error is popping up but it's occurred on 2 separate installs of 2.2.5-pl.

opengeek commented 11 years ago

opengeek submitted:

You cannot use any of the following characters/strings in MODX passwords at the current time:

array("'",'"','(',')',';','>','<','../');

This has always been the case in Revolution.

I would argue however, that those are not really the strongest passwords, just because they contain symbols. http://xkcd.com/936/

wuuti commented 9 years ago

The problem is combinated with an UI bug: if you try to save a new password with restricted characters in it (or if it is too short), the UI suggests that the save was somehow successfull. The only sign of an error is a small red border around the password field. That's all. The error message is only revealed if you hover your mouse over the password field. This is the case if you try to set a password in user management. In "Reset Password" at user profile the error message is shown.

OptimusCrime commented 9 years ago

Why does Modx limit the characters allowed in a password?

hackel commented 8 years ago

No resolution to this in over 3 years? Really?

This is quite a pretty serious bug. Not only should there be no restrictions on the characters allowed in a password (any utf-8 character should be valid), there is currently NO indication in the UI as to which characters are forbidden. This requires users to either just guess or hunt down this bug report. Ridiculous. Most of us use password managers of some kind now that generate strong passwords like e.g. Nf#z0->]POvn}ueD7yEa<v:j\F>4uKJ@ which Modx silently rejected.

pyrographics commented 8 years ago

I know this is an old ticket but thought this may be related so consider this a bump. MODX 2.5 accepted a > character in the password on setup but was not able to login after using it. I've encountered this several times.

Jako commented 8 years ago

Since the entered password does not have to be shown again, the htmlspecialchars restriction does not count here to avoid xss.

opengeek commented 8 years ago

I don't believe this is why the restriction is in place @Jako — I believe this is somehow related to encoding certain Javascript data for ExtJS in the connector responses — I can't recall exactly where the problem is but for some reason I believe it has everything to do with https://github.com/modxcms/revolution/blob/2.x/core/model/modx/modconnectorresponse.class.php#L223

Jako commented 8 years ago

The reason seems terribly logical. We need a check in the installer then and some message in the password change field.

BTW: The following ExtJS combo code should be quite nice to generate a new password with char restrictions.

MODx.combo.RandomString = function (config) {
    config = config || {};
    Ext.applyIf(config, {
        randomLength: 30,
        randomChars: '123456789abcdefghiklmnopqrstuwxyz',
        triggerAction: 'all',
        triggerClass: 'x-form-random-trigger',
        isUpdate: config.isUpdate,
        inputType: (config.isUpdate) ? 'password' : 'text'
    });
    MODx.combo.RandomString.superclass.constructor.call(this, config);
    this.config = config;
    if (!this.config.isUpdate) {
        this.setValue(this.createRandomString(this.config.randomLength, this.config.randomChars));
    }
};
Ext.extend(MODx.combo.RandomString, Ext.form.TriggerField, {
    onTriggerClick: function () {
        if (this.disabled) {
            return;
        }
        this.inputType = 'text';
        this.el.dom.type = 'text';
        this.submitValue = true;
        this.el.dom.name = this.config.name;
        this.setValue(this.createRandomString(this.config.randomLength, this.config.randomChars));
    },
    onDestroy: function () {
        MODx.combo.RandomString.superclass.onDestroy.call(this);
    },
    createRandomString: function (length, chars) {
        var result = '';
        for (var i = length; i > 0; --i) result += chars[Math.floor(Math.random() * chars.length)];
        return result;
    }
});
Ext.reg('modx-combo-randomstring', MODx.combo.RandomString);
Jako commented 6 years ago

Since the validating errors are shown now with the right message and the ExtJS/connector char restriction can't be solved at the moment I close this one.