search

plone / Products.CMFPlone

The core of the Plone content management system
https://plone.org
GNU General Public License v2.0
246 stars 186 forks source link

PLIP: Replace portal_skins-based login with a browserview-based one #2092

Closed esteele closed 6 years ago

esteele commented 7 years ago

PLIP (Plone Improvement Proposal)

Responsible Persons

Proposer:

Eric Steele

Seconder:

Philip Bauer

Abstract

Remove the remaining skin scripts dealing with login in CMFPlone with a more modern implementation that is pluggable, easier to override, test and extend.

Motivation

Skin scripts and templates are deprecated and need to die.

Assumptions

Everyone wants this.

Proposal & Implementation

In 2014 at the Emerald Sprint the package https://github.com/plone/plone.login was created. It is based on z3c.form and is used in various projects but was never finished. Since then some aspects of it have been reimplemented (e.g. password-reset and registration-forms) in various places but the main thing (the login) is still based on skin scripts.

The new forms and views could either live in plone.login or moved to CMFPlone. For now we have them in plone.login so it can be used as a addon.

Adding oauth-features like a integration with https://github.com/collective/pas.plugins.authomatic is out of the scope of this PLIP.

Deliverables

Risks

Might break customized login-processes that rely on the old scripts.

Participants

ebrehault commented 7 years ago

This PLIP has been approved

datakurre commented 6 years ago

@esteele @pbauer Would you be able to summarize what remains still missing after your Midsummer Sprint effort in July, and how your work could be tested already? https://github.com/plone/plone.login/commits/master

Is it possible already to use plone.login as an add-on or would it require merging into CMFPlone before testing/using and polishing?

pbauer commented 6 years ago

@datakurre this basically works fine in 5.1. Only for self-registration to work you also need the branch plonelogin of plone.app.users. Give it a try!

The changes in CMFPlone are basically only removing the old templates and are not strictly needed to use is as an addon.

To make the PLIP mergeable we need to fix quite a number of tests (see http://jenkins.plone.org/view/PLIPs/job/plip-2092-login/). I will work on that when time allows.

pbauer commented 6 years ago

I fixed a couple of bugs that I found when testing it. In https://github.com/collective/demo.plone.de/pull/22 I use it in demo.plone.de.

jensens commented 6 years ago

Adding oauth-features like a integration with https://github.com/collective/pas.plugins.authomatic is out of the scope of this PLIP.

The old login supports External Logins. I expect the new login to also support this feature. It is broadly used in larger companies.

pbauer commented 6 years ago

@jensens True. Support for the existing settings external_login_url, external_logout_url and external_login_iframe is still missing. @esteele Can you remember if we talked about that in Finnland or did we simply miss it?

jensens commented 6 years ago

external_login_iframe scares me, we may want to deprecate that ;)

jensens commented 6 years ago

Btw.: The current way providing these urls is not good. But its used and plone.login should provide feature parity.

Having a way to plugin a subform or subtemplate or whatever its is called in the shiny z3cform world would be probably better. Also we then would have better UX and the ability to support multiple login methods (like username-password and OAuth2 with google, twitter and facebook). But this can be done in a next enhancement step and its ok to have it outside this plip.

jensens commented 6 years ago

I implemented external-login in the old way. external iframe support is untested, but should work. pas.plugins.authomatic integration works, so should others. More improvements needed, but could be done later.

Also template customization using z3c.jbot is now supported.

@agitator and I will do some field-testing of all this in customer projects.

Once found ready, it can be released as addon for Plone 5.1.

And, all code will be moved over to Products.CMFPlone in order to not introduce yet another package with some browser views. It can live side-by-side with password reset tool there, good place.

jensens commented 6 years ago

Ah, and i18n/l10n is missing. But there is a PR already (wip) https://github.com/plone/plone.login/pull/46

jensens commented 6 years ago

state: plone.login is ready now. I am mergin the code now into Products.CMFPlone (target 5.2 master).

jensens commented 6 years ago

to merge before standalone.

to merge together

for jenkins PR job

https://github.com/plone/Products.CMFPlone/pull/2445
https://github.com/plone/plone.app.upgrade/pull/165
jensens commented 6 years ago

Note: the po-files (there are already german translations in plone.login) are not moved to plone.app.locales for now.

jensens commented 6 years ago

plip job here https://jenkins.plone.org/view/PLIPs/job/plip-2092-login/

jensens commented 6 years ago

released https://pypi.org/project/plone.login/ for 5.1 (and probably 5.0 - untested) as rc1

jensens commented 6 years ago

Note: The external login of the merged plone.login does no longer provide the more or less hacky way to pass __ac cookie of wrong domain from some second Plone login site to the current site and use it as login credentials. At the Buschenschanksprint we decided its ok to remove this rarely used feature including its tests. If someone needs this, please create an addon supporting this. Better, SSO can be done easily with OAuth2/Shibboleth/SAML2 or others these days.

jensens commented 6 years ago

Ops and sorry - for 2 commits on master (plone.app.users/plone.app.portlets) - I just forgot I merged 'em already and branches were gone. But looks good so far.

jensens commented 6 years ago

All GREEN. I merge it now.