Make authentication logic simpler and safer (#270)

* Fixed exposed password in session

The project password was set in clear text
in the session cookie. The cookie payload is
only base64 encoded so it must not be used to
store private information. The password is
simply replaced by a boolean.

* Simplify authentication logic
This commit is contained in:
0livd 2017-10-23 23:03:44 +02:00 committed by Alexis Metaireau
parent 74e9af59e6
commit 293735eca7
3 changed files with 29 additions and 37 deletions

View file

@ -17,6 +17,7 @@ Changed
======= =======
- Logged admin can see any project (#262) - Logged admin can see any project (#262)
- Simpler and safer authentication logic (#270)
- Better install doc (#275) - Better install doc (#275)
Added Added

View file

@ -181,7 +181,7 @@ class BudgetTestCase(IhatemoneyTestCase):
}) })
# session is updated # session is updated
self.assertEqual(session['raclette'], 'party') self.assertTrue(session['raclette'])
# project is created # project is created
self.assertEqual(len(models.Project.query.all()), 1) self.assertEqual(len(models.Project.query.all()), 1)
@ -373,7 +373,7 @@ class BudgetTestCase(IhatemoneyTestCase):
self.assertNotIn("Authentication", resp.data.decode('utf-8')) self.assertNotIn("Authentication", resp.data.decode('utf-8'))
self.assertIn('raclette', session) self.assertIn('raclette', session)
self.assertEqual(session['raclette'], 'raclette') self.assertTrue(session['raclette'])
# logout should wipe the session out # logout should wipe the session out
c.get("/exit") c.get("/exit")

View file

@ -105,7 +105,7 @@ def pull_project(endpoint, values):
project_id=project_id)) project_id=project_id))
is_admin = session.get('is_admin') is_admin = session.get('is_admin')
if (project.id in session and session[project.id] == project.password) or is_admin: if session.get(project.id) or is_admin:
# add project into kwargs and call the original function # add project into kwargs and call the original function
g.project = project g.project = project
else: else:
@ -159,43 +159,34 @@ def authenticate(project_id=None):
msg = _("You need to enter a project identifier") msg = _("You need to enter a project identifier")
form.errors["id"] = [msg] form.errors["id"] = [msg]
return render_template("authenticate.html", form=form) return render_template("authenticate.html", form=form)
else:
project = Project.query.get(project_id)
create_project = False # We don't want to create the project by default project = Project.query.get(project_id)
if not project: if not project:
# But if the user try to connect to an unexisting project, we will # If the user try to connect to an unexisting project, we will
# propose him a link to the creation form. # propose him a link to the creation form.
if request.method == "POST": return render_template("authenticate.html", form=form, create_project=project_id)
form.validate()
else:
create_project = project_id
else: # if credentials are already in session, redirect
# if credentials are already in session, redirect if session.get(project_id):
if project_id in session and project.password == session[project_id]: setattr(g, 'project', project)
setattr(g, 'project', project) return redirect(url_for(".list_bills"))
return redirect(url_for(".list_bills"))
# else process the form if request.method == "POST" and form.validate():
if request.method == "POST": if not form.password.data == project.password:
if form.validate(): msg = _("This private code is not the right one")
if not form.password.data == project.password: form.errors['password'] = [msg]
msg = _("This private code is not the right one") return render_template("authenticate.html", form=form)
form.errors['password'] = [msg] # maintain a list of visited projects
else: if "projects" not in session:
# maintain a list of visited projects session["projects"] = []
if "projects" not in session: # add the project on the top of the list
session["projects"] = [] session["projects"].insert(0, (project_id, project.name))
# add the project on the top of the list session[project_id] = True
session["projects"].insert(0, (project_id, project.name)) session.update()
session[project_id] = form.password.data setattr(g, 'project', project)
session.update() return redirect(url_for(".list_bills"))
setattr(g, 'project', project)
return redirect(url_for(".list_bills"))
return render_template("authenticate.html", form=form, return render_template("authenticate.html", form=form)
create_project=create_project)
@main.route("/") @main.route("/")
@ -233,7 +224,7 @@ def create_project():
db.session.commit() db.session.commit()
# create the session object (authenticate) # create the session object (authenticate)
session[project.id] = project.password session[project.id] = True
session.update() session.update()
# send reminder email # send reminder email
@ -290,8 +281,8 @@ def edit_project():
if request.method == "POST": if request.method == "POST":
if edit_form.validate(): if edit_form.validate():
project = edit_form.update(g.project) project = edit_form.update(g.project)
db.session.add(project)
db.session.commit() db.session.commit()
session[project.id] = project.password
return redirect(url_for(".list_bills")) return redirect(url_for(".list_bills"))
@ -359,7 +350,7 @@ def demo():
contact_email="demo@notmyidea.org") contact_email="demo@notmyidea.org")
db.session.add(project) db.session.add(project)
db.session.commit() db.session.commit()
session[project.id] = project.password session[project.id] = True
return redirect(url_for(".list_bills", project_id=project.id)) return redirect(url_for(".list_bills", project_id=project.id))