diff --git a/tests/migrations/test_fuzzy.py b/tests/migrations/test_fuzzy.py index 226e75ee6d31dcbeba220975937355c26ddc7f34..310889b99dc9c2c88b4a2a01b4d2e880d145ebfe 100644 --- a/tests/migrations/test_fuzzy.py +++ b/tests/migrations/test_fuzzy.py @@ -13,6 +13,7 @@ from uffd.models import ( Service, OAuth2Client, OAuth2LogoutURI, OAuth2Grant, OAuth2Token, OAuth2DeviceLoginInitiation, PasswordToken, + Session, ) from tests.utils import MigrationTestCase @@ -62,7 +63,15 @@ class TestFuzzy(MigrationTestCase): db.session.add_all([service, oauth2_client]) db.session.add(OAuth2Grant(user=user, client=oauth2_client, _code='testcode', redirect_uri='http://example.com/callback', expires=datetime.datetime.now())) db.session.add(OAuth2Token(user=user, client=oauth2_client, token_type='Bearer', _access_token='testcode', _refresh_token='testcode', expires=datetime.datetime.now())) - db.session.add(OAuth2DeviceLoginInitiation(client=oauth2_client, confirmations=[DeviceLoginConfirmation(user=user)])) + session = Session( + user=user, + secret='0919de9da3f7dc6c33ab849f44c20e8221b673ca701030de17488f3269fc5469f100e2ce56e5fd71305b23d8ecbb06d80d22004adcd3fefc5f5fcb80a436e31f2c2d9cc8fe8c59ae44871ae4524408d312474570280bf29d3ba145a4bd00010ca758eaa0795b180ec12978b42d13bf4c4f06f72103d44077022ce656610be855', + ip_address='127.0.0.1', + user_agent='Mozilla/5.0 (X11; Linux x86_64; rv:124.0) Gecko/20100101 Firefox/124.0', + mfa_done=True, + ) + db.session.add(session) + db.session.add(OAuth2DeviceLoginInitiation(client=oauth2_client, confirmations=[DeviceLoginConfirmation(session=session)])) db.session.add(PasswordToken(user=user)) db.session.commit() revs = [s.split('_', 1)[0] for s in os.listdir('uffd/migrations/versions') if '_' in s and s.endswith('.py')] diff --git a/tests/views/test_oauth2.py b/tests/views/test_oauth2.py index 2451af51519eb23b17ac0d2ab8225523ad7ae0d9..8efd0a84fea789cf069665158ce0c875ecc6b274 100644 --- a/tests/views/test_oauth2.py +++ b/tests/views/test_oauth2.py @@ -7,7 +7,7 @@ from flask import url_for, session from uffd.database import db from uffd.password_hash import PlaintextPasswordHash from uffd.remailer import remailer -from uffd.models import DeviceLoginConfirmation, Service, OAuth2Client, OAuth2DeviceLoginInitiation, RemailerMode, OAuth2Key +from uffd.models import DeviceLoginConfirmation, Service, OAuth2Client, OAuth2DeviceLoginInitiation, RemailerMode, OAuth2Key, Session from tests.utils import dump, UffdTestCase from tests.models.test_oauth2 import TEST_JWK @@ -143,7 +143,7 @@ class TestViews(UffdTestCase): with self.client.session_transaction() as _session: initiation = OAuth2DeviceLoginInitiation(client=OAuth2Client.query.filter_by(client_id='test').one()) db.session.add(initiation) - confirmation = DeviceLoginConfirmation(initiation=initiation, user=self.get_user()) + confirmation = DeviceLoginConfirmation(initiation=initiation, session=Session(user=self.get_user())) db.session.add(confirmation) db.session.commit() _session['devicelogin_id'] = initiation.id @@ -158,7 +158,7 @@ class TestViews(UffdTestCase): with self.client.session_transaction() as _session: initiation = OAuth2DeviceLoginInitiation(client=OAuth2Client.query.filter_by(client_id='test').one()) db.session.add(initiation) - confirmation = DeviceLoginConfirmation(initiation=initiation, user=self.get_user()) + confirmation = DeviceLoginConfirmation(initiation=initiation, session=Session(user=self.get_user())) db.session.add(confirmation) db.session.commit() _session['devicelogin_id'] = initiation.id diff --git a/tests/views/test_session.py b/tests/views/test_session.py index d2bf2b7c2d69823ce91f9384e8028495a399d059..3debfb23c02ac2ab389060982807cadc43b8b684 100644 --- a/tests/views/test_session.py +++ b/tests/views/test_session.py @@ -183,7 +183,7 @@ class TestSession(UffdTestCase): self.assertEqual(r.status_code, 200) initiation = OAuth2DeviceLoginInitiation.query.filter_by(code=code).one() self.assertEqual(len(initiation.confirmations), 1) - self.assertEqual(initiation.confirmations[0].user.loginname, 'testuser') + self.assertEqual(initiation.confirmations[0].session.user.loginname, 'testuser') self.assertIn(initiation.confirmations[0].code.encode(), r.data) r = self.client.get(path=url_for('session.deviceauth_finish'), follow_redirects=True) self.assertEqual(r.status_code, 200) diff --git a/uffd/migrations/versions/99df71f0f4a0_migrate_device_login_from_user_to_session.py b/uffd/migrations/versions/99df71f0f4a0_migrate_device_login_from_user_to_session.py new file mode 100644 index 0000000000000000000000000000000000000000..32655553e045368101418d3643cbe4623cb12d06 --- /dev/null +++ b/uffd/migrations/versions/99df71f0f4a0_migrate_device_login_from_user_to_session.py @@ -0,0 +1,63 @@ +"""Migrate device login from user to session + +Revision ID: 99df71f0f4a0 +Revises: 87cb93a329bf +Create Date: 2024-05-18 16:41:33.923207 + +""" +from alembic import op +import sqlalchemy as sa + +revision = '99df71f0f4a0' +down_revision = '87cb93a329bf' +branch_labels = None +depends_on = None + +def upgrade(): + op.drop_table('device_login_confirmation') + op.create_table('device_login_confirmation', + sa.Column('id', sa.Integer(), autoincrement=True, nullable=False), + sa.Column('initiation_id', sa.Integer(), nullable=False), + sa.Column('session_id', sa.Integer(), nullable=False), + sa.Column('code0', sa.String(length=32), nullable=False), + sa.Column('code1', sa.String(length=32), nullable=False), + sa.ForeignKeyConstraint(['initiation_id'], ['device_login_initiation.id'], name='fk_device_login_confirmation_initiation_id_', onupdate='CASCADE', ondelete='CASCADE'), + sa.ForeignKeyConstraint(['session_id'], ['session.id'], name=op.f('fk_device_login_confirmation_session_id_session'), onupdate='CASCADE', ondelete='CASCADE'), + sa.PrimaryKeyConstraint('id', name=op.f('pk_device_login_confirmation')), + sa.UniqueConstraint('initiation_id', 'code0', name='uq_device_login_confirmation_initiation_id_code0'), + sa.UniqueConstraint('initiation_id', 'code1', name='uq_device_login_confirmation_initiation_id_code1'), + sa.UniqueConstraint('session_id', name=op.f('uq_device_login_confirmation_session_id')) + ) + +def downgrade(): + # We don't drop and recreate the table here to improve fuzzy migration test coverage + with op.batch_alter_table('device_login_confirmation', schema=None) as batch_op: + batch_op.add_column(sa.Column('user_id', sa.Integer(), nullable=True)) + meta = sa.MetaData(bind=op.get_bind()) + device_login_confirmation = sa.Table('device_login_confirmation', meta, + sa.Column('id', sa.Integer(), autoincrement=True, nullable=False), + sa.Column('initiation_id', sa.Integer(), nullable=False), + sa.Column('session_id', sa.Integer(), nullable=False), + sa.Column('user_id', sa.Integer(), nullable=True), + sa.Column('code0', sa.String(length=32), nullable=False), + sa.Column('code1', sa.String(length=32), nullable=False), + sa.ForeignKeyConstraint(['initiation_id'], ['device_login_initiation.id'], name='fk_device_login_confirmation_initiation_id_', onupdate='CASCADE', ondelete='CASCADE'), + sa.ForeignKeyConstraint(['session_id'], ['session.id'], name=op.f('fk_device_login_confirmation_session_id_session'), onupdate='CASCADE', ondelete='CASCADE'), + sa.PrimaryKeyConstraint('id', name=op.f('pk_device_login_confirmation')), + sa.UniqueConstraint('initiation_id', 'code0', name='uq_device_login_confirmation_initiation_id_code0'), + sa.UniqueConstraint('initiation_id', 'code1', name='uq_device_login_confirmation_initiation_id_code1'), + sa.UniqueConstraint('session_id', name=op.f('uq_device_login_confirmation_session_id')) + ) + session = sa.table('session', + sa.column('id', sa.Integer), + sa.column('user_id', sa.Integer()), + ) + op.execute(device_login_confirmation.update().values(user_id=sa.select([session.c.user_id]).where(device_login_confirmation.c.session_id==session.c.id).as_scalar())) + op.execute(device_login_confirmation.delete().where(device_login_confirmation.c.user_id==None)) + with op.batch_alter_table('device_login_confirmation', copy_from=device_login_confirmation) as batch_op: + batch_op.alter_column('user_id', nullable=False, existing_type=sa.Integer()) + batch_op.create_foreign_key('fk_device_login_confirmation_user_id_user', 'user', ['user_id'], ['id'], onupdate='CASCADE', ondelete='CASCADE') + batch_op.create_unique_constraint('uq_device_login_confirmation_user_id', ['user_id']) + batch_op.drop_constraint(batch_op.f('fk_device_login_confirmation_session_id_session'), type_='foreignkey') + batch_op.drop_constraint(batch_op.f('uq_device_login_confirmation_session_id'), type_='unique') + batch_op.drop_column('session_id') diff --git a/uffd/models/session.py b/uffd/models/session.py index 842d4ec281b085a61031182e3be8c68c843b8abb..b29ba3e245153a530961e1e3433ed5938318f990 100644 --- a/uffd/models/session.py +++ b/uffd/models/session.py @@ -185,7 +185,7 @@ class DeviceLoginConfirmation(db.Model): A confirmation code is generated and displayed when an authenticated user enters an initiation code and confirms the device login attempt. Every - instance is bound to both an initiation code and a user. + instance is bound to both an initiation code and a login session. The code attribute is formed out of two indepentently unique parts to ensure that at any time all existing codes differ in at least two @@ -198,8 +198,8 @@ class DeviceLoginConfirmation(db.Model): name='fk_device_login_confirmation_initiation_id_', onupdate='CASCADE', ondelete='CASCADE'), nullable=False) initiation = relationship('DeviceLoginInitiation', back_populates='confirmations') - user_id = Column(Integer(), ForeignKey('user.id', onupdate='CASCADE', ondelete='CASCADE'), nullable=False, unique=True) - user = relationship('User') + session_id = Column(Integer(), ForeignKey('session.id', onupdate='CASCADE', ondelete='CASCADE'), nullable=False, unique=True) + session = relationship('Session') code0 = Column(String(32), nullable=False, default=lambda: token_typeable(1)) code1 = Column(String(32), nullable=False, default=lambda: token_typeable(1)) diff --git a/uffd/views/oauth2.py b/uffd/views/oauth2.py index f46305a12ba92eec0abd2a6b27d21ad34395ab1a..f9a0b7b802cb07ad34af4b56635e49a85ad36392 100644 --- a/uffd/views/oauth2.py +++ b/uffd/views/oauth2.py @@ -291,14 +291,14 @@ def authorize_user(client): del session['devicelogin_id'] del session['devicelogin_secret'] del session['devicelogin_confirmation'] - if not initiation or initiation.expired or not confirmation or confirmation.user.is_deactivated: + if not initiation or initiation.expired or not confirmation or confirmation.session.user.is_deactivated: raise LoginRequiredError( flash_message=_('Device login failed'), response=redirect(url_for('session.login', ref=request.full_path, devicelogin=True)) ) db.session.delete(initiation) db.session.commit() - return confirmation.user + return confirmation.session.user raise LoginRequiredError( flash_message=_('You need to login to access this service'), diff --git a/uffd/views/session.py b/uffd/views/session.py index 24ff1ba30baba8d248f82fb7697478d64ef89644..719b6b14700c721ab476d4005c8473c63513512e 100644 --- a/uffd/views/session.py +++ b/uffd/views/session.py @@ -261,12 +261,12 @@ def deviceauth(): @login_required() @csrf_protect(blueprint=bp) def deviceauth_submit(): - DeviceLoginConfirmation.query.filter_by(user=request.user).delete() + DeviceLoginConfirmation.query.filter_by(session=request.session).delete() initiation = DeviceLoginInitiation.query.filter_by(code=request.form['initiation-code']).one_or_none() if initiation is None or initiation.expired: flash(_('Invalid initiation code')) return redirect(url_for('session.deviceauth')) - confirmation = DeviceLoginConfirmation(user=request.user, initiation=initiation) + confirmation = DeviceLoginConfirmation(session=request.session, initiation=initiation) db.session.add(confirmation) db.session.commit() return render_template('session/deviceauth.html', initiation=initiation, confirmation=confirmation) @@ -274,6 +274,6 @@ def deviceauth_submit(): @bp.route("/device/finish", methods=['GET', 'POST']) @login_required() def deviceauth_finish(): - DeviceLoginConfirmation.query.filter_by(user=request.user).delete() + DeviceLoginConfirmation.query.filter_by(session=request.session).delete() db.session.commit() return redirect(url_for('index'))