diff --git a/alembic/versions/20240222_9966f6f95674_create_admincredentials_table.py b/alembic/versions/20240222_9966f6f95674_create_admincredentials_table.py new file mode 100644 index 000000000..01f76428a --- /dev/null +++ b/alembic/versions/20240222_9966f6f95674_create_admincredentials_table.py @@ -0,0 +1,38 @@ +"""Create admincredentials table + +Revision ID: 9966f6f95674 +Revises: 993729d4bf97 +Create Date: 2024-02-22 02:36:07.130941+00:00 + +""" +import sqlalchemy as sa + +from alembic import op + +# revision identifiers, used by Alembic. +revision = "9966f6f95674" +down_revision = "993729d4bf97" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "admincredentials", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("external_id", sa.Unicode(), nullable=False), + sa.Column("admin_id", sa.Integer(), nullable=False), + sa.ForeignKeyConstraint(["admin_id"], ["admins.id"], ondelete="CASCADE"), + sa.PrimaryKeyConstraint("id"), + ) + op.create_index( + op.f("ix_admincredentials_admin_id"), + "admincredentials", + ["admin_id"], + unique=False, + ) + + +def downgrade() -> None: + op.drop_index(op.f("ix_admincredentials_admin_id"), table_name="admincredentials") + op.drop_table("admincredentials") diff --git a/api/admin/controller/base.py b/api/admin/controller/base.py index fe9f496bd..397454700 100644 --- a/api/admin/controller/base.py +++ b/api/admin/controller/base.py @@ -6,6 +6,9 @@ import flask +from api.admin.ekirjasto_admin_authentication_provider import ( + EkirjastoAdminAuthenticationProvider, +) from api.admin.exceptions import AdminNotAuthorized from api.admin.password_admin_authentication_provider import ( PasswordAdminAuthenticationProvider, @@ -35,8 +38,10 @@ def __init__(self, manager): @property def admin_auth_providers(self): if Admin.with_password(self._db).count() != 0: - return [PasswordAdminAuthenticationProvider()] - + return [ + EkirjastoAdminAuthenticationProvider(), + PasswordAdminAuthenticationProvider(), + ] return [] def admin_auth_provider(self, type): diff --git a/api/admin/controller/individual_admin_settings.py b/api/admin/controller/individual_admin_settings.py index 010046ffc..ca08f4784 100644 --- a/api/admin/controller/individual_admin_settings.py +++ b/api/admin/controller/individual_admin_settings.py @@ -1,4 +1,5 @@ import json +from typing import Literal import flask from flask import Response @@ -75,7 +76,11 @@ def append_role(roles, role): roles.append(role_dict) admins = [] - for admin in self._db.query(Admin).order_by(Admin.email): + for admin in ( + self._db.query(Admin) + .join(Admin.admin_credentials, isouter=True) + .order_by(Admin.email) + ): roles = [] show_admin = True for role in admin.roles: @@ -105,12 +110,20 @@ def append_role(roles, role): append_role(roles, role) if len(roles): - admins.append(dict(email=admin.email, roles=roles)) + admins.append( + dict(email=admin.email, roles=roles, authType=self.auth_type(admin)) + ) return dict( individualAdmins=admins, ) + @staticmethod + def auth_type(admin: Admin) -> Literal["EXTERNAL", "PASSWORD"]: + if admin.admin_credentials: + return "EXTERNAL" + return "PASSWORD" + def process_post_create_first_admin(self, email: str): """Create the first admin in the system.""" @@ -280,6 +293,14 @@ def check_permissions(self, admin, settingUp): if admin.is_system_admin(): raise AdminNotAuthorized() + # Finland + # Only password-authenticated users can create other password-based users. + if ( + user.is_sitewide_library_manager() + and self.auth_type(user) != "PASSWORD" + ): + raise AdminNotAuthorized + # By this point, we know no one is a system admin. if user.is_sitewide_library_manager(): return @@ -421,6 +442,17 @@ def handle_password(self, password, admin: Admin, is_new, settingUp): can_change_pw = True if not can_change_pw: raise AdminNotAuthorized(message) + + # Finland + if ( + not admin.password_hashed + and admin.admin_credentials + and not user.is_system_admin() + ): + raise AdminNotAuthorized( + "Not authorized to set password for externally authenticated user." + ) + admin.password = password try: self._db.flush() diff --git a/api/admin/controller/reset_password.py b/api/admin/controller/reset_password.py index effc57857..74990f2de 100644 --- a/api/admin/controller/reset_password.py +++ b/api/admin/controller/reset_password.py @@ -70,7 +70,8 @@ def forgot_password(self) -> ProblemDetail | WerkzeugResponse: admin = self._extract_admin_from_request(flask.request) - if not admin: + # Finland, extra check to disable password reset for ekirjasto authenticated users. + if not admin or admin.admin_credentials: return self._response_with_message_and_redirect_button( INVALID_ADMIN_CREDENTIALS.detail, url_for("admin_forgot_password"), diff --git a/api/admin/controller/sign_in.py b/api/admin/controller/sign_in.py index 57f2e71b4..16f9ae531 100644 --- a/api/admin/controller/sign_in.py +++ b/api/admin/controller/sign_in.py @@ -10,12 +10,16 @@ from api.admin.config import Configuration as AdminClientConfig from api.admin.controller.base import AdminController +from api.admin.ekirjasto_admin_authentication_provider import ( + EkirjastoAdminAuthenticationProvider, +) from api.admin.password_admin_authentication_provider import ( PasswordAdminAuthenticationProvider, ) from api.admin.problem_details import ( ADMIN_AUTH_MECHANISM_NOT_CONFIGURED, ADMIN_AUTH_NOT_CONFIGURED, + ADMIN_NOT_AUTHORIZED, INVALID_ADMIN_CREDENTIALS, ) from api.admin.template_styles import ( @@ -26,6 +30,8 @@ section_style, small_link_style, ) +from core.model import get_one, get_one_or_create +from core.model.admin import Admin, AdminCredential, AdminRole from core.util.problem_detail import ProblemDetail @@ -67,6 +73,87 @@ class SignInController(AdminController): logo=logo_style, ) + # Finland + def ekirjasto_auth_finish(self): + auth: EkirjastoAdminAuthenticationProvider = self.admin_auth_provider( + EkirjastoAdminAuthenticationProvider.NAME + ) + if not auth: + return ADMIN_AUTH_MECHANISM_NOT_CONFIGURED + + result = flask.request.form.get("result") + if result != "success": + logging.error("Ekirjasto authentication failed, result = %s", result) + + ekirjasto_token = flask.request.form.get("token") + + user_info = auth.ekirjasto_authenticate(ekirjasto_token) + if type(user_info) is ProblemDetail: + return user_info + + try: + credentials = get_one(self._db, AdminCredential, external_id=user_info.sub) + + circulation_role = self._to_circulation_role(user_info.role) + if not circulation_role: + return self.error_response(ADMIN_NOT_AUTHORIZED) + + if credentials: + admin = credentials.admin + else: + # No existing credentials found, create new admin & credentials + # TODO: handle e-mail field + admin, _ = get_one_or_create( + self._db, + Admin, + email=f"{user_info.given_name}-{user_info.sub}", + ) + _, _ = get_one_or_create( + self._db, + AdminCredential, + external_id=user_info.sub, + admin_id=admin.id, + ) + + # Update roles if changed + existing_roles = [(role.role, role.library) for role in admin.roles] + if [(circulation_role, None)] != existing_roles: + for role in admin.roles: + admin.remove_role(role.role, role.library) + admin.add_role(circulation_role) + + except Exception as e: + logging.error("Internal error during signup", exc_info=e) + self._db.rollback() + # TODO: more specific error response, code 500 + return ADMIN_AUTH_MECHANISM_NOT_CONFIGURED + + # Set up the admin's flask session. + flask.session["admin_email"] = admin.email + flask.session["auth_type"] = auth.NAME + + # This one is extra compared to password auth provider + flask.session["ekirjasto_token"] = ekirjasto_token + + # A permanent session expires after a fixed time, rather than + # when the user closes the browser. + flask.session.permanent = True + + redirect_uri = flask.request.args.get("redirect_uri", "/admin/web") + return SanitizedRedirections.redirect(redirect_uri) + + def _to_circulation_role(self, ekirjasto_role: str) -> str | None: + if ekirjasto_role == "orgadmin": + return AdminRole.SYSTEM_ADMIN + if ekirjasto_role == "admin": + return AdminRole.SITEWIDE_LIBRARY_MANAGER + elif ekirjasto_role == "registrant": + return AdminRole.SITEWIDE_LIBRARIAN + else: + # other possible values are "sysadmin" and "customer", + # these are not allowed as circulation admins + return None + def sign_in(self): """Redirects admin if they're signed in, or shows the sign in page.""" if not self.admin_auth_providers: @@ -127,6 +214,9 @@ def sign_out(self): flask.session.pop("admin_email", None) flask.session.pop("auth_type", None) + # Finland, revoke ekirjasto session + self._try_revoke_ekirjasto_session() + redirect_url = url_for( "admin_sign_in", redirect=url_for("admin_view", _external=True), @@ -134,6 +224,15 @@ def sign_out(self): ) return SanitizedRedirections.redirect(redirect_url) + # Finland + def _try_revoke_ekirjasto_session(self): + ekirjasto_token = flask.session.pop("ekirjasto_token", None) + auth: EkirjastoAdminAuthenticationProvider = self.admin_auth_provider( + EkirjastoAdminAuthenticationProvider.NAME + ) + if ekirjasto_token and auth: + auth.try_revoke_ekirjasto_session(ekirjasto_token) + def error_response(self, problem_detail): """Returns a problem detail as an HTML response""" html = self.ERROR_RESPONSE_TEMPLATE % dict( diff --git a/api/admin/ekirjasto_admin_authentication_provider.py b/api/admin/ekirjasto_admin_authentication_provider.py new file mode 100644 index 000000000..9ce90b538 --- /dev/null +++ b/api/admin/ekirjasto_admin_authentication_provider.py @@ -0,0 +1,111 @@ +import logging + +import requests +from flask import url_for +from pydantic import BaseModel + +from api.admin.admin_authentication_provider import AdminAuthenticationProvider +from api.admin.template_styles import button_style, input_style, label_style +from api.admin.templates import ekirjasto_sign_in_template +from api.circulation_exceptions import RemoteInitiatedServerError +from api.problem_details import ( + EKIRJASTO_REMOTE_AUTHENTICATION_FAILED, + INVALID_EKIRJASTO_TOKEN, +) +from core.util.problem_detail import ProblemDetail + + +class EkirjastoUserInfo(BaseModel): + exp: int + family_name: str = "" + given_name: str = "" + role: str + sub: str + sid: str + municipality: str + verified: bool = False + passkeys: list[dict] = [] + + +class EkirjastoAdminAuthenticationProvider(AdminAuthenticationProvider): + NAME = "Ekirjasto Auth" + + SIGN_IN_TEMPLATE = ekirjasto_sign_in_template.format( + label=label_style, input=input_style, button=button_style + ) + + # TODO: make configurable + _ekirjasto_api_url = "https://e-kirjasto.loikka.dev" + + def sign_in_template(self, redirect): + redirect_uri = url_for( + "ekirjasto_auth_finish", redirect_uri=redirect, _external=True + ) + # TODO: make configurable + state = ":T0008" # test state for "orgadmin" role + ekirjasto_auth_url = ( + f"{self._ekirjasto_api_url}/v1/auth/tunnistus/start" + f"?locale=fi" + f"&state={state}" + f"&redirect_uri={redirect_uri}" + ) + return self.SIGN_IN_TEMPLATE % dict( + ekirjasto_auth_url=ekirjasto_auth_url, + ) + + def active_credentials(self, admin): + # TODO: Not sure where this is used, need to figure it out. + return True + + def ekirjasto_authenticate( + self, ekirjasto_token + ) -> EkirjastoUserInfo | ProblemDetail: + return self._get_user_info(ekirjasto_token) + + def _get_user_info(self, ekirjasto_token: str) -> EkirjastoUserInfo | ProblemDetail: + userinfo_url = self._ekirjasto_api_url + "/v1/auth/userinfo" + try: + response = requests.get( + userinfo_url, headers={"Authorization": f"Bearer {ekirjasto_token}"} + ) + except requests.exceptions.ConnectionError as e: + raise RemoteInitiatedServerError(str(e), self.__class__.__name__) + + if response.status_code == 401: + return INVALID_EKIRJASTO_TOKEN + elif response.status_code != 200: + logging.error( + "Got unexpected response code %d, content=%s", + response.status_code, + (response.content or b"No content").decode("utf-8", errors="replace"), + ) + return EKIRJASTO_REMOTE_AUTHENTICATION_FAILED + else: + try: + return EkirjastoUserInfo(**response.json()) + except requests.exceptions.JSONDecodeError as e: + raise RemoteInitiatedServerError(str(e), self.__class__.__name__) + + def try_revoke_ekirjasto_session(self, ekirjasto_token: str) -> None: + revoke_url = self._ekirjasto_api_url + "/v1/auth/revoke" + + try: + response = requests.post( + revoke_url, headers={"Authorization": f"Bearer {ekirjasto_token}"} + ) + except requests.exceptions.ConnectionError as e: + logging.exception( + "Failed to revoke ekirjasto session due to connection error." + ) + # Ignore connection error, we tried our best + + # Response codes in 4xx range mean that session is already expired, thus ok. + # For 5xx range, we want to log the response + if 500 <= response.status_code < 600: + # Decode response content assuming UTF-8, replace with the correct encoding if necessary + logging.error( + "Failed to revoke ekirjasto session due to server error, status=%s, content=%s", + response.status_code, + (response.content or b"No content").decode("utf-8", errors="replace"), + ) + # Ignore the error response, we tried our best diff --git a/api/admin/routes.py b/api/admin/routes.py index e4ec8d4df..1b061484e 100644 --- a/api/admin/routes.py +++ b/api/admin/routes.py @@ -115,6 +115,14 @@ def admin_sign_in(): return app.manager.admin_sign_in_controller.sign_in() +# Finland +# Ekirjasto auth client calls this endpoint when it finishes authenticating the user +@app.route("/admin/ekirjasto_auth_finish", methods=["POST"]) +@returns_problem_detail +def ekirjasto_auth_finish(): + return app.manager.admin_sign_in_controller.ekirjasto_auth_finish() + + @app.route("/admin/sign_out") @returns_problem_detail @requires_admin diff --git a/api/admin/templates.py b/api/admin/templates.py index d2bfab477..fc2693892 100644 --- a/api/admin/templates.py +++ b/api/admin/templates.py @@ -58,6 +58,11 @@ """ +# Finland +ekirjasto_sign_in_template = """ +Sign in with Suomi.fi e-Identification +""" + forgot_password_template = """
diff --git a/core/model/admin.py b/core/model/admin.py index c286cbf05..4679a8746 100644 --- a/core/model/admin.py +++ b/core/model/admin.py @@ -44,6 +44,12 @@ class Admin(Base, HasSessionCache): "AdminRole", backref="admin", cascade="all, delete-orphan", uselist=True ) + admin_credentials: Mapped[list[AdminCredential]] = relationship( + "AdminCredential", + cascade="all, delete-orphan", + back_populates="admin", + ) + # Token age is max 30 minutes, in seconds RESET_PASSWORD_TOKEN_MAX_AGE = 1800 @@ -329,6 +335,20 @@ def compare_role(self, other: AdminRole) -> int: return self.GREATER_THAN +# Finland +# Store credentials from external authentication providers (e.g. e-kirjasto) +class AdminCredential(Base): + __tablename__ = "admincredentials" + + id = Column(Integer, primary_key=True) + external_id = Column(Unicode, nullable=False) + + admin_id = Column( + Integer, ForeignKey("admins.id", ondelete="CASCADE"), index=True, nullable=False + ) + admin: Mapped[Admin] = relationship("Admin", back_populates="admin_credentials") + + Index( "ix_adminroles_admin_id_library_id_role", AdminRole.admin_id, diff --git a/tests/api/admin/controller/test_individual_admins.py b/tests/api/admin/controller/test_individual_admins.py index c10a7a292..93b664fad 100644 --- a/tests/api/admin/controller/test_individual_admins.py +++ b/tests/api/admin/controller/test_individual_admins.py @@ -116,6 +116,7 @@ def test_individual_admins_get( }, {"role": AdminRole.SITEWIDE_LIBRARIAN}, ], + "authType": "PASSWORD", }, { "email": "admin3@nypl.org", @@ -125,6 +126,7 @@ def test_individual_admins_get( "library": str(db.default_library().short_name), } ], + "authType": "PASSWORD", }, { "email": "admin6@l2.org", @@ -133,6 +135,7 @@ def test_individual_admins_get( "role": AdminRole.SITEWIDE_LIBRARY_MANAGER, } ], + "authType": "PASSWORD", }, ] assert sorted( @@ -154,6 +157,7 @@ def test_individual_admins_get( { "email": "admin2@nypl.org", "roles": [{"role": AdminRole.SITEWIDE_LIBRARIAN}], + "authType": "PASSWORD", }, { "email": "admin4@l2.org", @@ -163,6 +167,7 @@ def test_individual_admins_get( "library": str(library2.short_name), } ], + "authType": "PASSWORD", }, { "email": "admin5@l2.org", @@ -172,6 +177,7 @@ def test_individual_admins_get( "library": str(library2.short_name), } ], + "authType": "PASSWORD", }, { "email": "admin6@l2.org", @@ -180,6 +186,7 @@ def test_individual_admins_get( "role": AdminRole.SITEWIDE_LIBRARY_MANAGER, } ], + "authType": "PASSWORD", }, ] assert sorted( @@ -206,6 +213,7 @@ def test_individual_admins_get( }, {"role": AdminRole.SITEWIDE_LIBRARIAN}, ], + "authType": "PASSWORD", }, { "email": "admin3@nypl.org", @@ -215,6 +223,7 @@ def test_individual_admins_get( "library": str(db.default_library().short_name), } ], + "authType": "PASSWORD", }, { "email": "admin4@l2.org", @@ -224,6 +233,7 @@ def test_individual_admins_get( "library": str(library2.short_name), } ], + "authType": "PASSWORD", }, { "email": "admin5@l2.org", @@ -233,6 +243,7 @@ def test_individual_admins_get( "library": str(library2.short_name), } ], + "authType": "PASSWORD", }, { "email": "admin6@l2.org", @@ -241,6 +252,7 @@ def test_individual_admins_get( "role": AdminRole.SITEWIDE_LIBRARY_MANAGER, } ], + "authType": "PASSWORD", }, ] assert sorted( diff --git a/tests/api/admin/controller/test_sign_in.py b/tests/api/admin/controller/test_sign_in.py index 9850bd7d3..bb395db8f 100644 --- a/tests/api/admin/controller/test_sign_in.py +++ b/tests/api/admin/controller/test_sign_in.py @@ -40,17 +40,18 @@ def test_admin_auth_providers(self, sign_in_fixture: SignInFixture): sign_in_fixture.ctrl.db.session, Admin, email="pw@nypl.org" ) pw_admin.password = "password" - assert 1 == len(ctrl.admin_auth_providers) + assert ctrl.admin_auth_providers assert { PasswordAdminAuthenticationProvider.NAME, } == {provider.NAME for provider in ctrl.admin_auth_providers} # Only an admin with a password is left. sign_in_fixture.ctrl.db.session.delete(sign_in_fixture.admin) - assert 1 == len(ctrl.admin_auth_providers) - assert { - PasswordAdminAuthenticationProvider.NAME, - } == {provider.NAME for provider in ctrl.admin_auth_providers} + assert [ + provider + for provider in ctrl.admin_auth_providers + if provider.NAME == PasswordAdminAuthenticationProvider.NAME + ] # No admins. No one can log in anymore sign_in_fixture.ctrl.db.session.delete(pw_admin)