diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000..1cd3021 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,19 @@ +## What + +_what the PR changes_ + +## Why + +_why these changes were made_ + +## Test Plan + +_how did you verify these changes did what you expected_ + +## Env Vars + +_did you add, remove, or rename any environment variables_ + +## Checklist + +- [ ] Tested all changes locally diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index 62b63ea..bf376a9 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -18,10 +18,10 @@ jobs: - name: Install ldap dependencies run: sudo apt-get install libldap2-dev libsasl2-dev - uses: actions/checkout@v2 - - name: Set up Python 3.6 + - name: Set up Python 3.13 uses: actions/setup-python@v2 with: - python-version: 3.6 + python-version: 3.13 - name: Install dependencies run: | python -m pip install --upgrade pip diff --git a/.pylintrc b/.pylintrc index e8fdc7e..05e557d 100644 --- a/.pylintrc +++ b/.pylintrc @@ -16,7 +16,7 @@ persistent=yes # List of plugins (as comma separated values of python modules names) to load, # usually to register additional checkers. -load-plugins= +#load-plugins= # Use multiple processes to speed up Pylint. jobs=1 @@ -28,14 +28,14 @@ unsafe-load-any-extension=no # A comma-separated list of package or module names from where C extensions may # be loaded. Extensions are loading into the active Python interpreter and may # run arbitrary code -extension-pkg-whitelist= +#extension-pkg-whitelist= [MESSAGES CONTROL] # Only show warnings with the listed confidence levels. Leave empty to show # all. Valid levels: HIGH, INFERENCE, INFERENCE_FAILURE, UNDEFINED -confidence= +#confidence= # Enable the message, report, category or checker with the given id(s). You can # either give multiple identifier separated by comma (,) or put this option @@ -62,12 +62,11 @@ disable= too-few-public-methods, no-member, too-many-format-args, - bad-continuation, bare-except, inconsistent-return-statements, no-name-in-module, cyclic-import, - unnecessary-pass, + unnecessary-pass [REPORTS] @@ -77,11 +76,6 @@ disable= # mypackage.mymodule.MyReporterClass. output-format=text -# Put messages in a separate file for each module / package specified on the -# command line instead of printing them on stdout. Reports (if any) will be -# written in a file name "pylint_global.[txt|html]". -files-output=no - # Tells whether to display a full report or only the messages reports=no @@ -136,7 +130,7 @@ dummy-variables-rgx=_$|dummy # List of additional names supposed to be defined in builtins. Remember that # you should avoid to define new builtins when possible. -additional-builtins= +#additional-builtins= # List of strings which can identify a callback function by name. A callback # name must start or end with one of those strings. @@ -155,9 +149,6 @@ ignore-long-lines=^\s*(# )??$ # else. single-line-if-stmt=no -# List of optional constructs for which whitespace checking is disabled -no-space-check=trailing-comma,dict-separator - # Maximum number of lines in a module max-module-lines=2000 @@ -169,14 +160,11 @@ indent-string=' ' indent-after-paren=4 # Expected format of line ending, e.g. empty (any line ending), LF or CRLF. -expected-line-ending-format= +#expected-line-ending-format= [BASIC] -# List of builtins function names that should not be used, separated by a comma -bad-functions=map,filter,input - # Good variable names which should always be accepted, separated by a comma good-names=i,j,k,ex,Run,_ @@ -185,7 +173,7 @@ bad-names=foo,bar,baz,toto,tutu,tata # Colon-delimited sets of names that determine each other's naming style when # the name regexes allow several styles. -name-group= +#name-group= # Include a hint for the correct naming format with invalid-name include-naming-hint=no @@ -193,63 +181,33 @@ include-naming-hint=no # Regular expression matching correct function names function-rgx=[a-z_][a-z0-9_]{2,30}$ -# Naming hint for function names -function-name-hint=[a-z_][a-z0-9_]{2,30}$ - # Regular expression matching correct variable names variable-rgx=[a-z_][a-z0-9_]{2,30}$ -# Naming hint for variable names -variable-name-hint=[a-z_][a-z0-9_]{2,30}$ - # Regular expression matching correct constant names const-rgx=[a-z_][a-z0-9_]{2,30}$ -# Naming hint for constant names -const-name-hint=(([A-Z_][A-Z0-9_]*)|(__.*__))$ - # Regular expression matching correct attribute names attr-rgx=[a-z_][a-z0-9_]{2,30}$ -# Naming hint for attribute names -attr-name-hint=[a-z_][a-z0-9_]{2,30}$ - # Regular expression matching correct argument names argument-rgx=[a-z_][a-z0-9_]{2,30}$ -# Naming hint for argument names -argument-name-hint=[a-z_][a-z0-9_]{2,30}$ - # Regular expression matching correct class attribute names class-attribute-rgx=([A-Za-z_][A-Za-z0-9_]{2,30}|(__.*__))$ -# Naming hint for class attribute names -class-attribute-name-hint=([A-Za-z_][A-Za-z0-9_]{2,30}|(__.*__))$ - # Regular expression matching correct inline iteration names inlinevar-rgx=[A-Za-z_][A-Za-z0-9_]*$ -# Naming hint for inline iteration names -inlinevar-name-hint=[A-Za-z_][A-Za-z0-9_]*$ - # Regular expression matching correct class names class-rgx=[A-Z_][a-zA-Z0-9]+$ -# Naming hint for class names -class-name-hint=[A-Z_][a-zA-Z0-9]+$ - # Regular expression matching correct module names module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$ -# Naming hint for module names -module-name-hint=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$ - # Regular expression matching correct method names method-rgx=[a-z_][a-z0-9_]{2,30}$ -# Naming hint for method names -method-name-hint=[a-z_][a-z0-9_]{2,30}$ - # Regular expression which should only match function or class names that do # not require a docstring. no-docstring-rgx=__.*__ @@ -271,11 +229,11 @@ ignore-mixin-members=yes # List of module names for which member attributes should not be checked # (useful for modules/projects where namespaces are manipulated during runtime # and thus existing member attributes cannot be deduced by static analysis -ignored-modules= +#ignored-modules= # List of classes names for which member attributes should not be checked # (useful for classes with attributes dynamically set). -ignored-classes=SQLObject, optparse.Values, thread._local, _thread._local +ignored-classes=SQLObject,optparse.Values,thread._local,_thread._local # List of members which are set dynamically and missed by pylint inference # system, and so shouldn't trigger E1101 when accessed. Python regular @@ -291,13 +249,13 @@ contextmanager-decorators=contextlib.contextmanager # Spelling dictionary name. Available dictionaries: none. To make it working # install python-enchant package. -spelling-dict= +#spelling-dict= # List of comma separated words that should not be checked. -spelling-ignore-words= +#spelling-ignore-words= # A path to a file that contains private dictionary; one word per line. -spelling-private-dict-file= +#spelling-private-dict-file= # Tells whether to store unknown words to indicated private dictionary in # --spelling-private-dict-file option instead of raising a message. @@ -361,19 +319,19 @@ deprecated-modules=regsub,TERMIOS,Bastion,rexec # Create a graph of every (i.e. internal and external) dependencies in the # given file (report RP0402 must not be disabled) -import-graph= +#import-graph= # Create a graph of external dependencies in the given file (report RP0402 must # not be disabled) -ext-import-graph= +#ext-import-graph= # Create a graph of internal dependencies in the given file (report RP0402 must # not be disabled) -int-import-graph= +#int-import-graph= [EXCEPTIONS] # Exceptions that will emit a warning when being caught. Defaults to # "Exception" -overgeneral-exceptions=Exception +overgeneral-exceptions=builtins.Exception diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 0000000..955f438 --- /dev/null +++ b/Dockerfile @@ -0,0 +1,20 @@ +FROM docker.io/python:3.13-slim +MAINTAINER Computer Science House + +RUN mkdir /opt/selfservice + +ADD requirements.txt /opt/selfservice + +WORKDIR /opt/selfservice + +RUN apt-get -yq update && \ + apt-get -yq install libsasl2-dev libldap2-dev libldap-common libssl-dev git gcc g++ make && \ + pip install -r requirements.txt && \ + apt-get -yq clean all + +ADD . /opt/selfservice + +EXPOSE 8080 + +CMD ["gunicorn", "selfservice:app", "--bind=0.0.0.0:8080", "--access-logfile=-", "--timeout=256"] + diff --git a/config.env.py b/config.env.py index e510b81..cfb8823 100644 --- a/config.env.py +++ b/config.env.py @@ -35,12 +35,12 @@ ) SQLALCHEMY_TRACK_MODIFICATIONS = False -RECAPTCHA_ENABLED = True -RECAPTCHA_SITE_KEY = os.environ.get("RECAPTCHA_SITE_KEY", "") -RECAPTCHA_SECRET_KEY = os.environ.get("RECAPTCHA_SECRET_KEY", "") -RECAPTCHA_THEME = "light" -RECAPTCHA_TYPE = "image" -RECAPTCHA_SIZE = "normal" +XCAPTCHA_ENABLED = True +XCAPTCHA_SITE_KEY = os.environ.get("XCAPTCHA_SITE_KEY", "") +XCAPTCHA_SECRET_KEY = os.environ.get("XCAPTCHA_SECRET_KEY", "") +XCAPTCHA_THEME = "light" +XCAPTCHA_TYPE = "image" +XCAPTCHA_SIZE = "normal" TWILIO_SID = os.environ.get("TWILIO_SID", "") TWILIO_TOKEN = os.environ.get("TWILIO_TOKEN", "") diff --git a/docker-compose.yaml b/docker-compose.yaml index e088131..fbc8ff6 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -1,7 +1,7 @@ version: '2' services: postgres: - image: postgres:9.6 + image: docker.io/postgres:9.6 container_name: selfservice-postgres restart: always volumes: @@ -12,7 +12,7 @@ services: ports: - 127.0.0.1:5433:5432 phppgadmin: - image: bitnami/phppgadmin:latest + image: docker.io/dockage/phppgadmin:latest container_name: selfservice-pgadmin links: - postgres @@ -22,4 +22,4 @@ services: restart: always ports: - 127.0.0.1:8081:8080 - - 127.0.0.1:8444:8443 \ No newline at end of file + - 127.0.0.1:8444:8443 diff --git a/migrations/versions/a83f363599a0_.py b/migrations/versions/a83f363599a0_.py index 959c8e8..6441be0 100644 --- a/migrations/versions/a83f363599a0_.py +++ b/migrations/versions/a83f363599a0_.py @@ -20,8 +20,8 @@ def upgrade(): # ### commands auto generated by Alembic - please adjust! ### op.create_table('otp_session', sa.Column('secret', sa.String(length=100), nullable=False), - sa.Column('form', sa.Binary(), nullable=True), - sa.Column('session', sa.Binary(), nullable=True), + sa.Column('form', sa.LargeBinary(), nullable=True), + sa.Column('session', sa.LargeBinary(), nullable=True), sa.PrimaryKeyConstraint('secret') ) op.create_table('session', diff --git a/migrations/versions/fdb69cd98e19_remove_otpsession_table.py b/migrations/versions/fdb69cd98e19_remove_otpsession_table.py new file mode 100644 index 0000000..6d03ce6 --- /dev/null +++ b/migrations/versions/fdb69cd98e19_remove_otpsession_table.py @@ -0,0 +1,33 @@ +"""Remove OTPSession table + +Revision ID: fdb69cd98e19 +Revises: a541afdca952 +Create Date: 2025-12-21 15:34:01.851353 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = 'fdb69cd98e19' +down_revision = 'a541afdca952' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_table('otp_session') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('otp_session', + sa.Column('secret', sa.VARCHAR(length=100), autoincrement=False, nullable=False), + sa.Column('form', postgresql.BYTEA(), autoincrement=False, nullable=True), + sa.Column('session', postgresql.BYTEA(), autoincrement=False, nullable=True), + sa.PrimaryKeyConstraint('secret', name=op.f('otp_session_pkey')) + ) + # ### end Alembic commands ### diff --git a/requirements.dev b/requirements.dev deleted file mode 100644 index 8394dae..0000000 --- a/requirements.dev +++ /dev/null @@ -1,22 +0,0 @@ -beautifulsoup4 -black -bs4 -csh-ldap>=2.2.0 -dill -dnspython<2.0.0 -Flask -Flask-Limiter -Flask-Migrate -Flask-pyoidc -Flask-QRcode -Flask-ReCaptcha -Flask-SQLAlchemy -psycopg2 -pylint -python-freeipa -python-keycloak -qrcode -requests -sentry-sdk[flask]~=0.13.1 -srvlookup -twillio \ No newline at end of file diff --git a/requirements.in b/requirements.in new file mode 100644 index 0000000..3af5a9d --- /dev/null +++ b/requirements.in @@ -0,0 +1,20 @@ +Flask~=3.1.2 +Flask-pyoidc~=3.14.3 +Flask-Migrate~=4.1.0 +Flask-SQLAlchemy~=3.1.1 +Flask-Limiter~=4.1.1 +Flask-QRcode~=3.2.0 +Flask-xCaptcha~=0.5.5 +srvlookup~=3.0.0 +csh-ldap @ git+https://github.com/costowell/csh_ldap@cole-dev +python-freeipa~=1.0.10 +python-keycloak~=5.8.1 +sentry-sdk[Flask] +psycopg2-binary~=2.9.11 +twilio~=9.9.0 +pyotp~=2.9.0 +passlib~=1.7.4 +xkcdpass~=1.20.0 +gunicorn~=23.0.0 +black~=25.12.0 +pylint~=4.0.4 diff --git a/requirements.txt b/requirements.txt index ee9be7c..885a21b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,93 +1,260 @@ -alabaster==0.7.10 -alembic==0.9.9 -appdirs==1.4.3 -asn1crypto==0.24.0 -aspy.yaml==1.1.1 -astroid==2.4.2 -attrs==18.1.0 -Beaker==1.9.1 -beautifulsoup4==4.9.1 -black==20.8b1 -blinker==1.4 -bs4==0.0.1 -cached-property==1.4.2 -certifi==2018.4.16 -cffi==1.13.2 -cfgv==1.0.0 -chardet==3.0.4 -click==7.1.2 -cryptography==3.2 -csh-ldap==2.3.1 -dataclasses==0.7 -defusedxml==0.6.0 -dill==0.3.2 -distlib==0.3.1 -dnspython==1.16.0 -ecdsa==0.13.3 -filelock==3.0.12 -Flask==1.1.2 -Flask-Limiter==1.4 -Flask-Migrate==2.5.3 -Flask-pyoidc==3.5.1 -Flask-QRcode==3.0.0 -Flask-ReCaptcha==0.4.2 -Flask-SQLAlchemy==2.4.4 -future==0.16.0 -gunicorn==19.8.1 -httmock==1.2.5 -identify==1.0.18 -idna==2.6 -importlib-metadata==1.7.0 -importlib-resources==3.3.0 -isort==4.3.4 -itsdangerous==0.24 -Jinja2==2.10.1 -lazy-object-proxy==1.4.3 -limits==1.3 -Mako==1.0.7 -MarkupSafe==1.0 -mccabe==0.6.1 -mypy-extensions==0.4.3 -nodeenv==1.3.0 -oic==1.1.2 -passlib==1.7.1 -pathspec==0.8.0 -Pillow==6.2.2 -pre-commit==1.10.1 -psycopg2==2.8.5 -pyasn1==0.4.2 -pyasn1-modules==0.2.1 -pycparser==2.18 -pycrypto==2.6.1 -pycryptodomex==3.6.1 -pyjwkest==1.4.0 -PyJWT==1.7.1 -pylint==2.6.0 -pyOpenSSL==18.0.0 -pyotp==2.2.6 -python-dateutil==2.7.3 -python-editor==1.0.3 -python-freeipa==1.0.6 -python-jose==1.4.0 -python-keycloak==0.22.0 -python-ldap~=3.0.0 -pytz==2020.1 -PyYAML==5.4.1 -qrcode==6.1 -regex==2020.7.14 -requests==2.24.0 -sentry-sdk==0.13.5 -six==1.12.0 -soupsieve==2.0.1 -SQLAlchemy==1.3.0 -srvlookup==2.0.0 -toml==0.10.1 -twilio==6.45.1 -typed-ast==1.4.1 -typing-extensions==3.7.4.3 -urllib3==1.24.2 -virtualenv==16.0.0 -Werkzeug==0.15.3 -wrapt==1.11.2 -xkcdpass==1.16.5 -zipp==3.1.0 +# This file was autogenerated by uv via the following command: +# uv pip compile requirements.in +aiofiles==25.1.0 + # via python-keycloak +aiohappyeyeballs==2.6.1 + # via aiohttp +aiohttp==3.13.2 + # via + # aiohttp-retry + # twilio +aiohttp-retry==2.9.1 + # via twilio +aiosignal==1.4.0 + # via aiohttp +alembic==1.17.2 + # via flask-migrate +annotated-types==0.7.0 + # via pydantic +anyio==4.12.0 + # via httpx +astroid==4.0.2 + # via pylint +async-property==0.2.2 + # via python-keycloak +attrs==25.4.0 + # via aiohttp +black==25.12.0 + # via -r requirements.in +blinker==1.9.0 + # via + # flask + # sentry-sdk +certifi==2025.11.12 + # via + # httpcore + # httpx + # requests + # sentry-sdk +cffi==2.0.0 + # via cryptography +charset-normalizer==3.4.4 + # via requests +click==8.3.1 + # via + # black + # flask +cryptography==46.0.3 + # via + # jwcrypto + # oic +csh-ldap @ git+https://github.com/costowell/csh_ldap@dbc3193482615b4cd52c5d1fb260662ec8157ace + # via -r requirements.in +defusedxml==0.7.1 + # via oic +deprecated==1.3.1 + # via limits +deprecation==2.1.0 + # via python-keycloak +dill==0.4.0 + # via pylint +dnspython==2.8.0 + # via srvlookup +flask==3.1.2 + # via + # -r requirements.in + # flask-limiter + # flask-migrate + # flask-pyoidc + # flask-qrcode + # flask-sqlalchemy + # flask-xcaptcha + # sentry-sdk +flask-limiter==4.1.1 + # via -r requirements.in +flask-migrate==4.1.0 + # via -r requirements.in +flask-pyoidc==3.14.3 + # via -r requirements.in +flask-qrcode==3.2.0 + # via -r requirements.in +flask-sqlalchemy==3.1.1 + # via + # -r requirements.in + # flask-migrate +flask-xcaptcha==0.5.5 + # via -r requirements.in +frozenlist==1.8.0 + # via + # aiohttp + # aiosignal +future==1.0.0 + # via pyjwkest +greenlet==3.3.0 + # via sqlalchemy +gunicorn==23.0.0 + # via -r requirements.in +h11==0.16.0 + # via httpcore +httpcore==1.0.9 + # via httpx +httpx==0.28.1 + # via python-keycloak +idna==3.11 + # via + # anyio + # httpx + # requests + # yarl +importlib-resources==6.5.2 + # via flask-pyoidc +isort==7.0.0 + # via pylint +itsdangerous==2.2.0 + # via flask +jinja2==3.1.6 + # via flask +jwcrypto==1.5.6 + # via python-keycloak +limits==5.6.0 + # via flask-limiter +mako==1.3.10 + # via + # alembic + # oic +markupsafe==3.0.3 + # via + # flask + # flask-xcaptcha + # jinja2 + # mako + # sentry-sdk + # werkzeug +mccabe==0.7.0 + # via pylint +multidict==6.7.0 + # via + # aiohttp + # yarl +mypy-extensions==1.1.0 + # via black +oic==1.6.1 + # via flask-pyoidc +ordered-set==4.1.0 + # via flask-limiter +packaging==25.0 + # via + # black + # deprecation + # gunicorn + # limits +passlib==1.7.4 + # via -r requirements.in +pathspec==0.12.1 + # via black +pillow==12.0.0 + # via flask-qrcode +platformdirs==4.5.1 + # via + # black + # pylint +propcache==0.4.1 + # via + # aiohttp + # yarl +psycopg2-binary==2.9.11 + # via -r requirements.in +pyasn1==0.6.1 + # via + # pyasn1-modules + # python-ldap +pyasn1-modules==0.4.2 + # via python-ldap +pycparser==2.23 + # via cffi +pycryptodomex==3.23.0 + # via + # oic + # pyjwkest +pydantic==2.12.5 + # via pydantic-settings +pydantic-core==2.41.5 + # via pydantic +pydantic-settings==2.12.0 + # via oic +pyjwkest==1.4.4 + # via oic +pyjwt==2.10.1 + # via twilio +pylint==4.0.4 + # via -r requirements.in +pyotp==2.9.0 + # via -r requirements.in +python-dotenv==1.2.1 + # via pydantic-settings +python-freeipa==1.0.10 + # via -r requirements.in +python-keycloak==5.8.1 + # via -r requirements.in +python-ldap==3.4.5 + # via csh-ldap +pytokens==0.3.0 + # via black +qrcode==8.2 + # via flask-qrcode +requests==2.32.5 + # via + # flask-pyoidc + # flask-xcaptcha + # oic + # pyjwkest + # python-freeipa + # python-keycloak + # requests-toolbelt + # twilio +requests-toolbelt==1.0.0 + # via python-keycloak +sentry-sdk==2.48.0 + # via -r requirements.in +six==1.17.0 + # via pyjwkest +sqlalchemy==2.0.45 + # via + # alembic + # flask-sqlalchemy +srvlookup==3.0.0 + # via + # -r requirements.in + # csh-ldap +tomlkit==0.13.3 + # via pylint +twilio==9.9.0 + # via -r requirements.in +typing-extensions==4.15.0 + # via + # aiosignal + # alembic + # anyio + # flask-limiter + # jwcrypto + # limits + # pydantic + # pydantic-core + # sqlalchemy + # typing-inspection +typing-inspection==0.4.2 + # via + # pydantic + # pydantic-settings +urllib3==2.6.2 + # via + # requests + # sentry-sdk +werkzeug==3.1.4 + # via flask +wrapt==2.0.1 + # via deprecated +xkcdpass==1.20.0 + # via -r requirements.in +yarl==1.22.0 + # via aiohttp diff --git a/selfservice/__init__.py b/selfservice/__init__.py index f8e2889..4c41586 100644 --- a/selfservice/__init__.py +++ b/selfservice/__init__.py @@ -13,7 +13,7 @@ from flask import Flask, render_template, request, redirect, url_for, flash from flask_pyoidc.flask_pyoidc import OIDCAuthentication from flask_pyoidc.provider_configuration import ProviderConfiguration, ClientMetadata -from flask_recaptcha import ReCaptcha +from flask_xcaptcha import XCaptcha from flask_sqlalchemy import SQLAlchemy from python_freeipa import Client from flask_limiter import Limiter @@ -52,9 +52,9 @@ migrate = Migrate(app, db) -# Create recaptcha object -recaptcha = ReCaptcha() -recaptcha.init_app(app) +# Create xcaptcha object +if app.config["XCAPTCHA_ENABLED"]: + xcaptcha = XCaptcha(app=app) # OIDC Initialization OIDC_PROVIDER = "csh" @@ -77,8 +77,10 @@ # Configure rate limiting if not app.config["DEBUG"]: limiter = Limiter( - app, key_func=get_remote_address, default_limits=["50 per day", "10 per hour"] + get_remote_address, app=app, default_limits=["50 per day", "10 per hour"] ) +else: + limiter = Limiter(get_remote_address, app=app, default_limits=[]) # Initialize QR Code Generator qr = QRcode(app) diff --git a/selfservice/blueprints/otp.py b/selfservice/blueprints/otp.py index d17ce04..d98f269 100644 --- a/selfservice/blueprints/otp.py +++ b/selfservice/blueprints/otp.py @@ -7,20 +7,20 @@ from flask import Blueprint, render_template, request, redirect, flash from flask import session as flask_session -import dill as pickle from selfservice.utilities.keycloak import ( OTPConfigError, - create_kc_otp, - confirm_kc_otp, + OTPInvalidCode, OTPAlreadyConfigured, + get_kc_otp_is_registered, + generate_kc_otp, + register_kc_otp, delete_kc_otp, ) -from selfservice.utilities.ldap import create_ipa_otp, delete_ipa_otp +from selfservice.utilities.ldap import create_ipa_otp, has_ipa_otp, delete_ipa_otp from selfservice.utilities.app_passwd import set_app_passwd, delete_app_passwd -from selfservice.models import OTPSession -from selfservice import version, auth, db, OIDC_PROVIDER +from selfservice import version, auth, OIDC_PROVIDER otp_bp = Blueprint("otp", __name__) @@ -39,52 +39,67 @@ def enable(): otp_code = request.form.get("otp-code", default="") if request.method == "GET": - try: - session, form_data, secret = create_kc_otp(username) - - save_session = OTPSession( - secret=secret, - form=pickle.dumps(form_data), - session=pickle.dumps(session), + kc_registered = get_kc_otp_is_registered(username) + ipa_registered = has_ipa_otp(username) + + # If its registered in one place but not the other + if kc_registered != ipa_registered: + LOG.warning( + "%s does not have TOTP in both Keycloak and LDAP " + "(kc_registered=%s, ipa_registered=%s)", + username, + kc_registered, + ipa_registered, ) - db.session.add(save_session) - db.session.commit() - except OTPAlreadyConfigured: + + # If already registered *somewhere* + if kc_registered or ipa_registered: return render_template("otp.html", version=version, configured=True) + secret = generate_kc_otp(username) otp_uri = pyotp.totp.TOTP(secret).provisioning_uri( - "{}@csh.rit.edu".format(username), issuer_name="CSH" + f"{username}@csh.rit.edu", issuer_name="CSH" ) return render_template( "otp.html", version=version, otp_uri=otp_uri, secret=secret ) - otp_session = OTPSession.query.filter_by(secret=secret).first() - - if not secret or not otp_session: + otp_uri = pyotp.totp.TOTP(secret).provisioning_uri( + f"{username}@csh.rit.edu", issuer_name="CSH" + ) + if not secret: flash("Invalid secret provided. Please try again.") return redirect("/otp") if not otp_code: - flash("No one time password provided. Please scan the code and try again.") - return redirect("/otp".format(secret)) - - session = pickle.loads(otp_session.session) - form = pickle.loads(otp_session.form) + flash( + "One time password provided did not match expected value. " + "Please scan the code and try again." + ) + return render_template( + "otp.html", version=version, otp_uri=otp_uri, secret=secret + ) try: - confirm_kc_otp(session, form) + register_kc_otp(username, secret, otp_code) + except OTPInvalidCode: + flash( + "One time password provided did not match expected value." + "Please scan the code and try again." + ) + return render_template( + "otp.html", version=version, otp_uri=otp_uri, secret=secret + ) except OTPConfigError: - flash("Invalid one time code provided or session expired.") + flash("Invalid secret, try again.") + return redirect("/otp") + except OTPAlreadyConfigured: + flash("2FA already configured.") return redirect("/otp") create_ipa_otp(username, secret) app_passwd = set_app_passwd(username) - # Clean up used session data - OTPSession.query.filter_by(secret=secret).delete() - db.session.commit() - return render_template( "otp.html", version=version, configured=True, passwd=app_passwd ) diff --git a/selfservice/blueprints/recovery.py b/selfservice/blueprints/recovery.py index 1efbb1b..673b5a4 100644 --- a/selfservice/blueprints/recovery.py +++ b/selfservice/blueprints/recovery.py @@ -18,7 +18,7 @@ from selfservice.utilities.ldap import verif_methods, get_members from selfservice.models import RecoverySession, PhoneVerification, ResetToken -from selfservice import db, auth, recaptcha, ldap, version, OIDC_PROVIDER +from selfservice import db, auth, xcaptcha, ldap, version, OIDC_PROVIDER LOG = logging.getLogger(__name__) @@ -35,7 +35,7 @@ def create_session(): if request.method == "GET": return render_template("recovery.html", version=version) - if recaptcha.verify(): + if xcaptcha.verify(): # If we can't find an account, flash error. try: @@ -96,8 +96,8 @@ def verify_identity(recovery_id): # Make sure that methods are valid possible_methods = 0 - for method in methods: - if methods[method]: + for _, value in methods.items(): + if value: possible_methods += 1 if possible_methods == 0: @@ -247,7 +247,7 @@ def reset_password(): else: flash("Whoops, those passwords didn't match!") - return redirect("/reset?token={}".format(token)) + return redirect(f"/reset?token={token}") @recovery_bp.route("/admin", methods=["GET", "POST"]) diff --git a/selfservice/models.py b/selfservice/models.py index 7e73bc5..06cfeb9 100644 --- a/selfservice/models.py +++ b/selfservice/models.py @@ -9,7 +9,6 @@ ForeignKey, DateTime, Boolean, - Binary, func, ) from selfservice import db @@ -24,7 +23,7 @@ class ResetToken(db.Model): __tablename__ = "token" id = Column(Integer, primary_key=True) username = Column(String(64), nullable=False) - created = Column(DateTime, default=func.timezone("UTC", func.current_timestamp())) + created = Column(DateTime, default=func.timezone("UTC", func.current_timestamp)) token = Column(String(36)) session = Column(String(36), ForeignKey("session.id")) used = Column(Boolean) @@ -40,7 +39,7 @@ class RecoverySession(db.Model): __tablename__ = "session" id = Column(String(36), primary_key=True) username = Column(String(64), nullable=False) - created = Column(DateTime, default=func.timezone("UTC", func.current_timestamp())) + created = Column(DateTime, default=func.timezone("UTC", func.current_timestamp)) class PhoneVerification(db.Model): @@ -54,18 +53,6 @@ class PhoneVerification(db.Model): session = Column(String(36), ForeignKey("session.id")) -class OTPSession(db.Model): - """ - Once an OTP secret has been generated for a user, we pickle their session - so that we can retrieve it and eventually verify their token. - """ - - __tablename__ = "otp_session" - secret = Column(String(100), primary_key=True) - form = Column(Binary) - session = Column(Binary) - - class AppSpecificPassword(db.Model): """ Allows users to authenticate to applications that don't support two factor diff --git a/selfservice/templates/recovery.html b/selfservice/templates/recovery.html index 6d186b1..743eb2f 100644 --- a/selfservice/templates/recovery.html +++ b/selfservice/templates/recovery.html @@ -17,7 +17,7 @@

Account Recovery

- {{recaptcha}} + {{xcaptcha}}