Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ADD] 'auth_generate_password' module to generate randomly user's password #92

Merged
merged 11 commits into from
Sep 14, 2015

Conversation

legalsylvain
Copy link
Contributor

Propose to the community a simple module to:

  • block users that can not change them password;
  • generate password, and send to the users;

The type of password is defined in two parameters:

  • password_size (default : 6);
  • password_chars (default : string.ascii_letters + string.digits)
  • flake 8 / pylint : OK;
  • coverage of the module : ~ 95 %
  • change the module icon;
  • make this module compatible if password is encryted;

@legalsylvain
Copy link
Contributor Author

#91
@hbrunn , I change to use email.template. It works pretty fine but I set in the template "object.password". If the password is encrypted by another module, it will not work. Do you know how transmit a defined value to a template ?

UPDATE:

  • Fixed, using ctx.get to have the password in template;

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.33%) when pulling abc3852 on grap:7.0-auth_generate_password into 59f73f5 on OCA:7.0.

@legalsylvain
Copy link
Contributor Author

@ALL : Travis fails if 'mail_environment' is installed. ( so it doesn't fail in UNITEST mode);
The reason is that in my test.py file, I try to send mail and 'mail_environment' drops columns (making columns fields.function).
https://travis-ci.org/OCA/server-tools/jobs/43467649#L397

How can I solve the problem ?

UPDATE:

  • Problem fixed, deleting in the test, the ir_mail_server object.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.95%) when pulling e68717c on grap:7.0-auth_generate_password into 59f73f5 on OCA:7.0.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.13%) when pulling 2931114 on grap:7.0-auth_generate_password into 59f73f5 on OCA:7.0.

@hbrunn
Copy link
Member

hbrunn commented Dec 15, 2014

👍 (and sorry for not replying last week)

Please,
<ul>
<li>remember this password and delete this email;</li>
<li>Communicate the password to your team, if you are many people to use the same credentials;</li>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/if you are to use/if there are many people that are using

@coveralls
Copy link

Coverage Status

Coverage increased (+0.19%) when pulling 4b6bc92 on grap:7.0-auth_generate_password into 59f73f5 on OCA:7.0.

@pedrobaeza
Copy link
Member

👍

@legalsylvain
Copy link
Contributor Author

2 👍, but the runbot is failing.
I don't know how to identify the problem. (it is the first time I see the runbot interface)
Somebody can help me ?

et_obj = self.pool['email.template']
password_size = eval(icp_obj.get_param(
cr, uid, 'auth_generate_password.password_size'))
password_chars = eval(icp_obj.get_param(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you may

try:
   int(cp_obj.get_param(
        cr, uid, 'auth_generate_password.password_size'))
except:
    raise except_orm(_("error"), _("Only digit chars authorized"))

@bealdav
Copy link
Member

bealdav commented Jan 14, 2015

Except use of eval, it is ok for me

@legalsylvain
Copy link
Contributor Author

Thanks, but it only works for password_size not for password_chars.
I'll do the change.

@max3903 max3903 removed the 7.0 label Feb 17, 2015
@max3903 max3903 added this to the 7.0 milestone May 26, 2015
@legalsylvain
Copy link
Contributor Author

@pedrobaeza, @hbrunn, @bealdav : runbot is now green. (after a simple rebase).
This PR can be merged if you're OK.

kind regards.

@dreispt
Copy link
Sponsor Member

dreispt commented Sep 8, 2015

There is also a PR to prevent users from changing passwords: #249
I think that people could want random passwords but then let people change them.
Wouldn't it be best to have the two features in two separate modules?


- !record {model: ir.config_parameter, id: password_size}:
key: auth_generate_password.password_size
value: 6
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO 6 is quite a bad default. I would have 12, or at least 10.
Also, I guess this ought to be created with noupdate="1"...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you know how to set noupdate in a yml file ?
6 -> 12 : agree.

@legalsylvain
Copy link
Contributor Author

@dreispt :

There is also a PR to prevent users from changing passwords: #249
I think that people could want random passwords but then let people change them.
Wouldn't it be best to have the two features in two separate modules?

Yes, you're right. When I'll port this module, I'll remove this feature from this module. (my module is a V7, and you're pointing a V8 PR) ;

@dreispt
Copy link
Sponsor Member

dreispt commented Sep 8, 2015

I'm still concerned that parameters get overwritten on a module upgrade.

@legalsylvain
Copy link
Contributor Author

@dreispt :

I'm still concerned that parameters get overwritten on a module upgrade.

I can figure out your PoV, but it depends how do you apply settings on your database.
1/ you realize changes by UI:
-> if you don't set noupdate, a module upgrade will break the setting; (what you say)

2/ you realize changes by a custom module:
-> if you set noupdate, this will not work;

So, there is no perfect settings.

My personal case (2), I write a 'custom_module' and in this module I put all my specific configuration, and so I can recover a complete demo environment, if I want.
With what you propose, my kind of implementation will not work.

What do you think ?

But yes, I could understand that all ir.config_parameter should be noupdatable...
I don't know, if there are some conventions.

@dreispt
Copy link
Sponsor Member

dreispt commented Sep 9, 2015

@legalsylvain AFAIK mopdules should expect parameters to be modified by power users through the GUI. So 1/ is case to support, and Odoo has the tooling for that. You will need to convert the YML to XML.

@hbrunn
Copy link
Member

hbrunn commented Sep 9, 2015

or write a yaml stanza that modifies the generated ir.model.data record (I never use yaml, no idea if there's a better way to do it)

Concerning update/noupdate: You can have both of it. Set it to noupdate in this module, depending modules can do this:

<data noupdate="1">
    <function model="ir.model.data" name="write">
        <function model="ir.model.data" name="search">
            <value eval="[('module', '=', 'auth_generate_password'), ('name', '=', 'password_size')]" />
        </function>
        <value eval="{'noupdate': False}" />
    </function>
</data>
<data>
    <!-- change the noupdate record /-->
</data>
<data noupdate="1">
    <function model="ir.model.data" name="write">
        <function model="ir.model.data" name="search">
            <value eval="[('module', '=', 'auth_generate_password'), ('name', '=', 'password_size')]" />
        </function>
        <value eval="{'noupdate': True}" />
    </function>
</data>

This pattern allows us to change noupdate records on module installation, but doesn't mess with the records on subsequent updates

@legalsylvain
Copy link
Contributor Author

@hbrunn : Thanks a lot for this trick ! I didn't know. Very usefull.
@dreispt : Thanks for your review. I changed the code.

@dreispt
Copy link
Sponsor Member

dreispt commented Sep 9, 2015

All good, but it would be safer to add to the module description a "Roadmap" section explaining that the restrict password changing feature should be removed when porting to 8.0.

@legalsylvain
Copy link
Contributor Author

@dreispt : done.

@dreispt
Copy link
Sponsor Member

dreispt commented Sep 14, 2015

Thanks! 👍

hbrunn added a commit that referenced this pull request Sep 14, 2015
[ADD] 'auth_generate_password' module to generate randomly user's password
@hbrunn hbrunn merged commit 1587c39 into OCA:7.0 Sep 14, 2015
StefanRijnhart pushed a commit to StefanRijnhart/server-tools that referenced this pull request Feb 26, 2017
[PRT][REN] Added module web_hide_db_manager_link, as a port to v8 of web...
Garamotte pushed a commit to subteno-it/server-tools that referenced this pull request Jul 12, 2017
[FIX] base_report_to_printer: A button method should be @api.multi
petrus-v pushed a commit to foodles-tech/server-tools that referenced this pull request Mar 9, 2023
Signed-off-by simahawk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants