Merge pull request #11402 from camptocamp/GSBLGEOS-35-add-user-display

Add display name in the user for better OpenID Connect support

admin/tests/test_user.py

@@ -166,6 +166,7 @@ def test_submit_update(self, pw_gen_mock, smtp_mock, dbsession, test_app, users_
                 ("item_type", "user"),
                 ("id", user.id),
                 ("username", "new_name_withéàô"),
+                ("display_name", "New name withéàô"),
                 ("email", "[email protected]"),
                 ("settings_role_id", roles[2].id),
                 ("__start__", "roles:sequence"),
@@ -271,6 +272,7 @@ def test_submit_new(self, pw_gen_mock, smtp_mock, dbsession, test_app, users_tes
                 ("item_type", "user"),
                 ("id", ""),
                 ("username", "new_user"),
+                ("display_name", "New user"),
                 ("email", "[email protected]"),
                 ("settings_role_id", roles[2].id),
             ),
@@ -284,6 +286,7 @@ def test_submit_new(self, pw_gen_mock, smtp_mock, dbsession, test_app, users_tes
         ).group(1)
 
         assert user.username == "new_user"
+        assert user.display_name == "New user"
         assert user.email == "[email protected]"
         assert user.settings_role_id == roles[2].id
         assert user.password is not None and len(user.password)
@@ -315,6 +318,7 @@ def test_invalid_email(self, test_app):
                 "item_type": "user",
                 "id": "",
                 "username": "invalid_email",
+                "display_name": "Invalid email",
                 "email": "new_mail",
                 "role_name": "secretary_2",
                 "is_password_changed": "false",

commons/c2cgeoportal_commons/alembic/static/267b4c1bde2e_add_display_name_in_the_user_for_better_.py

@@ -0,0 +1,61 @@
+# Copyright (c) 2024, Camptocamp SA
+# All rights reserved.
+
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are met:
+
+# 1. Redistributions of source code must retain the above copyright notice, this
+#    list of conditions and the following disclaimer.
+# 2. Redistributions in binary form must reproduce the above copyright notice,
+#    this list of conditions and the following disclaimer in the documentation
+#    and/or other materials provided with the distribution.
+
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
+# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+# The views and conclusions contained in the software and documentation are those
+# of the authors and should not be interpreted as representing official policies,
+# either expressed or implied, of the FreeBSD Project.
+
+"""
+Add display name in the user for better OpenID Connect support.
+
+Revision ID: 267b4c1bde2e
+Revises: aa41e9613256
+Create Date: 2024-10-02 14:33:59.185133
+"""
+
+import sqlalchemy as sa
+from alembic import op
+from c2c.template.config import config
+from sqlalchemy.dialects import postgresql
+
+# revision identifiers, used by Alembic.
+revision = "267b4c1bde2e"
+down_revision = "aa41e9613256"
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    """Upgrade."""
+    staticschema = config["schema_static"]
+
+    op.add_column("user", sa.Column("display_name", sa.Unicode(), nullable=True), schema=staticschema)
+    op.execute(f'UPDATE {staticschema}."user" SET display_name = username')
+    op.alter_column("user", "display_name", nullable=False, schema=staticschema)
+
+
+def downgrade() -> None:
+    """Downgrade."""
+    staticschema = config["schema_static"]
+
+    op.drop_column("user", "display_name", schema=staticschema)

commons/c2cgeoportal_commons/models/static.py

@@ -142,6 +142,15 @@ class User(Base):  # type: ignore
             }
         },
     )
+    display_name: Mapped[str] = mapped_column(
+        Unicode,
+        info={
+            "colanderalchemy": {
+                "title": _("Display name"),
+                "description": _("Name displayed in the application."),
+            }
+        },
+    )
     _password: Mapped[str] = mapped_column(
         "password", Unicode, nullable=False, info={"colanderalchemy": {"exclude": True}}
     )
@@ -263,8 +272,10 @@ def __init__(  # nosec
         roles: list[Role] | None = None,
         expire_on: datetime | None = None,
         deactivated: bool = False,
+        display_name: str | None = None,
     ) -> None:
         self.username = username
+        self.display_name = display_name or username
         self.password = password
         self.tech_data = {}
         self.email = email

doc/integrator/authentication_oidc.rst

@@ -48,7 +48,8 @@ If we want to use OpenID Connect as an authentication provider, we need to set t
          url: <the service URL>
          client_id: <the client application ID>
          user_info_fields:
-           username: name  # Default value
+           username: sub  # Default value
+           display_name: name  # Default value
            email: email  # Default value
 
 With that the user will be create in the database at the first login, and the access right will be set in the GeoMapFish database.

geoportal/c2cgeoportal_geoportal/__init__.py

@@ -363,7 +363,8 @@ def get_user_from_request(
                         )
 
                         request.user_ = oidc.DynamicUser(
-                            username=user_info["username"],
+                            username=user_info["sub"],
+                            display_name=user_info["username"],
                             email=user_info["email"],
                             settings_role=(
                                 DBSession.query(Role).filter_by(name=user_info["settings_role"]).first()
@@ -376,7 +377,7 @@ def get_user_from_request(
                             ],
                         )
                     else:
-                        request.user_ = DBSession.query(User).filter_by(email=user_info["email"]).first()
+                        request.user_ = DBSession.query(User).filter_by(username=user_info["sub"]).first()
                         for user in DBSession.query(User).all():
                             _LOG.error(user.username)
                 else:

geoportal/c2cgeoportal_geoportal/lib/oidc.py

@@ -51,6 +51,7 @@ class DynamicUser(NamedTuple):
     """
 
     username: str
+    display_name: str
     email: str
     settings_role: main.Role | None
     roles: list[main.Role]
@@ -89,6 +90,7 @@ class OidcRememberObject(TypedDict):
     refresh_token: str | None
     refresh_token_expires: str | None
     username: str | None
+    display_name: str | None
     email: str | None
     settings_role: str | None
     roles: list[str]
@@ -144,6 +146,7 @@ def remember(
                 ).isoformat()
             ),
             "username": None,
+            "display_name": None,
             "email": None,
             "settings_role": None,
             "roles": [],
@@ -173,7 +176,8 @@ def remember(
             )
 
         for field_, default_field in (
-            ("username", "name"),
+            ("username", "sub"),
+            ("display_name", "name"),
             ("email", "email"),
             ("settings_role", None),
             ("roles", None),

geoportal/c2cgeoportal_geoportal/scaffolds/update/{{cookiecutter.project}}/geoportal/CONST_config-schema.yaml

@@ -243,6 +243,9 @@ mapping:
                 type: map
                 mapping:
                   username:
+                    type: str
+                    default: sub
+                  display_name:
                     type: str
                     default: name
                   email:

geoportal/c2cgeoportal_geoportal/views/login.py

@@ -329,7 +329,7 @@ def _user(self, user: static.User | None = None) -> dict[str, Any]:
         if user is not None:
             result.update(
                 {
-                    "username": user.username,
+                    "username": user.display_name,
                     "email": user.email,
                     "roles": [{"name": r.name, "id": r.id} for r in user.roles],
                 }
@@ -652,7 +652,8 @@ def oidc_callback(self) -> pyramid.response.Response:
                 models.DBSession.add(user)
         else:
             user = oidc.DynamicUser(
-                username=remember_object["username"],
+                username=remember_object["sub"],
+                display_name=remember_object["username"],
                 email=remember_object["email"],
                 settings_role=(
                     models.DBSession.query(main.Role).filter_by(name=remember_object["settings_role"]).first()
@@ -681,7 +682,7 @@ def oidc_callback(self) -> pyramid.response.Response:
                 # TODO respect the user interface...
                 json.dumps(
                     {
-                        "username": user.username,
+                        "username": user.display_name,
                         "email": user.email,
                         "is_intranet": is_intranet(self.request),
                         "functionalities": self._functionality(),

geoportal/tests/functional/test_login.py

@@ -320,6 +320,7 @@ def __init__(self, name, functionalities):
 
         class U:
             username = "__test_user"
+            display_name = "Test User"
             is_password_changed = True
             email = "[email protected]"
             settings_role = None
@@ -332,7 +333,7 @@ def __init__(self, role="__test_role", functionalities=None):
         request.user = U()
         login = Login(request)
         expected = {
-            "username": "__test_user",
+            "username": "Test User",
             "email": "[email protected]",
             "is_intranet": False,
             "login_type": "local",
@@ -350,7 +351,7 @@ class F:
         del request.response.headers["Vary"]
         login = Login(request)
         expected = {
-            "username": "__test_user",
+            "username": "Test User",
             "email": "[email protected]",
             "is_intranet": False,
             "login_type": "local",
@@ -392,6 +393,7 @@ def __init__(self, name, functionalities):
 
         class U:
             username = "__test_user"
+            display_name = "Test User"
             is_password_changed = True
             email = "[email protected]"
 
@@ -407,13 +409,13 @@ def __init__(self, role="__test_role", functionalities=None):
         self.assertEqual(
             login.loginuser(),
             {
+                "username": "Test User",
                 "email": "[email protected]",
                 "functionalities": {},
                 "is_intranet": False,
                 "login_type": "local",
                 "roles": [{"id": 123, "name": "__test_role"}],
                 "two_factor_enable": False,
-                "username": "__test_user",
             },
         )
 
@@ -422,13 +424,13 @@ def __init__(self, role="__test_role", functionalities=None):
         self.assertEqual(
             login.loginuser(),
             {
+                "username": "Test User",
                 "email": "[email protected]",
                 "functionalities": {},
                 "is_intranet": True,
                 "login_type": "local",
                 "roles": [{"id": 123, "name": "__test_role"}],
                 "two_factor_enable": False,
-                "username": "__test_user",
             },
         )