diff --git a/README.md b/README.md index 6b422ce62..5ceb780f4 100644 --- a/README.md +++ b/README.md @@ -66,7 +66,7 @@ In another terminal (`docker-compose up` must be running) : ./scripts/test.sh c2corg_api/tests/models/test_book.py::TestBook::test_to_archive ``` -Note: if you're using MinGW on Windows, be sure to prefix the command with `MSYS_PATH_NOCONV=1` +Note: if you're using MinGW on Windows, be sure to prefix the command with `MSYS_NO_PATHCONV=1` ## Useful links in [wiki](https://github.com/c2corg/v6_api/wiki) diff --git a/alembic_migration/README.md b/alembic_migration/README.md index 9ce9d8eea..4204097a0 100644 --- a/alembic_migration/README.md +++ b/alembic_migration/README.md @@ -9,7 +9,7 @@ model. Create the migration script with: -``` +```bash docker-compose exec api .build/venv/bin/alembic revision -m 'Add column x' ``` @@ -26,6 +26,6 @@ is used. A migration should be run each time the application code is updated or if you have just created a migration script. -``` +```bash docker-compose exec api .build/venv/bin/alembic upgrade head ``` diff --git a/alembic_migration/versions/1d851410e3af_add_column_for_terms_of_service.py b/alembic_migration/versions/1d851410e3af_add_column_for_terms_of_service.py new file mode 100644 index 000000000..3b493e8ee --- /dev/null +++ b/alembic_migration/versions/1d851410e3af_add_column_for_terms_of_service.py @@ -0,0 +1,27 @@ +"""Add column for terms of service + +Revision ID: 1d851410e3af +Revises: 305b064bdf66 +Create Date: 2023-03-03 17:29:38.587079 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '1d851410e3af' +down_revision = '305b064bdf66' +branch_labels = None +depends_on = None + +def upgrade(): + op.add_column( + 'user', + sa.Column('tos_validated', sa.DateTime(timezone=True), nullable=True), + schema='users' + ) + + +def downgrade(): + op.drop_column('user', 'tos_validated', schema='users') diff --git a/c2corg_api/models/user.py b/c2corg_api/models/user.py index 342d18204..0b9917043 100644 --- a/c2corg_api/models/user.py +++ b/c2corg_api/models/user.py @@ -91,6 +91,8 @@ class User(Base): DateTime(timezone=True), default=func.now(), onupdate=func.now(), nullable=False, index=True) blocked = Column(Boolean, nullable=False, default=False) + tos_validated = Column( + DateTime(timezone=True), nullable=True, unique=False) lang = Column( String(2), ForeignKey(schema + '.langs.lang'), diff --git a/c2corg_api/tests/__init__.py b/c2corg_api/tests/__init__.py index ca56146da..cfb28ea32 100644 --- a/c2corg_api/tests/__init__.py +++ b/c2corg_api/tests/__init__.py @@ -83,7 +83,8 @@ def _add_global_test_data(session): name='Contributor', username='contributor', email='contributor@camptocamp.org', forum_username='contributor', password='super pass', - email_validated=True, profile=contributor_profile) + tos_validated=datetime.datetime(2020, 12, 28), email_validated=True, + profile=contributor_profile) contributor2_profile = UserProfile( categories=['amateur'], @@ -94,6 +95,7 @@ def _add_global_test_data(session): username='contributor2', email='contributor2@camptocamp.org', forum_username='contributor2', password='better pass', email_validated=True, + tos_validated=datetime.datetime(2021, 2, 12), profile=contributor2_profile) contributor3_profile = UserProfile( @@ -105,8 +107,20 @@ def _add_global_test_data(session): username='contributor3', email='contributor3@camptocamp.org', forum_username='contributor3', password='poor pass', email_validated=True, + tos_validated=datetime.datetime(2006, 1, 1), profile=contributor3_profile) + contributor_notos_profile = UserProfile( + categories=['amateur'], + locales=[DocumentLocale(title='...', lang='en')]) + + contributor_notos = User( + name='Contributor no ToS', + username='contributornotos', email='contributornotos@camptocamp.org', + forum_username='contributornotos', + password='some pass', email_validated=True, + profile=contributor_notos_profile) + moderator_profile = UserProfile( categories=['mountain_guide'], locales=[DocumentLocale(title='', lang='en')]) @@ -116,6 +130,7 @@ def _add_global_test_data(session): username='moderator', email='moderator@camptocamp.org', forum_username='moderator', moderator=True, password='even better pass', + tos_validated=datetime.datetime(2021, 2, 12), email_validated=True, profile=moderator_profile) robot_profile = UserProfile( @@ -126,9 +141,11 @@ def _add_global_test_data(session): username='robot', email='robot@camptocamp.org', forum_username='robot', robot=True, password='bombproof pass', + tos_validated=datetime.datetime(2021, 6, 6), email_validated=True, profile=robot_profile) - users = [robot, moderator, contributor, contributor2, contributor3] + users = [robot, moderator, contributor, contributor2, + contributor3, contributor_notos] session.add_all(users) session.flush() @@ -155,7 +172,7 @@ def _add_global_test_data(session): now = datetime.datetime.utcnow() exp = now + datetime.timedelta(weeks=10) - for user in [robot, moderator, contributor, contributor2, contributor3]: + for user in users: claims = create_claims(user, exp) token = jwt.encode(claims, key=key, algorithm=algorithm). \ decode('utf-8') diff --git a/c2corg_api/tests/views/test_user.py b/c2corg_api/tests/views/test_user.py index 77d4861d4..7fe4d2950 100644 --- a/c2corg_api/tests/views/test_user.py +++ b/c2corg_api/tests/views/test_user.py @@ -14,6 +14,8 @@ from urllib.parse import urlparse import re +import time +import datetime from unittest.mock import Mock, MagicMock, patch @@ -116,6 +118,7 @@ def test_always_register_non_validated_users(self, _send_email): user_id = body.get('id') user = self.session.query(User).get(user_id) self.assertFalse(user.email_validated) + self.assertIsNotNone(user.tos_validated) _send_email.check_call_once() @patch('c2corg_api.emails.email_service.EmailService._send_email') @@ -506,7 +509,7 @@ def test_purge_tokens(self): self.assertEqual(0, query.count()) def login(self, username, password=None, status=200, sso=None, sig=None, - discourse=None): + discourse=None, accept_tos=None): if not password: password = self.global_passwords[username] @@ -521,6 +524,8 @@ def login(self, username, password=None, status=200, sso=None, sig=None, request_body['sig'] = sig if discourse: request_body['discourse'] = discourse + if accept_tos: + request_body['accept_tos'] = accept_tos url = '/users/login' response = self.app_post_json(url, request_body, status=status) @@ -572,8 +577,20 @@ def test_login_failure(self): body = self.login('moderator', password='invalid', status=403).json self.assertEqual(body['status'], 'error') + def test_login_no_tos_failure(self): + body = self.login('contributornotos', password='some pass', status=403).json + self.assertErrorsContain(body, 'Forbidden', 'TOS not accepted') + + def test_login_no_tos_success(self): + # A user which did not previously accepted ToS can login + # if he accepts them. It is stored in the db. + body = self.login('contributornotos', password='some pass', accept_tos=True, status=200).json + self.assertTrue('token' in body) + user = self.session.query(User).filter( + User.username == 'contributornotos').one() + self.assertTrue((datetime.datetime.now(datetime.timezone.utc) - user.tos_validated).total_seconds() < 5) + def assertExpireAlmostEqual(self, expire, days, seconds_delta): # noqa - import time now = int(round(time.time())) expected = days * 24 * 3600 + now # 14 days from now if (abs(expected - expire) > seconds_delta): diff --git a/c2corg_api/tests/views/test_user_profile.py b/c2corg_api/tests/views/test_user_profile.py index ef723f5a8..1e0397353 100644 --- a/c2corg_api/tests/views/test_user_profile.py +++ b/c2corg_api/tests/views/test_user_profile.py @@ -41,21 +41,21 @@ def test_get_collection_paginated(self): self.assertResultsEqual( self.get_collection( {'offset': 0, 'limit': 0}, user='contributor'), - [], 7) + [], 8) self.assertResultsEqual( self.get_collection( {'offset': 0, 'limit': 1}, user='contributor'), - [self.profile4.document_id], 7) + [self.profile4.document_id], 8) self.assertResultsEqual( self.get_collection( {'offset': 0, 'limit': 2}, user='contributor'), - [self.profile4.document_id, self.profile2.document_id], 7) + [self.profile4.document_id, self.profile2.document_id], 8) self.assertResultsEqual( self.get_collection( {'offset': 1, 'limit': 3}, user='contributor'), - [self.profile2.document_id, self.global_userids['contributor3'], - self.global_userids['contributor2']], 7) + [self.profile2.document_id, self.global_userids['contributornotos'], + self.global_userids['contributor3']], 8) def test_get_collection_lang(self): self.get_collection_lang(user='contributor') @@ -65,10 +65,11 @@ def test_get_collection_search(self): self.assertResultsEqual( self.get_collection_search({'l': 'en'}, user='contributor'), - [self.profile4.document_id, self.global_userids['contributor3'], - self.global_userids['contributor2'], self.profile1.document_id, - self.global_userids['moderator'], self.global_userids['robot']], - 6) + [self.profile4.document_id, self.global_userids['contributornotos'], + self.global_userids['contributor3'], self.global_userids['contributor2'], + self.profile1.document_id, self.global_userids['moderator'], + self.global_userids['robot']], + 7) def test_get_unauthenticated_private_profile(self): """Tests that only the user name is returned when requesting a private diff --git a/c2corg_api/views/user.py b/c2corg_api/views/user.py index 9f5c9c303..b733a9e12 100644 --- a/c2corg_api/views/user.py +++ b/c2corg_api/views/user.py @@ -217,13 +217,16 @@ def post(self): Purpose.registration, VALIDATION_EXPIRE_DAYS) - # directly create the user profile, the document id of the profile + # Directly create the user profile, the document id of the profile # is the user id lang = user.lang user.profile = UserProfile( categories=['amateur'], locales=[DocumentLocale(lang=lang, title='')] ) + # Checkbox is mandatory on the frontend when registering + # so we can store ToS acceptance. + user.tos_validated = datetime.datetime.utcnow() DBSession.add(user) try: @@ -453,6 +456,7 @@ def post(self): class LoginSchema(colander.MappingSchema): username = colander.SchemaNode(colander.String()) password = colander.SchemaNode(colander.String()) + accept_tos = colander.SchemaNode(colander.Boolean(), missing=False) login_schema = LoginSchema() @@ -486,8 +490,10 @@ def post(self): request = self.request username = request.validated['username'] password = request.validated['password'] + accept_tos = request.validated['accept_tos'] user = DBSession.query(User). \ filter(User.username == username).first() + # try to use the username as email if we didn't find the user if user is None and is_valid_email(username): user = DBSession.query(User). \ @@ -495,6 +501,26 @@ def post(self): token = try_login(user, password, request) if user else None if token: + # Check if the user has validated Terms of Service, if not, + # return a 403 with an explicit message that can be caught + # by the frontend + if user.tos_validated is None and accept_tos is not True: + raise HTTPForbidden('TOS not accepted') + + # If the user has not validated Terms of Service, but the request + # sends the accept field, store it in the database + if user.tos_validated is None and accept_tos is True: + try: + DBSession.execute( + User.__table__.update(). + where(User.id == user.id). + values(tos_validated=datetime.datetime.utcnow()) + ) + DBSession.flush() + except Exception: + log.warning('Error persisting user', exc_info=True) + raise HTTPInternalServerError('Error persisting user') + response = token_to_response(user, token, request) if 'discourse' in request.json: settings = request.registry.settings