From 6948ddca49aa70a1977d31a5a902f0cf491f7ad6 Mon Sep 17 00:00:00 2001
From: Julian Rother <julian@cccv.de>
Date: Fri, 25 Feb 2022 15:06:11 +0100
Subject: [PATCH] Fix "Migrate OAuth2 and API clients to database" (again)
The original change completely broke single logout support.
The migration now uses the correct hashing algorithm (unsalted SHA512 instead
of salted SHA512) for OAuth2/API secrets/passwords.
---
...9d3f7dac9db_move_api_and_oauth2_clients_to_db.py | 13 +++++--------
uffd/oauth2/models.py | 5 +++++
uffd/oauth2/templates/oauth2/logout.html | 9 +++------
uffd/oauth2/views.py | 2 +-
4 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/uffd/migrations/versions/b9d3f7dac9db_move_api_and_oauth2_clients_to_db.py b/uffd/migrations/versions/b9d3f7dac9db_move_api_and_oauth2_clients_to_db.py
index 5cfaf114..34c1a4fd 100644
--- a/uffd/migrations/versions/b9d3f7dac9db_move_api_and_oauth2_clients_to_db.py
+++ b/uffd/migrations/versions/b9d3f7dac9db_move_api_and_oauth2_clients_to_db.py
@@ -19,12 +19,9 @@ down_revision = '09d2edcaf0cc'
branch_labels = None
depends_on = None
-def hash_ssha512(password):
- salt = secrets.token_bytes(8)
- ctx = hashlib.new('sha512')
- ctx.update(password.encode())
- ctx.update(salt)
- return '{ssha512}' + base64.b64encode(ctx.digest() + salt).decode()
+def hash_sha512(password):
+ ctx = hashlib.new('sha512', password.encode())
+ return '{sha512}' + base64.b64encode(ctx.digest()).decode()
def upgrade():
used_service_names = set()
@@ -129,7 +126,7 @@ def upgrade():
sa.UniqueConstraint('auth_username', name=op.f('uq_api_client_auth_username'))
)
for service_name, auth_username, auth_password, perm_users, perm_checkpassword, perm_mail_aliases in api_clients:
- op.execute(api_client_table.insert().values(service_id=sa.select([service_table.c.id]).where(service_table.c.name==service_name).as_scalar(), auth_username=auth_username, auth_password=hash_ssha512(auth_password), perm_users=perm_users, perm_checkpassword=perm_checkpassword, perm_mail_aliases=perm_mail_aliases))
+ op.execute(api_client_table.insert().values(service_id=sa.select([service_table.c.id]).where(service_table.c.name==service_name).as_scalar(), auth_username=auth_username, auth_password=hash_sha512(auth_password), perm_users=perm_users, perm_checkpassword=perm_checkpassword, perm_mail_aliases=perm_mail_aliases))
oauth2client_table = op.create_table('oauth2client',
sa.Column('db_id', sa.Integer(), autoincrement=True, nullable=False),
@@ -156,7 +153,7 @@ def upgrade():
sa.PrimaryKeyConstraint('id', name=op.f('pk_oauth2redirect_uri'))
)
for service_name, client_id, client_secret, redirect_uris, logout_uris in oauth2_clients:
- op.execute(oauth2client_table.insert().values(service_id=sa.select([service_table.c.id]).where(service_table.c.name==service_name).as_scalar(), client_id=client_id, client_secret=hash_ssha512(client_secret)))
+ op.execute(oauth2client_table.insert().values(service_id=sa.select([service_table.c.id]).where(service_table.c.name==service_name).as_scalar(), client_id=client_id, client_secret=hash_sha512(client_secret)))
for method, uri, in logout_uris:
op.execute(oauth2logout_uri_table.insert().values(client_db_id=sa.select([oauth2client_table.c.db_id]).where(oauth2client_table.c.client_id==client_id).as_scalar(), method=method, uri=uri))
for uri in redirect_uris:
diff --git a/uffd/oauth2/models.py b/uffd/oauth2/models.py
index 4621f114..6ed91097 100644
--- a/uffd/oauth2/models.py
+++ b/uffd/oauth2/models.py
@@ -1,4 +1,5 @@
import datetime
+import json
from sqlalchemy import Column, Integer, String, DateTime, Text, ForeignKey
from sqlalchemy.orm import relationship
@@ -41,6 +42,10 @@ class OAuth2Client(db.Model):
def access_allowed(self, user):
return self.service.has_access(user)
+ @property
+ def logout_uris_json(self):
+ return json.dumps([[item.method, item.uri] for item in self.logout_uris])
+
class OAuth2RedirectURI(db.Model):
__tablename__ = 'oauth2redirect_uri'
id = Column(Integer, primary_key=True, autoincrement=True)
diff --git a/uffd/oauth2/templates/oauth2/logout.html b/uffd/oauth2/templates/oauth2/logout.html
index e45a9268..02363882 100644
--- a/uffd/oauth2/templates/oauth2/logout.html
+++ b/uffd/oauth2/templates/oauth2/logout.html
@@ -11,9 +11,9 @@
</noscript>
<p>{{_('While you successfully logged out of the Single-Sign-On service, you may still be logged in on these services:')}}</p>
<ul>
- {% for client in clients if client.logout_urls %}
- <li class="client" data-urls='{{ client.logout_urls|tojson }}'>
- {{ client.client_id }}
+ {% for client in clients if client.logout_uris %}
+ <li class="client" data-urls='{{ client.logout_uris_json }}'>
+ {{ client.service.name }}
<span class="status-active spinner-border spinner-border-sm d-none" role="status" aria-hidden="true"></span>
<i class="status-success fas fa-check d-none"></i>
<i class="status-failed fas fa-exclamation d-none"></i>
@@ -53,7 +53,6 @@ function logout_services() {
});
});
p = p.then(function () {
- console.log('done', elem);
elem.find('.status-active').addClass('d-none');
elem.find('.status-success').removeClass('d-none');
elem.removeClass('client');
@@ -61,13 +60,11 @@ function logout_services() {
.catch(function (err) {
elem.find('.status-active').addClass('d-none');
elem.find('.status-failed').removeClass('d-none');
- console.log(err);
throw err;
});
all_promises.push(p);
});
Promise.allSettled(all_promises).then(function (results) {
- console.log(results);
for (result of results) {
if (result.status == 'rejected')
throw result.reason;
diff --git a/uffd/oauth2/views.py b/uffd/oauth2/views.py
index cfb089b3..d13fd42d 100644
--- a/uffd/oauth2/views.py
+++ b/uffd/oauth2/views.py
@@ -249,5 +249,5 @@ def logout():
if not request.values.get('client_ids'):
return secure_local_redirect(request.values.get('ref', '/'))
client_ids = request.values['client_ids'].split(',')
- clients = [OAuth2Client.query.filter_by(name=client_id).one() for client_id in client_ids]
+ clients = [OAuth2Client.query.filter_by(client_id=client_id).one() for client_id in client_ids]
return render_template('oauth2/logout.html', clients=clients)
--
GitLab