Skip to content

[high] fix: [website] reuse the admin user row instead of inserting it on every login - #814

Open
elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/001-website-login-duplicate-user
Open

elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/001-website-login-duplicate-user

Conversation

@elhoim

@elhoim elhoim commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

BLUF — The website admin cannot log back in after the first logout: login 500s on a unique constraint.

  • Problem — The login view in website/app/account/account.py inserts a fresh User row with the unique email admin@admin.admin on every successful password check, and the matching delete in logout() is never committed, so the second login hits the unique constraint and raises IntegrityError out of the view.
  • Fix — Look the user up by email and only insert when absent, and remove the no-op delete from logout().
  • Effect — An admin can log back into the website UI after the first logout instead of getting a 500.

The login view unconditionally inserts a new User row on every successful password check, while the email column is unique:

if form.password.data == str(admin_password()):
    user = User(
        email="admin@admin.admin", first_name="admin", last_name="admin"
    )
    db.session.add(user)
    db.session.commit()
    login_user(user, form.remember_me.data)

logout() looked like it cleaned the row up, but the delete was never committed and got rolled back at session teardown:

@account_blueprint.route("/logout")
@login_required
def logout():
    User.query.filter_by(id=current_user.id).delete()
    logout_user()

Impact: the admin@admin.admin row from the first login is still there on the second login attempt. The unconditional insert then violates the email unique constraint, IntegrityError propagates out of the view, and the admin gets a 500 instead of a session — the account is effectively locked out of the web UI after the very first login/logout cycle.

Fix: login() now looks the user up by email first and only inserts when the row is absent, so re-logins reuse the existing account instead of colliding with it. The no-op delete() in logout() is removed (it never persisted, so removing it changes nothing observable) and logout_user() alone ends the session; the now-unused current_user import was dropped with it.

No behaviour change beyond fixing the crash: logout still ends the session the same way it always effectively did.

Verification

website/ has no automated test coverage in this repository. Verification was:

  • python -m py_compile on the changed file — clean (website/ is excluded from flake8 in CI; flake8 was also run manually on the file with no output).
  • Full module test suite: 161 passed, 4 skipped, 5 subtests passed in 247.81s (0:04:07).

Found during a review of the repository; other findings are being submitted as separate PRs.

🤖 Generated with Claude Code

…ery login

The login view built a fresh User(email="admin@admin.admin") and committed it
on every successful password check, but User.email is declared unique, and the
matching delete in logout() was never committed so the row survived session
teardown. The result is that the very first login after a logout raises
sqlalchemy IntegrityError on the unique index and the admin sees a 500 error
page, permanently locked out of the website until the database is wiped by
hand. Login now looks the admin row up by email and only inserts it when it is
absent, and the uncommitted delete is dropped from logout() so that
logout_user() alone ends the session; since that delete was rolled back at
teardown it never removed anything, so nothing observable changes there.
Dropping it also leaves current_user unused in this module, so its import goes
with it.

Verified with python -m py_compile on the changed file and the full test suite
against a live modules server: 161 passed, 4 skipped, 5 subtests passed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018dfYpyaSZd1nxSRLr8suj8
@elhoim elhoim changed the title fix: [website] reuse the admin user row instead of inserting it on every login [high] fix: [website] reuse the admin user row instead of inserting it on every login Sep 3, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant