Mirror
/
app-MAIL-temp
mirror of https://github.com/simple-login/app.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Display recovery codes for mfa only once (#1317) 

* Recovery codes can only be shown after adding a 2FA code and cannot be seen afterwards

* Added recovery codes fix

* Updated models and script

* Formatting

* Format

* Added base code

* Updated wording

* Set the config by default

Co-authored-by: Adrià Casajús <adria.casajus@proton.ch>
This commit is contained in:
Adrià Casajús 2022-10-03 12:32:45 +02:00 committed by GitHub
parent faaff7e9b9
commit faeddc365c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 137 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/auth/views/recovery.py
View File

@ -42,7 +42,7 @@ def recovery_route():
if recovery_form.validate_on_submit():
code = recovery_form.code.data
recovery_code = RecoveryCode.get_by(user_id=user.id, code=code)
recovery_code = RecoveryCode.find_by_user_code(user, code)
if recovery_code:
if recovery_code.used:

9
app/config.py
View File

@ -494,3 +494,12 @@ JOB_TAKEN_RETRY_WAIT_MINS = 30
# MEM_STORE
MEM_STORE_URI = os.environ.get("MEM_STORE_URI", None)
# Recovery codes hash salt
RECOVERY_CODE_HMAC_SECRET = os.environ.get("RECOVERY_CODE_HMAC_SECRET") or (
FLASK_SECRET + "generatearandomtoken"
)
if not RECOVERY_CODE_HMAC_SECRET or len(RECOVERY_CODE_HMAC_SECRET) < 16:
raise RuntimeError(
"Please define RECOVERY_CODE_HMAC_SECRET in your configuration with a random string at least 16 chars long"
)

1
app/dashboard/__init__.py
View File

@ -23,7 +23,6 @@ from .views import (
mailbox_detail,
refused_email,
referral,
recovery_code,
contact_detail,
setup_done,
batch_import,

8
app/dashboard/views/fido_setup.py
View File

@ -78,10 +78,10 @@ def fido_setup():
)
flash("Security key has been activated", "success")
if not RecoveryCode.filter_by(user_id=current_user.id).all():
return redirect(url_for("dashboard.recovery_code_route"))
else:
return redirect(url_for("dashboard.fido_manage"))
recovery_codes = RecoveryCode.generate(current_user)
return render_template(
"dashboard/recovery_code.html", recovery_codes=recovery_codes
)
# Prepare information for key registration process
fido_uuid = (

7
app/dashboard/views/mfa_setup.py
View File

@ -8,6 +8,7 @@ from app.dashboard.base import dashboard_bp
from app.dashboard.views.enter_sudo import sudo_required
from app.db import Session
from app.log import LOG
from app.models import RecoveryCode
class OtpTokenForm(FlaskForm):
@ -39,8 +40,10 @@ def mfa_setup():
current_user.last_otp = token
Session.commit()
flash("MFA has been activated", "success")
return redirect(url_for("dashboard.recovery_code_route"))
recovery_codes = RecoveryCode.generate(current_user)
return render_template(
"dashboard/recovery_code.html", recovery_codes=recovery_codes
)
else:
flash("Incorrect token", "warning")

30
app/dashboard/views/recovery_code.py
View File

@ -1,30 +0,0 @@
from flask import render_template, flash, redirect, url_for, request
from flask_login import login_required, current_user
from app.dashboard.base import dashboard_bp
from app.log import LOG
from app.models import RecoveryCode
@dashboard_bp.route("/recovery_code", methods=["GET", "POST"])
@login_required
def recovery_code_route():
if not current_user.two_factor_authentication_enabled():
flash("you need to enable either TOTP or WebAuthn", "warning")
return redirect(url_for("dashboard.index"))
recovery_codes = RecoveryCode.filter_by(user_id=current_user.id).all()
if request.method == "GET" and not recovery_codes:
# user arrives at this page for the first time
LOG.d("%s has no recovery keys, generate", current_user)
RecoveryCode.generate(current_user)
recovery_codes = RecoveryCode.filter_by(user_id=current_user.id).all()
if request.method == "POST":
RecoveryCode.generate(current_user)
flash("New recovery codes generated", "success")
return redirect(url_for("dashboard.recovery_code_route"))
return render_template(
"dashboard/recovery_code.html", recovery_codes=recovery_codes
)

31
app/models.py
View File

@ -2683,12 +2683,21 @@ class RecoveryCode(Base, ModelMixin):
__table_args__ = (sa.UniqueConstraint("user_id", "code", name="uq_recovery_code"),)
user_id = sa.Column(sa.ForeignKey(User.id, ondelete="cascade"), nullable=False)
code = sa.Column(sa.String(16), nullable=False)
code = sa.Column(sa.String(64), nullable=False)
used = sa.Column(sa.Boolean, nullable=False, default=False)
used_at = sa.Column(ArrowType, nullable=True, default=None)
user = orm.relationship(User)
@classmethod
def _hash_code(cls, code: str) -> str:
code_hmac = hmac.new(
config.RECOVERY_CODE_HMAC_SECRET.encode("utf-8"),
code.encode("utf-8"),
"sha3_224",
)
return base64.urlsafe_b64encode(code_hmac.digest()).decode("utf-8").rstrip("=")
@classmethod
def generate(cls, user):
"""generate recovery codes for user"""
@ -2697,14 +2706,27 @@ class RecoveryCode(Base, ModelMixin):
Session.flush()
nb_code = 0
raw_codes = []
while nb_code < _NB_RECOVERY_CODE:
code = random_string(_RECOVERY_CODE_LENGTH)
if not cls.get_by(user_id=user.id, code=code):
cls.create(user_id=user.id, code=code)
raw_code = random_string(_RECOVERY_CODE_LENGTH)
encoded_code = cls._hash_code(raw_code)
if not cls.get_by(user_id=user.id, code=encoded_code):
cls.create(user_id=user.id, code=encoded_code)
raw_codes.append(raw_code)
nb_code += 1
LOG.d("Create recovery codes for %s", user)
Session.commit()
return raw_codes
@classmethod
def find_by_user_code(cls, user: User, code: str):
hashed_code = cls._hash_code(code)
# TODO: Only return hashed codes once there aren't unhashed codes in the db.
found_code = cls.get_by(user_id=user.id, code=hashed_code)
if found_code:
return found_code
return cls.get_by(user_id=user.id, code=code)
@classmethod
def empty(cls, user):
@ -3301,7 +3323,6 @@ class NewsletterUser(Base, ModelMixin):
class ApiToCookieToken(Base, ModelMixin):
__tablename__ = "api_cookie_token"
code = sa.Column(sa.String(128), unique=True, nullable=False)
user_id = sa.Column(sa.ForeignKey(User.id, ondelete="cascade"), nullable=False)

29
migrations/versions/2022_092716_bd95b2b4217f_updated_recovery_code_string_length.py Normal file
View File

@ -0,0 +1,29 @@
"""Updated recovery code string length
Revision ID: bd95b2b4217f
Revises: 9cc0f0712b29
Create Date: 2022-09-27 16:14:35.021846
"""
import sqlalchemy_utils
from alembic import op
import sqlalchemy as sa
# revision identifiers, used by Alembic.
revision = 'bd95b2b4217f'
down_revision = '9cc0f0712b29'
branch_labels = None
depends_on = None
def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.execute('ALTER TABLE recovery_code ALTER COLUMN code TYPE VARCHAR(64)')
# ### end Alembic commands ###
def downgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.execute('ALTER TABLE recovery_code ALTER COLUMN code TYPE VARCHAR(16)')
# ### end Alembic commands ###

71
shell.py
View File

@ -4,27 +4,27 @@ import flask_migrate
from IPython import embed
from sqlalchemy_utils import create_database, database_exists, drop_database
from app import models
from app.config import DB_URI
from app.db import Session
from app.email_utils import send_email, render
from app.log import LOG
from app.models import *
from job_runner import (
onboarding_pgp,
onboarding_browser_extension,
onboarding_mailbox,
onboarding_send_from_alias,
)
def create_db():
if not database_exists(DB_URI):
LOG.d("db not exist, create database")
create_database(DB_URI)
if False:
# noinspection PyUnreachableCode
def create_db():
if not database_exists(DB_URI):
LOG.d("db not exist, create database")
create_database(DB_URI)
# Create all tables
# Use flask-migrate instead of db.create_all()
flask_migrate.upgrade()
# Create all tables
# Use flask-migrate instead of db.create_all()
flask_migrate.upgrade()
# noinspection PyUnreachableCode
def reset_db():
if database_exists(DB_URI):
drop_database(DB_URI)
create_db()
def change_password(user_id, new_password):
@ -33,10 +33,41 @@ def change_password(user_id, new_password):
Session.commit()
def reset_db():
if database_exists(DB_URI):
drop_database(DB_URI)
create_db()
def migrate_recovery_codes():
last_id = -1
while True:
recovery_codes = (
RecoveryCode.filter(RecoveryCode.id > last_id)
.order_by(RecoveryCode.id)
.limit(100)
.all()
)
batch_codes = len(recovery_codes)
old_codes = 0
new_codes = 0
last_code = None
last_code_id = None
for recovery_code in recovery_codes:
if len(recovery_code.code) == models._RECOVERY_CODE_LENGTH:
last_code = recovery_code.code
last_code_id = recovery_code.id
recovery_code.code = RecoveryCode._hash_code(recovery_code.code)
old_codes += 1
Session.flush()
else:
new_codes += 1
last_id = recovery_code.id
Session.commit()
LOG.i(
f"Updated {old_codes}/{batch_codes} for this batch ({new_codes} already updated)"
)
if last_code is not None:
recovery_code = RecoveryCode.get_by(id=last_code_id)
assert RecoveryCode._hash_code(last_code) == recovery_code.code
LOG.i("Check is Good")
if len(recovery_codes) == 0:
break
if __name__ == "__main__":

28
templates/dashboard/recovery_code.html
View File

@ -12,29 +12,15 @@
your account again.
Each code can only be used once, make sure to store them in a safe place.
</p>
<p class="alert alert-warning">
<strong>
If you had recovery codes before, they have been invalidated.
Store these codes in a safe place. You won't be able to retrieve them again!
</strong>
</p>
<ul>
{% for recovery_code in recovery_codes %}
{% if recovery_code.used %}
<li>
<span style="text-decoration: line-through">{{ recovery_code.code }}</span>.
Used {{ recovery_code.used_at | dt }}.
</li>
{% else %}
<li>{{ recovery_code.code }}</li>
{% endif %}
{% endfor %}
{% for recovery_code in recovery_codes %}<li>{{ recovery_code }}</li>{% endfor %}
</ul>
<form method="post" class="mt-6">
<input type="submit"
class="btn btn-outline-primary"
value="Generate New Codes">
</form>
<div class="small-text">Warning: Generating new codes will invalidate the older ones.</div>
<hr />
<a href="{{ url_for('dashboard.index') }}"
class="btn btn-primary btn-lg">Back to the home page</a>
</div>
</div>
{% endblock %}

12
templates/dashboard/setting.html
View File

@ -102,8 +102,6 @@
{% else %}
<a href="{{ url_for('dashboard.mfa_cancel') }}"
class="btn btn-outline-danger">Disable TOTP</a>
<a href="{{ url_for('dashboard.recovery_code_route') }}"
class="btn btn-outline-secondary">Recovery Codes</a>
{% endif %}
</div>
</div>
@ -124,8 +122,6 @@
{% else %}
<a href="{{ url_for('dashboard.fido_manage') }}"
class="btn btn-outline-info">Manage WebAuthn</a>
<a href="{{ url_for('dashboard.recovery_code_route') }}"
class="btn btn-outline-secondary">Recovery Codes</a>
{% endif %}
</div>
</div>
@ -264,14 +260,10 @@
<div class="card" id="change_password">
<div class="card-body">
<div class="card-title">Password</div>
<div class="mb-3">
You will receive an email containing instructions on how to change your password.
</div>
<div class="mb-3">You will receive an email containing instructions on how to change your password.</div>
<form method="post">
<input type="hidden" name="form-name" value="change-password">
<button class="btn btn-outline-primary">
Change password
</button>
<button class="btn btn-outline-primary">Change password</button>
</form>
</div>
</div>

4
tests/test.env
View File

@ -61,4 +61,6 @@ PROTON_CLIENT_ID=to_fill
PROTON_CLIENT_SECRET=to_fill
PROTON_BASE_URL=https://localhost/api
POSTMASTER=postmaster@test.domain
POSTMASTER=postmaster@test.domain
RECOVERY_CODE_HMAC_SECRET=1234567890123456789