Skip to content

Commit

Permalink
Rebase: add flash messaging for expiration, logout, etc.
Browse files Browse the repository at this point in the history
  • Loading branch information
lsd-cat committed Sep 14, 2022
1 parent c15f4ad commit a47b9bd
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 63 deletions.
2 changes: 1 addition & 1 deletion securedrop/i18n.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ def get_locale(config: SDConfig) -> str:
- config.FALLBACK_LOCALE
"""
preferences = []
if session.get("locale"):
if session and session.get("locale"):
preferences.append(session.get("locale"))
if request.args.get("l"):
preferences.insert(0, request.args.get("l"))
Expand Down
5 changes: 2 additions & 3 deletions securedrop/journalist_app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from datetime import datetime, timedelta, timezone
from pathlib import Path
from datetime import datetime
from flask import (Flask, redirect, url_for, flash, g, request,
from flask import (Flask, redirect, url_for, g, request,
render_template, json, abort)
from flask_assets import Environment
from flask_babel import gettext
Expand Down Expand Up @@ -96,9 +96,8 @@ def default(self, obj: "Any") -> "Any":
@app.errorhandler(CSRFError) # type: ignore
def handle_csrf_error(e: CSRFError) -> "Response":
app.logger.error("The CSRF token is invalid.")
session.destroy()
msg = gettext('You have been logged out due to inactivity.')
flash(msg, 'error')
session.destroy(('error', msg), session.get('locale'))
return redirect(url_for('main.login'))

def _handle_http_exception(
Expand Down
10 changes: 5 additions & 5 deletions securedrop/journalist_app/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ def new_password() -> werkzeug.Response:
token = request.form.get('token')
error_message = gettext('Incorrect password or two-factor code.')
# If the user is validated, change their password
if validate_user(user.username, current_password, token, error_message):
password = request.form.get("password")
set_diceware_password(user, password)
session.destroy()
return redirect(url_for('main.login'))
if validate_user(user.username, current_password, token,
error_message):
password = request.form.get('password')
if set_diceware_password(user, password):
return redirect(url_for('main.login'))
return redirect(url_for('account.edit'))

@view.route("/2fa", methods=("GET", "POST"))
Expand Down
4 changes: 2 additions & 2 deletions securedrop/journalist_app/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import werkzeug
from flask import (Blueprint, render_template, request, url_for, redirect, g,
current_app, flash, abort, session)
current_app, flash, abort)
from flask_babel import gettext
from sqlalchemy.exc import IntegrityError
from sqlalchemy.orm.exc import NoResultFound
Expand All @@ -26,7 +26,7 @@
)
from flask_babel import gettext
from journalist_app.decorators import admin_required
from journalist_app.sessions import logout_user
from journalist_app.sessions import logout_user, session
from journalist_app.utils import (commit_account_changes, set_diceware_password,
validate_hotp_secret)
from journalist_app.forms import LogoForm, NewUserForm, SubmissionPreferencesForm, OrgNameForm
Expand Down
22 changes: 6 additions & 16 deletions securedrop/journalist_app/col.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
current_app,
escape,
flash,
g,
redirect,
render_template,
request,
Expand All @@ -21,20 +20,11 @@
)
from flask_babel import gettext
from journalist_app.forms import ReplyForm
from journalist_app.utils import (
col_delete,
col_delete_data,
col_download_all,
col_download_unread,
col_star,
col_un_star,
delete_collection,
get_source,
make_star_false,
make_star_true,
mark_seen,
)
from models import Reply, Submission
from journalist_app.sessions import session
from journalist_app.utils import (make_star_true, make_star_false, get_source,
delete_collection, col_download_unread,
col_download_all, col_star, col_un_star,
col_delete, col_delete_data, mark_seen)
from sdconfig import SDConfig
from sqlalchemy.orm.exc import NoResultFound
from store import Storage
Expand Down Expand Up @@ -151,7 +141,7 @@ def download_single_file(filesystem_id: str, fn: str) -> werkzeug.Response:

# mark as seen by the current user
try:
journalist = g.get("user")
journalist = session.get_user()
if fn.endswith("reply.gpg"):
reply = Reply.query.filter(Reply.filename == fn).one()
mark_seen([reply], journalist)
Expand Down
4 changes: 2 additions & 2 deletions securedrop/journalist_app/decorators.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# -*- coding: utf-8 -*-
from typing import Any

from flask import redirect, url_for, flash, session
from flask import redirect, url_for, flash
from flask_babel import gettext
from functools import wraps
from typing import Any, Callable
from journalist_app.sessions import session

from typing import Callable

Expand Down
45 changes: 29 additions & 16 deletions securedrop/journalist_app/sessions.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import typing
from datetime import datetime, timedelta, timezone
from secrets import token_urlsafe
from flask_babel import gettext
from flask import Flask, Request, Response
from flask import current_app as app
from typing import Optional, Any
from typing import Optional, Any, Dict, Tuple
from flask.sessions import SessionInterface as FlaskSessionInterface
from flask.sessions import SessionMixin, session_json_serializer
from json.decoder import JSONDecodeError
Expand All @@ -24,7 +25,7 @@ def on_update(self: ServerSideSession) -> None:
self.set_uid(initial['uid'])
self.set_user()
else:
self.uid = None
self.uid: Optional[int] = None
self.user = None
CallbackDict.__init__(self, initial, on_update)
self.sid = sid
Expand All @@ -34,6 +35,8 @@ def on_update(self: ServerSideSession) -> None:
self.to_destroy = False
self.to_regenerate = False
self.modified = False
self.flash: Optional[Tuple[str, str]] = None
self.locale: Optional[str] = None

def get_token(self) -> Optional[str]:
return self.token
Expand Down Expand Up @@ -65,7 +68,14 @@ def logged_in(self) -> bool:
else:
return False

def destroy(self) -> None:
def destroy(
self,
flash: Optional[Tuple[str, str]] = None,
locale: Optional[str] = None
) -> None:
# The parameters are needed to pass the information to the new session
self.locale = locale
self.flash = flash
self.uid = None
self.user = None
self.to_destroy = True
Expand Down Expand Up @@ -93,8 +103,6 @@ def _get_signer(self, app: Flask) -> URLSafeTimedSerializer:
:param header_name: if use_header, set the header name to parse
"""

session_class = ServerSideSession

def __init__(self, lifetime: int, renew_count: int, redis: Optional[Redis],
key_prefix: str, salt: str, header_name: str) -> None:
self.serializer = session_json_serializer
Expand All @@ -111,10 +119,13 @@ def __init__(self, lifetime: int, renew_count: int, redis: Optional[Redis],
self.new = False
self.has_same_site_capability = hasattr(self, "get_cookie_samesite")

def _new_session(self, is_api: bool = False) -> ServerSideSession:
def _new_session(self, is_api: bool = False, initial: Any = None) -> ServerSideSession:
sid = self._generate_sid()
token = self._get_signer(app).dumps(sid)
session = self.session_class(sid=sid, token=token, lifetime=self.lifetime) # type: ignore
token: str = self._get_signer(app).dumps(sid) # type: ignore
session = ServerSideSession(sid=sid,
token=token,
lifetime=self.lifetime,
initial=initial)
session.new = True
session.is_api = is_api
return session
Expand Down Expand Up @@ -150,15 +161,14 @@ def open_session(self, app: Flask, request: Request) -> Optional[ServerSideSessi
if val is not None:
try:
data = self.serializer.loads(val.decode())

return self.session_class(sid=sid,
token=self._get_signer(app).dumps(sid), # type: ignore
initial=data)
token: str = self._get_signer(app).dumps(sid) # type: ignore
return ServerSideSession(sid=sid, token=token, initial=data)
except (JSONDecodeError, NotImplementedError):
return self._new_session(is_api)
# signed session_id provided in cookie is valid, but the session is not on the server
# anymore so maybe here is the code path for a meaningful error message
return self._new_session(is_api)
msg = gettext("You have been logged out due to inactivity.")
return self._new_session(is_api, initial={"_flashes": [("error", msg)]})

def save_session( # type: ignore[override] # noqa
self,
Expand All @@ -172,11 +182,14 @@ def save_session( # type: ignore[override] # noqa
domain = self.get_cookie_domain(app)
path = self.get_cookie_path(app)
if session.to_destroy:
initial: Dict[str, Any] = {"locale": session.locale}
if session.flash:
initial["_flashes"] = [session.flash]
self.redis.delete(self.key_prefix + session.sid)
if not session.is_api:
response.delete_cookie(app.session_cookie_name,
domain=domain, path=path)
return
# Instead of deleting the cookie and send a new sid with the next request
# create the new session already, so we can pass along messages and locale
session = self._new_session(False, initial=initial)
expires = self.redis.ttl(name=self.key_prefix + session.sid)
if session.new:
session['renew_count'] = self.renew_count
Expand Down
28 changes: 16 additions & 12 deletions securedrop/journalist_app/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
import flask
import werkzeug
from flask import (flash, current_app, abort, send_file, redirect, url_for,
Markup, escape, session)
Markup, escape)
from flask_babel import gettext, ngettext
from journalist_app.sessions import session
from sqlalchemy.exc import IntegrityError

from db import db
Expand Down Expand Up @@ -466,19 +467,22 @@ def set_diceware_password(user: Journalist, password: Optional[str]) -> bool:
return False

# using Markup so the HTML isn't escaped
flash(
Markup(
"<p>{message} <span><code>{password}</code></span></p>".format(
message=Markup.escape(
gettext(
"Password updated. Don't forget to save it in your KeePassX database. "
"New password:"
)
),
password=Markup.escape("" if password is None else password),
session.destroy(
(
'success',
Markup(
"<p>{message} <span><code>{password}</code></span></p>".format(
message=Markup.escape(
gettext(
"Password updated. Don't forget to save it in your KeePassX database. "
"New password:"
)
),
password=Markup.escape("" if password is None else password)
)
)
),
"success",
session.get('locale')
)
return True

Expand Down
8 changes: 4 additions & 4 deletions securedrop/tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,16 @@
import mock

from bs4 import BeautifulSoup
from flask import escape, session
from flask import escape
from pyotp import HOTP, TOTP

import journalist_app as journalist_app_module
import mock
from bs4 import BeautifulSoup
from db import db
from encryption import EncryptionManager
from flask import escape, g, session
from pyotp import HOTP, TOTP
from store import Storage
from journalist_app.sessions import session
from source_app.session_manager import SessionManager
from store import Storage

Expand All @@ -44,7 +44,7 @@ def _login_user(app, user_dict):
follow_redirects=True,
)
assert resp.status_code == 200
assert user_dict['username'] in resp.data.decode('utf-8')
assert session.get_user() is not None


def test_submit_message(journalist_app, source_app, test_journo, app_storage):
Expand Down
14 changes: 12 additions & 2 deletions securedrop/tests/test_journalist.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,18 @@
from db import db
from encryption import EncryptionManager, GpgKeyNotFoundError
from flaky import flaky
from flask import current_app, escape, g, session, url_for
from flask import current_app, escape, g, url_for
from flask_babel import gettext, ngettext
from mock import patch
from pyotp import TOTP
from sqlalchemy.exc import IntegrityError
from sqlalchemy.orm.exc import StaleDataError
from sqlalchemy.sql.expression import func
from html import escape as htmlescape

import journalist_app as journalist_app_module
from encryption import EncryptionManager, GpgKeyNotFoundError
from journalist_app.sessions import session
from journalist_app.utils import mark_seen
from mock import call, patch
from models import (
Expand Down Expand Up @@ -68,7 +78,7 @@ def _login_user(app, username, password, otp_secret, success=True):
follow_redirects=True,
)
assert resp.status_code == 200
assert ((username in resp.data.decode('utf-8')) == success)
assert ((session.get_user() is not None) == success)


@pytest.mark.parametrize("otp_secret", ["", "GA", "GARBAGE", "JHCOGO7VCER3EJ4"])
Expand Down

0 comments on commit a47b9bd

Please sign in to comment.