From c25a0707dcd2fdad123b57c9bf24f32637c7b260 Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Sun, 3 Mar 2024 18:13:38 +0100 Subject: [PATCH 01/18] add login with apple support --- setup.py | 1 + src/pas/plugins/oidc/browser/view.py | 34 ++++++++++++++- src/pas/plugins/oidc/plugins.py | 63 +++++++++++++++++++++++++++- src/pas/plugins/oidc/utils.py | 9 +++- 4 files changed, 104 insertions(+), 3 deletions(-) diff --git a/setup.py b/setup.py index c601816..99adb11 100644 --- a/setup.py +++ b/setup.py @@ -55,6 +55,7 @@ "plone.api", "plone.restapi>=8.34.0", "oic", + "PyJWT", ], extras_require={ "test": [ diff --git a/src/pas/plugins/oidc/browser/view.py b/src/pas/plugins/oidc/browser/view.py index 91868d6..bd21154 100644 --- a/src/pas/plugins/oidc/browser/view.py +++ b/src/pas/plugins/oidc/browser/view.py @@ -10,6 +10,9 @@ from urllib.parse import quote from zExceptions import Unauthorized +import json +import urllib.parse + class RequireLoginView(BrowserView): """Our version of the require-login view from Plone. @@ -120,6 +123,10 @@ def __call__(self): session = utils.load_existing_session(self.context, self.request) client = self.context.get_oauth2_client() qs = self.request.environ["QUERY_STRING"] + if not qs: + # With Apple, the response comes as a POST, thus it does not + # com in the QUERY_STRING but in the request.form + qs = urllib.parse.urlencode(self.request.form) args, state = utils.parse_authorization_response( self.context, qs, client, session ) @@ -130,10 +137,35 @@ def __call__(self): "phone_number_verified": utils.SINGLE_OPTIONAL_BOOLEAN_AS_STRING, } ) - # The response you get back is an instance of an AccessTokenResponse # or again possibly an ErrorResponse instance. + + if self.context.getProperty('apple_login_enabled'): + args.update( + { + "client_id": self.context.getProperty("client_id"), + "client_secret": self.context._build_apple_secret() + } + ) + + initial_user_info = {} + if self.context.getProperty('apple_login_enabled'): + # Let's check if this is this user's first login + # if so, their name and email could come in the first + # response from authorization response + # Weird Apple issues... + user = self.request.form.get('user', "") + if user: + user_decoded = json.loads(user) + first_name = user_decoded.get("name", {}).get("firstName", "") + last_name = user_decoded.get("name", {}).get("lastName", "") + email = user_decoded.get("email", "") + initial_user_info['given_name'] = first_name + initial_user_info['family_name'] = last_name + initial_user_info['email'] = email + user_info = utils.get_user_info(client, state, args) + user_info.update(initial_user_info) if user_info: self.context.rememberIdentity(user_info) self.request.response.setHeader( diff --git a/src/pas/plugins/oidc/plugins.py b/src/pas/plugins/oidc/plugins.py index a1a2c3c..e100867 100644 --- a/src/pas/plugins/oidc/plugins.py +++ b/src/pas/plugins/oidc/plugins.py @@ -6,6 +6,7 @@ from oic.oic.message import RegistrationResponse from oic.utils.authn.client import CLIENT_AUTHN_METHOD from pas.plugins.oidc import logger +from pas.plugins.oidc import PLUGIN_ID from plone.base.utils import safe_text from plone.protect.utils import safeWrite from Products.CMFCore.utils import getToolByName @@ -68,6 +69,9 @@ class OIDCPlugin(BasePlugin): use_deprecated_redirect_uri_for_logout = False use_modified_openid_schema = False user_property_as_userid = "sub" + apple_login_enabled = False + apple_consumer_team = "" + apple_consumer_id_key = "" _properties = ( dict(id="issuer", type="string", mode="w", label="OIDC/Oauth2 Issuer"), @@ -135,8 +139,37 @@ class OIDCPlugin(BasePlugin): mode="w", label="User info property used as userid, default 'sub'", ), + dict( + id="apple_login_enabled", + type="boolean", + mode="w", + label="Check if you want to login with Apple", + ), + dict( + id="apple_consumer_team", + type="string", + mode="w", + label="Apple consumer team as defined by Apple", + ), + dict( + id="apple_consumer_id_key", + type="string", + mode="w", + label="Apple consumer id key as defined by Apple", + ), + ) +<<<<<<< HEAD +======= + APPLE_TOKEN_TTL_SEC = 6 * 30 * 24 * 60 * 60 + APPLE_TOKEN_AUDIENCE = "https://appleid.apple.com" + + def __init__(self, id, title=None): + self._setId(id) + self.title = title + +>>>>>>> 5c68af3 (add login with apple support) def rememberIdentity(self, userinfo): if not isinstance(userinfo, (OpenIDSchema, dict)): raise AssertionError( @@ -295,6 +328,26 @@ def _setupJWTTicket(self, user_id, user): # TODO: take care of path, cookiename and domain options ? response.setCookie("auth_token", token, path="/") + def _build_apple_secret(self): + now = int(time.time()) + + client_id = self.getProperty("client_id") + team_id = self.getProperty('apple_consumer_team') + key_id = self.getProperty('apple_consumer_id_key') + private_key = self.getProperty('client_secret') + + headers = {"kid": key_id} + payload = { + "iss": team_id, + "iat": now, + "exp": now + self.APPLE_TOKEN_TTL_SEC, + "aud": self.APPLE_TOKEN_AUDIENCE, + "sub": client_id, + } + + private_key = f'-----BEGIN PRIVATE KEY-----\n{private_key}\n-----END PRIVATE KEY-----' + return jwt.encode(payload, key=private_key.encode(), algorithm="ES256", headers=headers) + # TODO: memoize (?) def get_oauth2_client(self): try: @@ -307,8 +360,16 @@ def get_oauth2_client(self): provider_info = client.provider_config(self.getProperty("issuer")) # noqa info = { "client_id": self.getProperty("client_id"), - "client_secret": self.getProperty("client_secret"), + "token_endpoint_auth_method": provider_info.get( + "token_endpoint_auth_methods_supported" + ), } + + if self.getProperty('apple_login_enabled'): + info.update({"client_secret": self._build_apple_secret()}) + else: + info.update({"client_secret": self.getProperty("client_secret")}) + client_reg = RegistrationResponse(**info) client.store_registration_info(client_reg) return client diff --git a/src/pas/plugins/oidc/utils.py b/src/pas/plugins/oidc/utils.py index 4102a96..e04fb92 100644 --- a/src/pas/plugins/oidc/utils.py +++ b/src/pas/plugins/oidc/utils.py @@ -127,6 +127,10 @@ def authorization_flow_args(plugin: plugins.OIDCPlugin, session: Session) -> dic # and send it in the request as a base64-encoded urlsafe string of the sha256 hash of that string args["code_challenge"] = pkce_code_verifier_challenge(session.get("verifier")) args["code_challenge_method"] = "S256" + + if plugin.getProperty('apple_login_enabled'): + args['response_mode'] = 'form_post' + return args @@ -168,13 +172,14 @@ def get_user_info(client, state, args) -> Union[message.OpenIDSchema, dict]: resp = client.do_access_token_request( state=state, request_args=args, - authn_method="client_secret_basic", + authn_method=client.registration_response.get('token_endpoint_auth_method', 'client_secret_basic'), ) user_info = {} if isinstance(resp, message.AccessTokenResponse): # If it's an AccessTokenResponse the information in the response will be stored in the # client instance with state as the key for future use. user_info = resp.to_dict().get("id_token", {}) + if client.userinfo_endpoint: # https://openid.net/specs/openid-connect-core-1_0.html#UserInfo @@ -193,6 +198,8 @@ def get_user_info(client, state, args) -> Union[message.OpenIDSchema, dict]: exc_info=exc, ) user_info = {} + else: + pass # userinfo in an instance of OpenIDSchema or ErrorResponse # It could also be dict, if there is no userinfo_endpoint if not (user_info and isinstance(user_info, (message.OpenIDSchema, dict))): From bf7e98079e5d63118831e150ee0a5c0477f23500 Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Sun, 3 Mar 2024 18:29:42 +0100 Subject: [PATCH 02/18] docs --- README.md | 147 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 85 insertions(+), 62 deletions(-) diff --git a/README.md b/README.md index bb0a712..d8bd534 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,6 @@ [![PyPI - License](https://img.shields.io/pypi/l/pas.plugins.oidc)](https://pypi.org/project/pas.plugins.oidc/) [![PyPI - Status](https://img.shields.io/pypi/status/pas.plugins.oidc)](https://pypi.org/project/pas.plugins.oidc/) - [![PyPI - Plone Versions](https://img.shields.io/pypi/frameworkversions/plone/pas.plugins.oidc)](https://pypi.org/project/pas.plugins.oidc/) [![Meta](https://github.com/collective/pas.plugins.oidc/actions/workflows/meta.yml/badge.svg)](https://github.com/collective/pas.plugins.oidc/actions/workflows/meta.yml) @@ -22,6 +21,7 @@ ## Intro + This is a Plone authentication plugin for OpenID Connect. OAuth 2.0 should work as well because OpenID Connect is built on top of this protocol. @@ -40,40 +40,39 @@ This package supports Plone sites using Volto and ClassicUI. For proper Volto support, the requirements are: -* plone.restapi >= 8.34.0 -* Volto >= 16.10.0 +- plone.restapi >= 8.34.0 +- Volto >= 16.10.0 Add **pas.plugins.oidc** to the Plone installation using `pip`: -``bash +`bash pip install pas.plugins.oidc -`` +` ### Requirements -As of version 2.* of this package the minimum requirements are Plone 6.0 and python 3.8. +As of version 2.\* of this package the minimum requirements are Plone 6.0 and python 3.8. ### Warning Pay attention to the customization of `User info property used as userid` field, with the wrong configuration it's easy to impersonate another user. - ## Configure the plugin -* Go to the Add-ons control panel and install `pas.plugins.oidc`. -* In the ZMI go to the plugin properties at `http://localhost:8080/Plone/acl_users/oidc/manage_propertiesForm` -* Configure the properties with the data obtained from your provider: - * `OIDC/Oauth2 Issuer` - * `Client ID` - * `Client secret` - * `redirect_uris`: this needs to match the **public URL** where the user will be redirected after the login flow is completed. It needs to include +- Go to the Add-ons control panel and install `pas.plugins.oidc`. +- In the ZMI go to the plugin properties at `http://localhost:8080/Plone/acl_users/oidc/manage_propertiesForm` +- Configure the properties with the data obtained from your provider: + - `OIDC/Oauth2 Issuer` + - `Client ID` + - `Client secret` + - `redirect_uris`: this needs to match the **public URL** where the user will be redirected after the login flow is completed. It needs to include the `/Plone/acl_users/oidc/callback` part. When using Volto you need to expose Plone somehow to have the login process finish correctly. - * `Use Zope session data manager`: see the section below about the usage of session. - * `Create user / update user properties`: when selected the user data in Plone will be updated with the data coming from the OIDC provider. - * `Create authentication __ac ticket`: when selected the user will be allowed to act as a logged-in user in Plone. - * `Create authentication auth_token (Volto/REST API) ticket`: when selected the user will be allowed to act as a logged-in user in the Volto frontend. - * `Open ID scopes to request to the server`: information requested to the OIDC provider. Leave it as it is or modify it according to your provider's information. - * `Use PKCE`: when enabled uses [PKCE](https://datatracker.ietf.org/doc/html/rfc7636) when requesting authentication from the provider. + - `Use Zope session data manager`: see the section below about the usage of session. + - `Create user / update user properties`: when selected the user data in Plone will be updated with the data coming from the OIDC provider. + - `Create authentication __ac ticket`: when selected the user will be allowed to act as a logged-in user in Plone. + - `Create authentication auth_token (Volto/REST API) ticket`: when selected the user will be allowed to act as a logged-in user in the Volto frontend. + - `Open ID scopes to request to the server`: information requested to the OIDC provider. Leave it as it is or modify it according to your provider's information. + - `Use PKCE`: when enabled uses [PKCE](https://datatracker.ietf.org/doc/html/rfc7636) when requesting authentication from the provider. ### Login and Logout URLs @@ -81,27 +80,26 @@ Pay attention to the customization of `User info property used as userid` field, When using this plugin with a [Volto frontend](https://6.docs.plone.org/volto/index.html), please install [@plone-collective/volto-authomatic](https://github.com/collective/volto-authomatic) add-on on your frontend project. -* **Login URL**: ``/login -* **Logout URL**: ``/logout +- **Login URL**: ``/login +- **Logout URL**: ``/logout Also, on the OpenID provider, configure the Redirect URL as **``/login_oidc/oidc**. - #### Classic UI -When using this plugin with *Plone 6 Classic UI* the standard URLs used for login (`http://localhost:8080/Plone/login`) and logout (`http://localhost:8080/Plone/logout`) +When using this plugin with _Plone 6 Classic UI_ the standard URLs used for login (`http://localhost:8080/Plone/login`) and logout (`http://localhost:8080/Plone/logout`) will not trigger the usage of the plugin. To login into a site using the OIDC provider, you will need to change those login URLs to the following: -* **Login URL**: /``/acl_users/``/login -* **Logout URL**: /``/acl_users/``/logout +- **Login URL**: /``/acl_users/``/login +- **Logout URL**: /``/acl_users/``/logout -*Where:* +_Where:_ - * `Plone Site Id`: is the id you gave to the Plone site when you created it. It is usually `Plone` but may vary. It is the last part of the URL when you browse Plone directly without using any proxy server, ex. `http://localhost:8080/Plone+` -> `Plone`. +- `Plone Site Id`: is the id you gave to the Plone site when you created it. It is usually `Plone` but may vary. It is the last part of the URL when you browse Plone directly without using any proxy server, ex. `http://localhost:8080/Plone+` -> `Plone`. - * `oidc pas plugin id`: is the id you gave to the OIDC plugin when you created it inside the Plone PAS administration panel. If you just used the default configuration and installed this plugin using Plone's Add-on Control Panel, this id will be `oidc`. +- `oidc pas plugin id`: is the id you gave to the OIDC plugin when you created it inside the Plone PAS administration panel. If you just used the default configuration and installed this plugin using Plone's Add-on Control Panel, this id will be `oidc`. ### Example setup with Keycloak @@ -121,8 +119,8 @@ This command will use the [`docker-compose.yml`](./tests/docker-compose.yml) fil After start up, Keycloak will be accessible on [http://127.0.0.1:8180](http://127.0.0.1:8180), and you can manage it with the following credentials: -* **username**: admin -* **password**: admin +- **username**: admin +- **password**: admin #### Realms @@ -130,13 +128,13 @@ There are two realms configured `plone` and `plone-test`. The later is used in a The `plone` realm ships with an user that has the following credentials: -* username: **user** -* password: **12345678** +- username: **user** +- password: **12345678** And, to configure the oidc plugins, please use: -* client id: **plone** -* client secret: **12345678** +- client id: **plone** +- client secret: **12345678** #### Stop Keycloak @@ -154,18 +152,19 @@ parameter enabled also. The problem is that if the deprecated parameter is enabl will not work. So, this is the way it works: -* With legacy `redirect_uri` parameter enabled in Keycloak, the plugin works in default mode. -* With legacy `redirect_uri` parameter enabled in Keycloak, the plugin also works with legacy mode. -* With legacy `redirect_uri` parameter disabled in Keycloak (default after version 18), the plugin works in default mode. -* With legacy `redirect_uri` parameter disabled in Keycloak (default after version 18), the plugin does NOT work with legacy mode. + +- With legacy `redirect_uri` parameter enabled in Keycloak, the plugin works in default mode. +- With legacy `redirect_uri` parameter enabled in Keycloak, the plugin also works with legacy mode. +- With legacy `redirect_uri` parameter disabled in Keycloak (default after version 18), the plugin works in default mode. +- With legacy `redirect_uri` parameter disabled in Keycloak (default after version 18), the plugin does NOT work with legacy mode. So, for Keycloak, it does not matter if we use the default or legacy mode if the Keycloak runs in legacy mode. -*Notes:* +_Notes:_ -* If legacy `redirect_uri` parameter is disabled in Keycloak, this is the default since version 18 of Keycloak according - to this comment in *Starck Overflow*: https://stackoverflow.com/a/72142887. -* The plugin will work only if the `Use deprecated redirect_uri for logout url(/Plone/acl_users/oidc/logout)` +- If legacy `redirect_uri` parameter is disabled in Keycloak, this is the default since version 18 of Keycloak according + to this comment in _Starck Overflow_: https://stackoverflow.com/a/72142887. +- The plugin will work only if the `Use deprecated redirect_uri for logout url(/Plone/acl_users/oidc/logout)` option is un-checked at the plugin properties at http://localhost:8081/Plone/acl_users/oidc/manage_propertiesForm. #### Additional Documentation @@ -175,20 +174,20 @@ Specifically, here we will use a Docker image, so follow the instructions on how #### Setup Plone as a client -* Make sure **pas.plugins.oidc** is installed. -* Start Plone and create a Plone site with id Plone. -* In the Add-ons control panel, install `pas.plugins.oidc`. -* In the ZMI go to the plugin properties at http://localhost:8081/Plone/acl_users/oidc/manage_propertiesForm -* Set these properties: - * `OIDC/Oauth2 Issuer`: http://127.0.0.1:8081/realms/plone/ - * `Client ID`: *plone* (**Warning:** This property must match the `Client ID` you have set in Keycloak.) - * `Client secret`: *12345678* (**Warning:** This property must match the `Client secret` you have get in Keycloak.) - * `Use deprecated redirect_uri for logout url` checked. Use this if you need to run old versions of Keycloak. - * `Open ID scopes to request to the server`: this depends on which version of Keycloak you are using, and which scopes are available there. - In recent Keycloak versions, you *must* include `openid` as scope. +- Make sure **pas.plugins.oidc** is installed. +- Start Plone and create a Plone site with id Plone. +- In the Add-ons control panel, install `pas.plugins.oidc`. +- In the ZMI go to the plugin properties at http://localhost:8081/Plone/acl_users/oidc/manage_propertiesForm +- Set these properties: + - `OIDC/Oauth2 Issuer`: http://127.0.0.1:8081/realms/plone/ + - `Client ID`: _plone_ (**Warning:** This property must match the `Client ID` you have set in Keycloak.) + - `Client secret`: _12345678_ (**Warning:** This property must match the `Client secret` you have get in Keycloak.) + - `Use deprecated redirect_uri for logout url` checked. Use this if you need to run old versions of Keycloak. + - `Open ID scopes to request to the server`: this depends on which version of Keycloak you are using, and which scopes are available there. + In recent Keycloak versions, you _must_ include `openid` as scope. Suggestion is to use `openid` and `profile`. - * **Tip:** Leave the rest at the defaults, unless you know what you are doing. - * Click `Save`. + - **Tip:** Leave the rest at the defaults, unless you know what you are doing. + - Click `Save`. **Plone is ready done configured!** @@ -201,9 +200,9 @@ See this screenshot: Go to the other browser, or logout as admin from [Keycloak Admin Console](http://localhost:8080/admin). Currently, the Plone login form is unchanged. -Instead, for testing go to the login page of the plugin: http://localhost:8081/Plone/acl_users/oidc/login, +Instead, for testing go to the login page of the plugin: http://localhost:8081/Plone/acl*users/oidc/login, this will take you to Keycloak to login, and then return. You should now be logged in to Plone, and see the -*full name* and *email*, if you have set this in Keycloak. +\_full name* and _email_, if you have set this in Keycloak. #### Logout @@ -213,6 +212,31 @@ Currently, the Plone logout form is unchanged. Instead, for testing go to the logout page of the plugin: http://localhost:8081/Plone/acl_users/oidc/logout, this will take you to Keycloak to logout, and then return to the post-logout redirect URL. +### Configuration example for Sign in with Apple + +[Sign in with Apple](https://developer.apple.com/sign-in-with-apple/) is a way to delegate user authentication on Apple, so all Apple users can sign in in your site seamesly. + +But this means that you will need to do some extra steps on the Apple side in order to get your site correctly configured. + +1. Register an application + +Go to the Apple Developer Portal and create a new App ID in the [Certificates, Identifiers and Profiles](https://developer.apple.com/account/resources/identifiers/list/bundleId) section. + +2. Create a Service ID + +In the same page, create a new Service ID. The identifier you will add here will be the client*id to be used when configuring the login process. Use the reverse-domain-name style notation to create this id, something like: com.yourcompany.yoursite. Tick the \_Sign in with Apple* option. Click on _Configure_ next to the option and set your application domain and the callback url. You must enter an _https_ URL. + +3. Create a Private key + +Choose Keys on the side tab, select "Sign in with Apple" and click _Configure_. You will need to select the App Id created on the first step and download your private key, that will be shown just once. + +Now you have all the required items to configure this plugin: + +- client_id: the service id you created +- client_secret: the value of the private key downloaded in the last step. Open the file with a text editor of your choice, remove the ----PRIVATE KEY BEGIN---- and ----PRIVATE KEY END---- markers and any new line. +- Apple consumer team: this is your id in Apple. You can find in in the top right part of the site when logged in in the Apple developer portal +- Apple consumer id key: it is shown in the private you created in the last step. + ## Technical Decisions ### Usage of sessions in the login process @@ -246,7 +270,6 @@ removes most cookies to improve anonymous caching. The solution is to make sure the `__ac_session` cookie is added to the `cookie-pass` option. Check what the current default is in the buildout recipe, and update it: - ## Contribute - Issue Tracker: https://github.com/collective/pas.plugins.oidc/issues @@ -282,8 +305,8 @@ There are two realms configured `plone` and `plone-test`. The later is used in a The `plone` realm ships with an user that has the following credentials: -* username: **user** -* password: **12345678** +- username: **user** +- password: **12345678** To stop a running `Keycloak` (needed when running tests), use: @@ -327,7 +350,7 @@ Run tests named `TestServiceOIDCPost`: ## References -* Blog post: https://www.codesyntax.com/en/blog/log-in-in-plone-using-your-google-workspace-account +- Blog post: https://www.codesyntax.com/en/blog/log-in-in-plone-using-your-google-workspace-account ## License From 0cd09c47a7ce57cd5e541a96f3fc6f8c59ef7121 Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Sun, 3 Mar 2024 19:01:55 +0100 Subject: [PATCH 03/18] fix readme --- README.md | 51 ++++++++++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index d8bd534..6694722 100644 --- a/README.md +++ b/README.md @@ -85,9 +85,10 @@ When using this plugin with a [Volto frontend](https://6.docs.plone.org/volto/in Also, on the OpenID provider, configure the Redirect URL as **``/login_oidc/oidc**. + #### Classic UI -When using this plugin with _Plone 6 Classic UI_ the standard URLs used for login (`http://localhost:8080/Plone/login`) and logout (`http://localhost:8080/Plone/logout`) +When using this plugin with *Plone 6 Classic UI* the standard URLs used for login (`http://localhost:8080/Plone/login`) and logout (`http://localhost:8080/Plone/logout`) will not trigger the usage of the plugin. To login into a site using the OIDC provider, you will need to change those login URLs to the following: @@ -119,8 +120,8 @@ This command will use the [`docker-compose.yml`](./tests/docker-compose.yml) fil After start up, Keycloak will be accessible on [http://127.0.0.1:8180](http://127.0.0.1:8180), and you can manage it with the following credentials: -- **username**: admin -- **password**: admin +* **username**: admin +* **password**: admin #### Realms @@ -128,13 +129,13 @@ There are two realms configured `plone` and `plone-test`. The later is used in a The `plone` realm ships with an user that has the following credentials: -- username: **user** -- password: **12345678** +* username: **user** +* password: **12345678** And, to configure the oidc plugins, please use: -- client id: **plone** -- client secret: **12345678** +* client id: **plone** +* client secret: **12345678** #### Stop Keycloak @@ -153,18 +154,18 @@ will not work. So, this is the way it works: -- With legacy `redirect_uri` parameter enabled in Keycloak, the plugin works in default mode. -- With legacy `redirect_uri` parameter enabled in Keycloak, the plugin also works with legacy mode. -- With legacy `redirect_uri` parameter disabled in Keycloak (default after version 18), the plugin works in default mode. -- With legacy `redirect_uri` parameter disabled in Keycloak (default after version 18), the plugin does NOT work with legacy mode. +* With legacy `redirect_uri` parameter enabled in Keycloak, the plugin works in default mode. +* With legacy `redirect_uri` parameter enabled in Keycloak, the plugin also works with legacy mode. +* With legacy `redirect_uri` parameter disabled in Keycloak (default after version 18), the plugin works in default mode. +* With legacy `redirect_uri` parameter disabled in Keycloak (default after version 18), the plugin does NOT work with legacy mode. So, for Keycloak, it does not matter if we use the default or legacy mode if the Keycloak runs in legacy mode. _Notes:_ -- If legacy `redirect_uri` parameter is disabled in Keycloak, this is the default since version 18 of Keycloak according +* If legacy `redirect_uri` parameter is disabled in Keycloak, this is the default since version 18 of Keycloak according to this comment in _Starck Overflow_: https://stackoverflow.com/a/72142887. -- The plugin will work only if the `Use deprecated redirect_uri for logout url(/Plone/acl_users/oidc/logout)` +* The plugin will work only if the `Use deprecated redirect_uri for logout url(/Plone/acl_users/oidc/logout)` option is un-checked at the plugin properties at http://localhost:8081/Plone/acl_users/oidc/manage_propertiesForm. #### Additional Documentation @@ -174,20 +175,20 @@ Specifically, here we will use a Docker image, so follow the instructions on how #### Setup Plone as a client -- Make sure **pas.plugins.oidc** is installed. -- Start Plone and create a Plone site with id Plone. -- In the Add-ons control panel, install `pas.plugins.oidc`. -- In the ZMI go to the plugin properties at http://localhost:8081/Plone/acl_users/oidc/manage_propertiesForm -- Set these properties: - - `OIDC/Oauth2 Issuer`: http://127.0.0.1:8081/realms/plone/ - - `Client ID`: _plone_ (**Warning:** This property must match the `Client ID` you have set in Keycloak.) - - `Client secret`: _12345678_ (**Warning:** This property must match the `Client secret` you have get in Keycloak.) - - `Use deprecated redirect_uri for logout url` checked. Use this if you need to run old versions of Keycloak. - - `Open ID scopes to request to the server`: this depends on which version of Keycloak you are using, and which scopes are available there. +* Make sure **pas.plugins.oidc** is installed. +* Start Plone and create a Plone site with id Plone. +* In the Add-ons control panel, install `pas.plugins.oidc`. +* In the ZMI go to the plugin properties at http://localhost:8081/Plone/acl_users/oidc/manage_propertiesForm +* Set these properties: + * `OIDC/Oauth2 Issuer`: http://127.0.0.1:8081/realms/plone/ + * `Client ID`: _plone_ (**Warning:** This property must match the `Client ID` you have set in Keycloak.) + * `Client secret`: _12345678_ (**Warning:** This property must match the `Client secret` you have get in Keycloak.) + * `Use deprecated redirect_uri for logout url` checked. Use this if you need to run old versions of Keycloak. + * `Open ID scopes to request to the server`: this depends on which version of Keycloak you are using, and which scopes are available there. In recent Keycloak versions, you _must_ include `openid` as scope. Suggestion is to use `openid` and `profile`. - - **Tip:** Leave the rest at the defaults, unless you know what you are doing. - - Click `Save`. + * **Tip:** Leave the rest at the defaults, unless you know what you are doing. + * Click `Save`. **Plone is ready done configured!** From 534f0b95e0e1244b65e59b6abb906f49aa904628 Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Sun, 3 Mar 2024 19:03:04 +0100 Subject: [PATCH 04/18] fix readme --- README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 6694722..0c8e761 100644 --- a/README.md +++ b/README.md @@ -161,10 +161,10 @@ So, this is the way it works: So, for Keycloak, it does not matter if we use the default or legacy mode if the Keycloak runs in legacy mode. -_Notes:_ +*Notes:* * If legacy `redirect_uri` parameter is disabled in Keycloak, this is the default since version 18 of Keycloak according - to this comment in _Starck Overflow_: https://stackoverflow.com/a/72142887. + to this comment in *Stack Overflow*: https://stackoverflow.com/a/72142887. * The plugin will work only if the `Use deprecated redirect_uri for logout url(/Plone/acl_users/oidc/logout)` option is un-checked at the plugin properties at http://localhost:8081/Plone/acl_users/oidc/manage_propertiesForm. @@ -181,11 +181,11 @@ Specifically, here we will use a Docker image, so follow the instructions on how * In the ZMI go to the plugin properties at http://localhost:8081/Plone/acl_users/oidc/manage_propertiesForm * Set these properties: * `OIDC/Oauth2 Issuer`: http://127.0.0.1:8081/realms/plone/ - * `Client ID`: _plone_ (**Warning:** This property must match the `Client ID` you have set in Keycloak.) - * `Client secret`: _12345678_ (**Warning:** This property must match the `Client secret` you have get in Keycloak.) + * `Client ID`: *plone* (**Warning:** This property must match the `Client ID` you have set in Keycloak.) + * `Client secret`: *12345678* (**Warning:** This property must match the `Client secret` you have get in Keycloak.) * `Use deprecated redirect_uri for logout url` checked. Use this if you need to run old versions of Keycloak. * `Open ID scopes to request to the server`: this depends on which version of Keycloak you are using, and which scopes are available there. - In recent Keycloak versions, you _must_ include `openid` as scope. + In recent Keycloak versions, you *must* include `openid` as scope. Suggestion is to use `openid` and `profile`. * **Tip:** Leave the rest at the defaults, unless you know what you are doing. * Click `Save`. @@ -201,9 +201,9 @@ See this screenshot: Go to the other browser, or logout as admin from [Keycloak Admin Console](http://localhost:8080/admin). Currently, the Plone login form is unchanged. -Instead, for testing go to the login page of the plugin: http://localhost:8081/Plone/acl*users/oidc/login, +Instead, for testing go to the login page of the plugin: http://localhost:8081/Plone/acl_users/oidc/login, this will take you to Keycloak to login, and then return. You should now be logged in to Plone, and see the -\_full name* and _email_, if you have set this in Keycloak. +*full name* and *email*, if you have set this in Keycloak. #### Logout From ff10e92c34aeca848d8040e8be4ab9a90577d0fe Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Sun, 3 Mar 2024 19:03:47 +0100 Subject: [PATCH 05/18] fix readme --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0c8e761..180307c 100644 --- a/README.md +++ b/README.md @@ -271,6 +271,7 @@ removes most cookies to improve anonymous caching. The solution is to make sure the `__ac_session` cookie is added to the `cookie-pass` option. Check what the current default is in the buildout recipe, and update it: + ## Contribute - Issue Tracker: https://github.com/collective/pas.plugins.oidc/issues @@ -306,8 +307,8 @@ There are two realms configured `plone` and `plone-test`. The later is used in a The `plone` realm ships with an user that has the following credentials: -- username: **user** -- password: **12345678** +* username: **user** +* password: **12345678** To stop a running `Keycloak` (needed when running tests), use: @@ -351,7 +352,7 @@ Run tests named `TestServiceOIDCPost`: ## References -- Blog post: https://www.codesyntax.com/en/blog/log-in-in-plone-using-your-google-workspace-account +* Blog post: https://www.codesyntax.com/en/blog/log-in-in-plone-using-your-google-workspace-account ## License From d682a66920db96c318ff70290e3427955ee7e4ff Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Mon, 4 Mar 2024 11:26:23 +0100 Subject: [PATCH 06/18] explicitely disable bandit --- src/pas/plugins/oidc/plugins.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pas/plugins/oidc/plugins.py b/src/pas/plugins/oidc/plugins.py index e100867..0bcc14b 100644 --- a/src/pas/plugins/oidc/plugins.py +++ b/src/pas/plugins/oidc/plugins.py @@ -163,6 +163,7 @@ class OIDCPlugin(BasePlugin): <<<<<<< HEAD ======= APPLE_TOKEN_TTL_SEC = 6 * 30 * 24 * 60 * 60 + # nosec bandit: disable hardcoded_password_string APPLE_TOKEN_AUDIENCE = "https://appleid.apple.com" def __init__(self, id, title=None): From 5100692aa34fbd3cf611bbec8bdf66a346499ed1 Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Mon, 4 Mar 2024 11:50:25 +0100 Subject: [PATCH 07/18] disable bandit in a given line --- src/pas/plugins/oidc/plugins.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/pas/plugins/oidc/plugins.py b/src/pas/plugins/oidc/plugins.py index 0bcc14b..09b5542 100644 --- a/src/pas/plugins/oidc/plugins.py +++ b/src/pas/plugins/oidc/plugins.py @@ -163,8 +163,9 @@ class OIDCPlugin(BasePlugin): <<<<<<< HEAD ======= APPLE_TOKEN_TTL_SEC = 6 * 30 * 24 * 60 * 60 - # nosec bandit: disable hardcoded_password_string - APPLE_TOKEN_AUDIENCE = "https://appleid.apple.com" + APPLE_TOKEN_AUDIENCE = ( + "https://appleid.apple.com" # nosec bandit: disable hardcoded_password_string + ) def __init__(self, id, title=None): self._setId(id) From 69ccdcc19a167a5fcef27e2a8ce0ad44af1d2722 Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Wed, 6 Mar 2024 08:04:46 +0100 Subject: [PATCH 08/18] rework authentication method selection --- src/pas/plugins/oidc/utils.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/pas/plugins/oidc/utils.py b/src/pas/plugins/oidc/utils.py index e04fb92..e9ec587 100644 --- a/src/pas/plugins/oidc/utils.py +++ b/src/pas/plugins/oidc/utils.py @@ -169,10 +169,21 @@ def parse_authorization_response( def get_user_info(client, state, args) -> Union[message.OpenIDSchema, dict]: + # Decide which authentication method to use + allowed_authn_methods = client.registration_response.get("token_endpoint_auth_method") + if allowed_authn_methods and isinstance(allowed_authn_methods, list): + # arbitrary decision: take the first one + allowed_authn_method = allowed_authn_methods[0] + elif allowed_authn_methods and isinstance(allowed_authn_methods, str): + allowed_authn_method = allowed_authn_methods + else: + # If nothing is returned, try with the most basic one + allowed_authn_method = "client_secret_basic" + resp = client.do_access_token_request( state=state, request_args=args, - authn_method=client.registration_response.get('token_endpoint_auth_method', 'client_secret_basic'), + authn_method=allowed_authn_method ) user_info = {} if isinstance(resp, message.AccessTokenResponse): From e4334385a935c9160fa57e7ab0ff861aff83d0f0 Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Tue, 12 Mar 2024 21:59:34 +0100 Subject: [PATCH 09/18] on registration, we should take just one authn method --- src/pas/plugins/oidc/plugins.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pas/plugins/oidc/plugins.py b/src/pas/plugins/oidc/plugins.py index 09b5542..bed9dfa 100644 --- a/src/pas/plugins/oidc/plugins.py +++ b/src/pas/plugins/oidc/plugins.py @@ -364,7 +364,7 @@ def get_oauth2_client(self): "client_id": self.getProperty("client_id"), "token_endpoint_auth_method": provider_info.get( "token_endpoint_auth_methods_supported" - ), + )[0], } if self.getProperty('apple_login_enabled'): From 23d6aaf6376deca0b2fc6cc045b2cf2173b67878 Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Fri, 15 Mar 2024 18:17:55 +0100 Subject: [PATCH 10/18] lint --- src/pas/plugins/oidc/__init__.py | 1 + src/pas/plugins/oidc/browser/view.py | 14 +++++++------- src/pas/plugins/oidc/interfaces.py | 1 + src/pas/plugins/oidc/locales/update.py | 1 + src/pas/plugins/oidc/plugins.py | 23 ++++++++++++----------- src/pas/plugins/oidc/utils.py | 12 ++++++------ 6 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/pas/plugins/oidc/__init__.py b/src/pas/plugins/oidc/__init__.py index 8a56a07..9acb769 100644 --- a/src/pas/plugins/oidc/__init__.py +++ b/src/pas/plugins/oidc/__init__.py @@ -1,4 +1,5 @@ """Init and utils.""" + from AccessControl.Permissions import manage_users as ManageUsers from Products.PluggableAuthService import PluggableAuthService as PAS from zope.i18nmessageid import MessageFactory diff --git a/src/pas/plugins/oidc/browser/view.py b/src/pas/plugins/oidc/browser/view.py index bd21154..d1c170e 100644 --- a/src/pas/plugins/oidc/browser/view.py +++ b/src/pas/plugins/oidc/browser/view.py @@ -140,29 +140,29 @@ def __call__(self): # The response you get back is an instance of an AccessTokenResponse # or again possibly an ErrorResponse instance. - if self.context.getProperty('apple_login_enabled'): + if self.context.getProperty("apple_login_enabled"): args.update( { "client_id": self.context.getProperty("client_id"), - "client_secret": self.context._build_apple_secret() + "client_secret": self.context._build_apple_secret(), } ) initial_user_info = {} - if self.context.getProperty('apple_login_enabled'): + if self.context.getProperty("apple_login_enabled"): # Let's check if this is this user's first login # if so, their name and email could come in the first # response from authorization response # Weird Apple issues... - user = self.request.form.get('user', "") + user = self.request.form.get("user", "") if user: user_decoded = json.loads(user) first_name = user_decoded.get("name", {}).get("firstName", "") last_name = user_decoded.get("name", {}).get("lastName", "") email = user_decoded.get("email", "") - initial_user_info['given_name'] = first_name - initial_user_info['family_name'] = last_name - initial_user_info['email'] = email + initial_user_info["given_name"] = first_name + initial_user_info["family_name"] = last_name + initial_user_info["email"] = email user_info = utils.get_user_info(client, state, args) user_info.update(initial_user_info) diff --git a/src/pas/plugins/oidc/interfaces.py b/src/pas/plugins/oidc/interfaces.py index eb28036..07657d6 100644 --- a/src/pas/plugins/oidc/interfaces.py +++ b/src/pas/plugins/oidc/interfaces.py @@ -1,4 +1,5 @@ """Module where all interfaces, events and exceptions live.""" + from zope.publisher.interfaces.browser import IDefaultBrowserLayer diff --git a/src/pas/plugins/oidc/locales/update.py b/src/pas/plugins/oidc/locales/update.py index 2225b49..d1ebd70 100644 --- a/src/pas/plugins/oidc/locales/update.py +++ b/src/pas/plugins/oidc/locales/update.py @@ -1,4 +1,5 @@ """Update locales.""" + from pathlib import Path import logging diff --git a/src/pas/plugins/oidc/plugins.py b/src/pas/plugins/oidc/plugins.py index bed9dfa..42da117 100644 --- a/src/pas/plugins/oidc/plugins.py +++ b/src/pas/plugins/oidc/plugins.py @@ -6,7 +6,6 @@ from oic.oic.message import RegistrationResponse from oic.utils.authn.client import CLIENT_AUTHN_METHOD from pas.plugins.oidc import logger -from pas.plugins.oidc import PLUGIN_ID from plone.base.utils import safe_text from plone.protect.utils import safeWrite from Products.CMFCore.utils import getToolByName @@ -22,8 +21,10 @@ from zope.interface import Interface import itertools +import jwt import plone.api as api import string +import time PWCHARS = string.ascii_letters + string.digits + string.punctuation @@ -157,11 +158,8 @@ class OIDCPlugin(BasePlugin): mode="w", label="Apple consumer id key as defined by Apple", ), - ) -<<<<<<< HEAD -======= APPLE_TOKEN_TTL_SEC = 6 * 30 * 24 * 60 * 60 APPLE_TOKEN_AUDIENCE = ( "https://appleid.apple.com" # nosec bandit: disable hardcoded_password_string @@ -171,7 +169,6 @@ def __init__(self, id, title=None): self._setId(id) self.title = title ->>>>>>> 5c68af3 (add login with apple support) def rememberIdentity(self, userinfo): if not isinstance(userinfo, (OpenIDSchema, dict)): raise AssertionError( @@ -334,9 +331,9 @@ def _build_apple_secret(self): now = int(time.time()) client_id = self.getProperty("client_id") - team_id = self.getProperty('apple_consumer_team') - key_id = self.getProperty('apple_consumer_id_key') - private_key = self.getProperty('client_secret') + team_id = self.getProperty("apple_consumer_team") + key_id = self.getProperty("apple_consumer_id_key") + private_key = self.getProperty("client_secret") headers = {"kid": key_id} payload = { @@ -347,8 +344,12 @@ def _build_apple_secret(self): "sub": client_id, } - private_key = f'-----BEGIN PRIVATE KEY-----\n{private_key}\n-----END PRIVATE KEY-----' - return jwt.encode(payload, key=private_key.encode(), algorithm="ES256", headers=headers) + private_key = ( + f"-----BEGIN PRIVATE KEY-----\n{private_key}\n-----END PRIVATE KEY-----" + ) + return jwt.encode( + payload, key=private_key.encode(), algorithm="ES256", headers=headers + ) # TODO: memoize (?) def get_oauth2_client(self): @@ -367,7 +368,7 @@ def get_oauth2_client(self): )[0], } - if self.getProperty('apple_login_enabled'): + if self.getProperty("apple_login_enabled"): info.update({"client_secret": self._build_apple_secret()}) else: info.update({"client_secret": self.getProperty("client_secret")}) diff --git a/src/pas/plugins/oidc/utils.py b/src/pas/plugins/oidc/utils.py index e9ec587..2a43c70 100644 --- a/src/pas/plugins/oidc/utils.py +++ b/src/pas/plugins/oidc/utils.py @@ -128,8 +128,8 @@ def authorization_flow_args(plugin: plugins.OIDCPlugin, session: Session) -> dic args["code_challenge"] = pkce_code_verifier_challenge(session.get("verifier")) args["code_challenge_method"] = "S256" - if plugin.getProperty('apple_login_enabled'): - args['response_mode'] = 'form_post' + if plugin.getProperty("apple_login_enabled"): + args["response_mode"] = "form_post" return args @@ -170,7 +170,9 @@ def parse_authorization_response( def get_user_info(client, state, args) -> Union[message.OpenIDSchema, dict]: # Decide which authentication method to use - allowed_authn_methods = client.registration_response.get("token_endpoint_auth_method") + allowed_authn_methods = client.registration_response.get( + "token_endpoint_auth_method" + ) if allowed_authn_methods and isinstance(allowed_authn_methods, list): # arbitrary decision: take the first one allowed_authn_method = allowed_authn_methods[0] @@ -181,9 +183,7 @@ def get_user_info(client, state, args) -> Union[message.OpenIDSchema, dict]: allowed_authn_method = "client_secret_basic" resp = client.do_access_token_request( - state=state, - request_args=args, - authn_method=allowed_authn_method + state=state, request_args=args, authn_method=allowed_authn_method ) user_info = {} if isinstance(resp, message.AccessTokenResponse): From 6d66fd91647ec046d238a6f99dfbbc37b0dc16cd Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Fri, 15 Mar 2024 18:18:17 +0100 Subject: [PATCH 11/18] changelog --- news/49.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/49.feature diff --git a/news/49.feature b/news/49.feature new file mode 100644 index 0000000..a5b582c --- /dev/null +++ b/news/49.feature @@ -0,0 +1 @@ +Add Sign in with Apple support @erral From 627fd7ab9ac049320d2f05560f72c6f5c308ee1d Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Fri, 15 Mar 2024 18:23:25 +0100 Subject: [PATCH 12/18] use pluginid --- src/pas/plugins/oidc/setuphandlers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pas/plugins/oidc/setuphandlers.py b/src/pas/plugins/oidc/setuphandlers.py index a584dd3..aa96182 100644 --- a/src/pas/plugins/oidc/setuphandlers.py +++ b/src/pas/plugins/oidc/setuphandlers.py @@ -23,6 +23,7 @@ def post_install(context): # Create plugin if it does not exist. if PLUGIN_ID not in pas.objectIds(): plugin = OIDCPlugin( + id=PLUGIN_ID, title="OpenID Connect", ) plugin.id = PLUGIN_ID From fdf59537143d66159bb015c607614146a4ee8ed5 Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Fri, 15 Mar 2024 22:08:42 +0100 Subject: [PATCH 13/18] change authn method negotiation --- src/pas/plugins/oidc/utils.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/pas/plugins/oidc/utils.py b/src/pas/plugins/oidc/utils.py index 2a43c70..8196318 100644 --- a/src/pas/plugins/oidc/utils.py +++ b/src/pas/plugins/oidc/utils.py @@ -173,14 +173,19 @@ def get_user_info(client, state, args) -> Union[message.OpenIDSchema, dict]: allowed_authn_methods = client.registration_response.get( "token_endpoint_auth_method" ) + allowed_authn_method = "client_secret_basic" if allowed_authn_methods and isinstance(allowed_authn_methods, list): - # arbitrary decision: take the first one - allowed_authn_method = allowed_authn_methods[0] - elif allowed_authn_methods and isinstance(allowed_authn_methods, str): - allowed_authn_method = allowed_authn_methods - else: - # If nothing is returned, try with the most basic one - allowed_authn_method = "client_secret_basic" + # Here we should decide which method we will use among the ones + # offered by the provider. + # But we would need extra information to implement some of those + # methods such as private_key_jwt or client_secret_jwt. + # So that's the reason why we only allow `client_secret_post` + # or `client_secret_basic`, which are the most basic ones. + # + if 'client_secret_post' in allowed_authn_methods: + allowed_authn_method = 'client_secret_post' + elif 'client_secret_basic' in allowed_authn_methods: + allowed_authn_method = "client_secret_basic" resp = client.do_access_token_request( state=state, request_args=args, authn_method=allowed_authn_method From 3a922bdbd80ab442d599697ff504ee8b2a9d43b1 Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Fri, 15 Mar 2024 22:09:40 +0100 Subject: [PATCH 14/18] docs --- src/pas/plugins/oidc/utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pas/plugins/oidc/utils.py b/src/pas/plugins/oidc/utils.py index 8196318..de29aba 100644 --- a/src/pas/plugins/oidc/utils.py +++ b/src/pas/plugins/oidc/utils.py @@ -179,9 +179,9 @@ def get_user_info(client, state, args) -> Union[message.OpenIDSchema, dict]: # offered by the provider. # But we would need extra information to implement some of those # methods such as private_key_jwt or client_secret_jwt. - # So that's the reason why we only allow `client_secret_post` - # or `client_secret_basic`, which are the most basic ones. - # + # So that's the reason why we only allow `client_secret_post` (the + # only one allowed by Apple) or `client_secret_basic` (the most + # basic one, allowed by most of the providers we have worked with) if 'client_secret_post' in allowed_authn_methods: allowed_authn_method = 'client_secret_post' elif 'client_secret_basic' in allowed_authn_methods: From 146e83e99afe0f40542c67ff60459d81cac571ef Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Fri, 15 Mar 2024 22:12:00 +0100 Subject: [PATCH 15/18] lint --- src/pas/plugins/oidc/utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pas/plugins/oidc/utils.py b/src/pas/plugins/oidc/utils.py index de29aba..8a9e20d 100644 --- a/src/pas/plugins/oidc/utils.py +++ b/src/pas/plugins/oidc/utils.py @@ -182,9 +182,9 @@ def get_user_info(client, state, args) -> Union[message.OpenIDSchema, dict]: # So that's the reason why we only allow `client_secret_post` (the # only one allowed by Apple) or `client_secret_basic` (the most # basic one, allowed by most of the providers we have worked with) - if 'client_secret_post' in allowed_authn_methods: - allowed_authn_method = 'client_secret_post' - elif 'client_secret_basic' in allowed_authn_methods: + if "client_secret_post" in allowed_authn_methods: + allowed_authn_method = "client_secret_post" + elif "client_secret_basic" in allowed_authn_methods: allowed_authn_method = "client_secret_basic" resp = client.do_access_token_request( From f352cbf66249cd109570ea7d72b9124e5c1dd3ae Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Fri, 15 Mar 2024 22:16:37 +0100 Subject: [PATCH 16/18] unneeded changes --- src/pas/plugins/oidc/plugins.py | 4 ---- src/pas/plugins/oidc/utils.py | 3 --- 2 files changed, 7 deletions(-) diff --git a/src/pas/plugins/oidc/plugins.py b/src/pas/plugins/oidc/plugins.py index 42da117..1767631 100644 --- a/src/pas/plugins/oidc/plugins.py +++ b/src/pas/plugins/oidc/plugins.py @@ -165,10 +165,6 @@ class OIDCPlugin(BasePlugin): "https://appleid.apple.com" # nosec bandit: disable hardcoded_password_string ) - def __init__(self, id, title=None): - self._setId(id) - self.title = title - def rememberIdentity(self, userinfo): if not isinstance(userinfo, (OpenIDSchema, dict)): raise AssertionError( diff --git a/src/pas/plugins/oidc/utils.py b/src/pas/plugins/oidc/utils.py index 8a9e20d..07d382a 100644 --- a/src/pas/plugins/oidc/utils.py +++ b/src/pas/plugins/oidc/utils.py @@ -195,7 +195,6 @@ def get_user_info(client, state, args) -> Union[message.OpenIDSchema, dict]: # If it's an AccessTokenResponse the information in the response will be stored in the # client instance with state as the key for future use. user_info = resp.to_dict().get("id_token", {}) - if client.userinfo_endpoint: # https://openid.net/specs/openid-connect-core-1_0.html#UserInfo @@ -214,8 +213,6 @@ def get_user_info(client, state, args) -> Union[message.OpenIDSchema, dict]: exc_info=exc, ) user_info = {} - else: - pass # userinfo in an instance of OpenIDSchema or ErrorResponse # It could also be dict, if there is no userinfo_endpoint if not (user_info and isinstance(user_info, (message.OpenIDSchema, dict))): From d4b8e8d8aeb8ff649f8b02ce163fe8bc0f6dd156 Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Fri, 15 Mar 2024 22:59:04 +0100 Subject: [PATCH 17/18] more Apple weirdness --- src/pas/plugins/oidc/utils.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/pas/plugins/oidc/utils.py b/src/pas/plugins/oidc/utils.py index 07d382a..e306d81 100644 --- a/src/pas/plugins/oidc/utils.py +++ b/src/pas/plugins/oidc/utils.py @@ -186,6 +186,11 @@ def get_user_info(client, state, args) -> Union[message.OpenIDSchema, dict]: allowed_authn_method = "client_secret_post" elif "client_secret_basic" in allowed_authn_methods: allowed_authn_method = "client_secret_basic" + elif allowed_authn_methods and isinstance(allowed_authn_methods, str): + # Yay, Apple returns a string in the allowed_authn_methods + # We may face the same in some other providers, so we keep it + # as we receive it + allowed_authn_method = allowed_authn_methods resp = client.do_access_token_request( state=state, request_args=args, authn_method=allowed_authn_method From 8bb4bd54d6644a4b7e4b9ff837b13ba53a253e09 Mon Sep 17 00:00:00 2001 From: Mauro Amico Date: Fri, 24 May 2024 01:40:15 +0200 Subject: [PATCH 18/18] fix dependence checker --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 2d8fd56..44371d3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -122,6 +122,7 @@ Zope = [ 'Products.CMFCore', 'Products.CMFDynamicViewFTI', ] python-dateutil = ['dateutil'] +PyJWT = ['jwt'] ignore-packages = ['plone.restapi', 'plone.volto', 'zestreleaser.towncrier', 'zest.releaser', 'pytest', 'pytest-cov', 'pytest-plone', 'pytest-docker', 'pytest-vcr', 'pytest-mock', 'zope.pytestlayer', 'requests-mock', 'vcrpy'] Plone = ['Products.CMFPlone', 'Products.CMFCore', 'Products.GenericSetup', 'Products.PluggableAuthService', 'Products.PlonePAS']