add new @login endpoint to return available external login options #1757

erral · 2024-02-28T19:01:11Z

This PR adds a generic GET /@login endpoint that would expose links to additional login providers in a Plone site.

This way the endpoints added in packages like pas.plugins.authomatic and pas.plugins.oidc could be exchanged with adapter registrations and allow both products to be installed in the same Plone site for its use.

I am open to discuss whether the adapter needs to adapt just the IPloneSiteRoot object or perhaps may be a multiadapter including the request (perhaps to register the adapter for a given IThemeSpecific interface) or any other things.

mister-roboto · 2024-02-28T19:01:20Z

@erral thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

netlify · 2024-02-28T19:01:28Z

✅ Deploy Preview for plone-restapi canceled.

Name	Link
🔨 Latest commit	`f52a73d`
🔍 Latest deploy log	https://app.netlify.com/sites/plone-restapi/deploys/65f4b29ed1a9c2000846c1d8

erral · 2024-02-28T19:13:43Z

@jenkins-plone-org please run jobs

stevepiercy

This needs tests and documentation, similar to other endpoints. https://6.docs.plone.org/plone.restapi/docs/source/endpoints/index.html

news/1757.feature

Co-authored-by: Steve Piercy <web@stevepiercy.com>

erral · 2024-02-29T09:52:45Z

This needs tests and documentation, similar to other endpoints. https://6.docs.plone.org/plone.restapi/docs/source/endpoints/index.html

Yes, I am preparing that.

news/1757.feature

Co-authored-by: Steve Piercy <web@stevepiercy.com>

erral · 2024-03-01T08:58:11Z

@jenkins-plone-org please run jobs

stevepiercy

I decided it would be faster, easier, and less frustrating to make the changes directly instead of a proper review. I hope that is OK.

erral · 2024-03-01T09:29:47Z

I decided it would be faster, easier, and less frustrating to make the changes directly instead of a proper review. I hope that is OK.

Thanks! ❤️

erral · 2024-03-01T11:15:50Z

I am investigating why the output for the documentation is empty.

stevepiercy · 2024-03-01T11:46:32Z

@erral what output? The preview looks good: https://deploy-preview-1757--plone-restapi.netlify.app/endpoints/login.html

erral · 2024-03-01T12:48:12Z

@stevepiercy the last output, this one:

HTTP/1.1 200 OK
Content-Type: application/json

{
    "options": []
}

That should not be empty :/

stevepiercy

Question for @erral

stevepiercy · 2024-03-01T13:11:50Z

src/plone/restapi/tests/test_documentation.py

+        response = self.api_session.get(
+            f"{self.portal.absolute_url()}/@login",
+        )
+        save_request_and_response_for_docs("external_authentication_links", response)


With my rename of the docs file, should this be changed as well?

Suggested change

save_request_and_response_for_docs("external_authentication_links", response)

save_request_and_response_for_docs("login", response)

No, this is for the test-generated request and response file. I would leave this as it is, because we already have login.req file in the http-examples folder

stevepiercy · 2024-03-01T13:12:59Z

It's not an issue with the docs, because the docs only pull from this file:

https://github.com/plone/plone.restapi/blob/erral-login-options/src/plone/restapi/tests/http-examples/external_authentication_links.resp

Can you show what the content of that file should be?

davisagli

I like this idea in general.

Things I'm wondering:

What about a site that wants to let the user choose between login from external providers and Plone's standard login?
What if the integrator wants more control over the order of different login options?

So I'm thinking that maybe there should be a registry setting with a list of login providers to show, and this endpoint could use the info from the registry setting plus the details from the IExternalLoginProviders adapters. But -- this can probably be added later, so I'm not necessarily asking you to do it as a condition for adding this endpoint.

docs/source/endpoints/login.md

davisagli · 2024-03-02T23:25:49Z

src/plone/restapi/interfaces.py

+    def get_providers():
+        """
+        return a list of login providers, with its id, title, plugin and url
+        """


Maybe this should be defined in plone.base instead of plone.restapi, so that an addon doesn't need to depend on plone.restapi just to register this adapter and provide information about what providers it supports?

On the other hand, that would make it harder to backport support for this feature to older versions of Plone.

I don't feel strongly about it but in general, plone.restapi's purpose is to provide API access to features that exist at a lower level and might also be accessed via other interfaces.

the whole idea of the endpoint is to avoid duplicated @login endpoints such the ones we have in pas.plugins.authomatic and pas.plugins.oidc to support volto-authomatic

I agree that perhaps this should go in plone.base somewhere and integrate it with Plone's standard login page.

Perhaps we can open an issue in Plone and work on that later on?

erral · 2024-03-03T18:46:34Z

It's not an issue with the docs, because the docs only pull from this file:

https://github.com/plone/plone.restapi/blob/erral-login-options/src/plone/restapi/tests/http-examples/external_authentication_links.resp

Can you show what the content of that file should be?

I have already fixed this. There was a bug in my code 😈

erral · 2024-03-03T19:01:33Z

@jenkins-plone-org please run jobs

stevepiercy

Preview of docs with update look good: https://deploy-preview-1757--plone-restapi.netlify.app/endpoints/login.html

erral · 2024-03-12T12:43:17Z

@jenkins-plone-org please run jobs

erral · 2024-08-20T17:18:04Z

@jenkins-plone-org please run jobs

erral · 2024-09-16T08:19:10Z

@jenkins-plone-org please run jobs

add new @login endpoint to return available external login options

cf79461

erral requested a review from ericof February 28, 2024 19:01

erral added 6 commits February 28, 2024 20:03

changelog

3ff293b

lint

783a079

lint

cc1347c

lint

6cf2636

lint

98c0cf5

lint

68ce3be

Merge branch 'main' into erral-login-options

46f891e

erral marked this pull request as ready for review February 28, 2024 19:20

stevepiercy requested changes Feb 28, 2024

View reviewed changes

news/1757.feature Outdated Show resolved Hide resolved

news/1757.feature Outdated Show resolved Hide resolved

erral and others added 2 commits February 29, 2024 10:52

Update news/1757.feature

879752e

Co-authored-by: Steve Piercy <web@stevepiercy.com>

Update news/1757.feature

53002d4

Co-authored-by: Steve Piercy <web@stevepiercy.com>

stevepiercy requested changes Feb 29, 2024

View reviewed changes

news/1757.feature Outdated Show resolved Hide resolved

erral and others added 2 commits February 29, 2024 12:33

Update news/1757.feature

37c47e7

Co-authored-by: Steve Piercy <web@stevepiercy.com>

add docs

06be87c

erral and others added 6 commits March 1, 2024 10:01

yaml

8168d4a

yaml

961def7

docs

3cd4daa

docs

0eccce4

Review of docs

499d74a

Revert ' to "

c97c97b

stevepiercy approved these changes Mar 1, 2024

View reviewed changes

stevepiercy reviewed Mar 1, 2024

View reviewed changes

davisagli reviewed Mar 2, 2024

View reviewed changes

erral added 5 commits March 3, 2024 19:27

properly implement the adapter in tests

4cc4288

add docs rsults

fd67f94

Merge branch 'main' into erral-login-options

1a0c196

black

861673f

fix response

7a9e378

This was referenced Mar 3, 2024

register the adapter instead of the @login endpoint collective/pas.plugins.authomatic#73

Draft

register the adapter instead of the @login endpoint collective/pas.plugins.oidc#47

Open

stevepiercy approved these changes Mar 4, 2024

View reviewed changes

Merge branch 'main' into erral-login-options

83c802e

erral added 2 commits March 15, 2024 21:42

Merge branch 'main' into erral-login-options

f52a73d

Merge branch 'main' into erral-login-options

92ae900

erral added 2 commits September 6, 2024 14:50

Merge branch 'main' into erral-login-options

49a018f

Merge branch 'main' into erral-login-options

b5c5899

Merge branch 'main' into erral-login-options

730e8d2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add new @login endpoint to return available external login options #1757

add new @login endpoint to return available external login options #1757

erral commented Feb 28, 2024 •

edited

Loading

mister-roboto commented Feb 28, 2024

netlify bot commented Feb 28, 2024 •

edited

Loading

erral commented Feb 28, 2024

stevepiercy left a comment

erral commented Feb 29, 2024

erral commented Mar 1, 2024

stevepiercy left a comment

erral commented Mar 1, 2024

erral commented Mar 1, 2024

stevepiercy commented Mar 1, 2024

erral commented Mar 1, 2024

stevepiercy left a comment

stevepiercy Mar 1, 2024

erral Mar 3, 2024

stevepiercy commented Mar 1, 2024

davisagli left a comment

davisagli Mar 2, 2024

erral Mar 3, 2024

erral commented Mar 3, 2024

erral commented Mar 3, 2024

stevepiercy left a comment

erral commented Mar 12, 2024

erral commented Aug 20, 2024

erral commented Sep 16, 2024

	save_request_and_response_for_docs("external_authentication_links", response)
	save_request_and_response_for_docs("login", response)

add new @login endpoint to return available external login options #1757

Are you sure you want to change the base?

add new @login endpoint to return available external login options #1757

Conversation

erral commented Feb 28, 2024 • edited Loading

mister-roboto commented Feb 28, 2024

netlify bot commented Feb 28, 2024 • edited Loading

✅ Deploy Preview for plone-restapi canceled.

erral commented Feb 28, 2024

stevepiercy left a comment

Choose a reason for hiding this comment

erral commented Feb 29, 2024

erral commented Mar 1, 2024

stevepiercy left a comment

Choose a reason for hiding this comment

erral commented Mar 1, 2024

erral commented Mar 1, 2024

stevepiercy commented Mar 1, 2024

erral commented Mar 1, 2024

stevepiercy left a comment

Choose a reason for hiding this comment

stevepiercy Mar 1, 2024

Choose a reason for hiding this comment

erral Mar 3, 2024

Choose a reason for hiding this comment

stevepiercy commented Mar 1, 2024

davisagli left a comment

Choose a reason for hiding this comment

davisagli Mar 2, 2024

Choose a reason for hiding this comment

erral Mar 3, 2024

Choose a reason for hiding this comment

erral commented Mar 3, 2024

erral commented Mar 3, 2024

stevepiercy left a comment

Choose a reason for hiding this comment

erral commented Mar 12, 2024

erral commented Aug 20, 2024

erral commented Sep 16, 2024

erral commented Feb 28, 2024 •

edited

Loading

netlify bot commented Feb 28, 2024 •

edited

Loading