collective / dexterity.membrane

enables dexterity content items to be used as users and groups in Plone sites
3 stars 14 forks source link

Bcrypt passwords #15

Closed mattss closed 9 years ago

mattss commented 9 years ago

This changes the default encryption to bcrypt. Uses AccessControl to provide backwards compatibility with existing SSHA password hashes.

mattss commented 9 years ago

Security experts have been recommending moving away from SHA1 for some time now.

Ref: https://www.schneier.com/blog/archives/2012/10/when_will_we_se.html

mattss commented 9 years ago

@mauritsvanrees Any thoughts on this? We are hoping to improve the encryption used on Plone Intranet, which uses dexterity.membrane under the hood.

hvelarde commented 9 years ago

awesome!

mgrbyte commented 9 years ago

Anyone able to help with the travis failure on 4.2?

hvelarde commented 9 years ago

I think the problem is this line in the configure.zcml file:

<includeDependencies package="." />

when running tests, z3c.autoinclude is ignored and the package is not initialized by Zope.

you have 2 options:

mauritsvanrees commented 9 years ago

Looks good to me, pending Travis approval. I added one comment.

mgrbyte commented 9 years ago

@mauritsvanrees @mattss https://github.com/zopefoundation/AccessControl/pull/11

mgrbyte commented 9 years ago

@mauritsvanrees probably best to hold off on merging this until we know the chances of it going into AccessControl?

mauritsvanrees commented 9 years ago

I do not expect an AccessControl release soon. But even if that happens, that would preferably then be added to a new Zope2 release first, or Plone would need to override the version and we wait for a new Plone release.

So: it's fine with me to merge this. But as indicated I would like to have the registerEncoding call in a condition:

if 'BCRYPT' not in AuthEncoding.listSchemes(): ...
mgrbyte commented 9 years ago

@mauritsvanrees Thanks for the feedback, if there's more I can do here, let me know.

mauritsvanrees commented 9 years ago

Looks good, let me merge this.

mauritsvanrees commented 9 years ago

I have released 1.1.0 with this. Travis passes on Plone 4.2, 4.3, 5.0. Thanks.

I wanted to update the pin in ploneintranet, but apparently we do not believe in pinning there...

mattss commented 9 years ago

@mauritsvanrees Thanks for the merge and release. I'm just looking at getting this into plone intranet now.