diff --git a/Makefile.local b/Makefile.local --- a/Makefile.local +++ b/Makefile.local @@ -35,6 +35,7 @@ rm -f swh/web/settings/testdb.sqlite3 django-admin migrate --settings=swh.web.settings.tests -v0 2>/dev/null cat swh/web/tests/create_test_admin.py | django-admin shell --settings=swh.web.settings.tests + cat swh/web/tests/create_test_users.py | django-admin shell --settings=swh.web.settings.tests .PHONY: clear-memcached clear-memcached: diff --git a/assets/src/bundles/save/index.js b/assets/src/bundles/save/index.js --- a/assets/src/bundles/save/index.js +++ b/assets/src/bundles/save/index.js @@ -33,6 +33,16 @@ }); } +const userRequestsFilterCheckbox = ` +
+ + +
+`; + export function initOriginSave() { $(document).ready(() => { @@ -58,8 +68,29 @@ language: { processing: `` }, - ajax: Urls.origin_save_requests_list('all'), + ajax: { + url: Urls.origin_save_requests_list('all'), + data: (d) => { + if (swh.webapp.isUserLoggedIn() && $('#swh-save-requests-user-filter').prop('checked')) { + d.user_requests_only = '1'; + } + } + }, searchDelay: 1000, + // see https://datatables.net/examples/advanced_init/dom_toolbar.html and the comments section + // this option customizes datatables UI components by adding an extra checkbox above the table + // while keeping bootstrap layout + dom: '<"row"<"col-sm-3"l><"col-sm-6 text-left user-requests-filter"><"col-sm-3"f>>' + + '<"row"<"col-sm-12"tr>>' + + '<"row"<"col-sm-5"i><"col-sm-7"p>>', + fnInitComplete: function() { + if (swh.webapp.isUserLoggedIn()) { + $('div.user-requests-filter').html(userRequestsFilterCheckbox); + $('#swh-save-requests-user-filter').on('change', () => { + saveRequestsTable.draw(); + }); + } + }, columns: [ { data: 'save_request_date', diff --git a/cypress/integration/origin-save.spec.js b/cypress/integration/origin-save.spec.js --- a/cypress/integration/origin-save.spec.js +++ b/cypress/integration/origin-save.spec.js @@ -400,7 +400,7 @@ }); it('should create save request for authenticated user', function() { - cy.adminLogin(); + cy.userLogin(); cy.visit(url); const originUrl = 'https://git.example.org/account/repo'; stubSaveRequest({requestUrl: this.Urls.api_1_save_origin('git', originUrl), @@ -415,4 +415,61 @@ }); }); + it('should not show user requests filter checkbox for anonymous users', function() { + cy.get('#swh-origin-save-requests-list-tab').click(); + cy.get('#swh-save-requests-user-filter').should('not.exist'); + }); + + it('should show user requests filter checkbox for authenticated users', function() { + cy.userLogin(); + cy.visit(url); + cy.get('#swh-origin-save-requests-list-tab').click(); + cy.get('#swh-save-requests-user-filter').should('exist'); + }); + + it('should show only user requests when filter is activated', function() { + cy.intercept('POST', '/api/1/origin/save/**') + .as('saveRequest'); + + const originAnonymousUser = 'https://some.git.server/project/'; + const originAuthUser = 'https://other.git.server/project/'; + + // anonymous user creates a save request + makeOriginSaveRequest('git', originAnonymousUser); + cy.wait('@saveRequest'); + + // authenticated user creates another save request + cy.userLogin(); + cy.visit(url); + makeOriginSaveRequest('git', originAuthUser); + cy.wait('@saveRequest'); + + // user requests filter checkbox should be in the DOM + cy.get('#swh-origin-save-requests-list-tab').click(); + cy.get('#swh-save-requests-user-filter').should('exist'); + + // check unfiltered user requests + cy.get('tbody tr').then(rows => { + expect(rows.length).to.eq(2); + expect($(rows[0].cells[2]).text()).to.contain(originAuthUser); + expect($(rows[1].cells[2]).text()).to.contain(originAnonymousUser); + }); + + // activate filter and check filtered user requests + cy.get('#swh-save-requests-user-filter') + .click({force: true}); + cy.get('tbody tr').then(rows => { + expect(rows.length).to.eq(1); + expect($(rows[0].cells[2]).text()).to.contain(originAuthUser); + }); + + // deactivate filter and check unfiltered user requests + cy.get('#swh-save-requests-user-filter') + .click({force: true}); + cy.get('tbody tr').then(rows => { + expect(rows.length).to.eq(2); + }); + + }); + }); diff --git a/cypress/support/index.js b/cypress/support/index.js --- a/cypress/support/index.js +++ b/cypress/support/index.js @@ -19,7 +19,7 @@ expect(Object.keys(aliasRoute.requests || {})).to.have.length(timesCalled); }); -Cypress.Commands.add('adminLogin', () => { +function loginUser(username, password) { const url = '/admin/login/'; return cy.request({ url: url, @@ -33,8 +33,8 @@ form: true, followRedirect: false, body: { - username: 'admin', - password: 'admin', + username: username, + password: password, csrfmiddlewaretoken: token } }).then(() => { @@ -43,6 +43,14 @@ }); }); }); +} + +Cypress.Commands.add('adminLogin', () => { + return loginUser('admin', 'admin'); +}); + +Cypress.Commands.add('userLogin', () => { + return loginUser('user', 'user'); }); before(function() { diff --git a/swh/web/common/migrations/0011_saveoriginrequest_user_ids.py b/swh/web/common/migrations/0011_saveoriginrequest_user_ids.py new file mode 100644 --- /dev/null +++ b/swh/web/common/migrations/0011_saveoriginrequest_user_ids.py @@ -0,0 +1,22 @@ +# Copyright (C) 2021 The Software Heritage developers +# See the AUTHORS file at the top-level directory of this distribution +# License: GNU General Public License version 3, or any later version +# See top-level LICENSE file for more information + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("swh_web_common", "0010_saveoriginrequest_user_id"), + ] + + operations = [ + migrations.RemoveField(model_name="saveoriginrequest", name="user_id",), + migrations.AddField( + model_name="saveoriginrequest", + name="user_ids", + field=models.TextField(null=True), + ), + ] diff --git a/swh/web/common/models.py b/swh/web/common/models.py --- a/swh/web/common/models.py +++ b/swh/web/common/models.py @@ -100,9 +100,8 @@ loading_task_status = models.TextField( choices=SAVE_TASK_STATUS, default=SAVE_TASK_NOT_CREATED ) - # user integer ids computed from keycloak subs are too large - # to be stored in SQLite so we store them as strings - user_id = models.CharField(max_length=200, null=True) + # store ids of users that submitted the request as string list + user_ids = models.TextField(null=True) class Meta: app_label = "swh_web_common" diff --git a/swh/web/common/origin_save.py b/swh/web/common/origin_save.py --- a/swh/web/common/origin_save.py +++ b/swh/web/common/origin_save.py @@ -448,7 +448,7 @@ origin_url=origin_url, status=save_request_status, loading_task_id=task["id"], - user_id=str(user_id) if user_id else None, + user_ids=f'"{user_id}"' if user_id else None, ) # save request must be manually reviewed for acceptation elif save_request_status == SAVE_REQUEST_PENDING: @@ -458,13 +458,19 @@ sor = SaveOriginRequest.objects.get( visit_type=visit_type, origin_url=origin_url, status=save_request_status ) + user_ids = sor.user_ids if sor.user_ids is not None else "" + if user_id is not None and f'"{user_id}"' not in user_ids: + # update user ids list + sor.user_ids = f'{sor.user_ids},"{user_id}"' + sor.save() + # if not add it to the database except ObjectDoesNotExist: sor = SaveOriginRequest.objects.create( visit_type=visit_type, origin_url=origin_url, status=save_request_status, - user_id=str(user_id) if user_id else None, + user_ids=f'"{user_id}"' if user_id else None, ) # origin can not be saved as its url is blacklisted, # log the request to the database anyway @@ -473,7 +479,7 @@ visit_type=visit_type, origin_url=origin_url, status=save_request_status, - user_id=str(user_id) if user_id else None, + user_ids=f'"{user_id}"' if user_id else None, ) if save_request_status == SAVE_REQUEST_REJECTED: diff --git a/swh/web/misc/origin_save.py b/swh/web/misc/origin_save.py --- a/swh/web/misc/origin_save.py +++ b/swh/web/misc/origin_save.py @@ -61,6 +61,12 @@ | Q(origin_url__icontains=search_value) ) + if ( + int(request.GET.get("user_requests_only", "0")) + and request.user.is_authenticated + ): + save_requests = save_requests.filter(user_ids__contains=f'"{request.user.id}"') + table_data["recordsFiltered"] = save_requests.count() paginator = Paginator(save_requests, length) table_data["data"] = [sor.to_dict() for sor in paginator.page(page).object_list] diff --git a/swh/web/templates/misc/origin-save.html b/swh/web/templates/misc/origin-save.html --- a/swh/web/templates/misc/origin-save.html +++ b/swh/web/templates/misc/origin-save.html @@ -96,6 +96,9 @@

Once a save request has been accepted, you can follow its current status in the submitted save requests list. +
+ If you submitted requests while authenticated, you will be able + to only display your own requests.

diff --git a/swh/web/tests/api/views/test_origin_save.py b/swh/web/tests/api/views/test_origin_save.py --- a/swh/web/tests/api/views/test_origin_save.py +++ b/swh/web/tests/api/views/test_origin_save.py @@ -7,6 +7,7 @@ import pytest +from django.contrib.auth.models import User from django.core.exceptions import ObjectDoesNotExist from django.utils import timezone @@ -488,7 +489,7 @@ sor = SaveOriginRequest.objects.get(origin_url=origin_url) - assert sor.user_id is None + assert sor.user_ids is None def test_create_save_request_authenticated_user_id( @@ -507,5 +508,23 @@ assert response.wsgi_request.user.id is not None user_id = str(response.wsgi_request.user.id) - sor = SaveOriginRequest.objects.get(user_id=user_id) - assert sor.user_id == user_id + sor = SaveOriginRequest.objects.get(user_ids=f'"{user_id}"') + assert sor.user_ids == f'"{user_id}"' + + +def test_create_pending_save_request_multiple_authenticated_users(api_client): + origin_url = "https://some.git.hosters/user/repo3" + first_user = User.objects.create_user(username="first_user", password="") + second_user = User.objects.create_user(username="second_user", password="") + url = reverse( + "api-1-save-origin", url_args={"visit_type": "git", "origin_url": origin_url}, + ) + + api_client.force_login(first_user) + check_api_post_response(api_client, url, status_code=200) + + api_client.force_login(second_user) + check_api_post_response(api_client, url, status_code=200) + + assert SaveOriginRequest.objects.get(user_ids__contains=f'"{first_user.id}"') + assert SaveOriginRequest.objects.get(user_ids__contains=f'"{second_user.id}"') diff --git a/swh/web/tests/create_test_users.py b/swh/web/tests/create_test_users.py new file mode 100644 --- /dev/null +++ b/swh/web/tests/create_test_users.py @@ -0,0 +1,16 @@ +# Copyright (C) 2021 The Software Heritage developers +# See the AUTHORS file at the top-level directory of this distribution +# License: GNU Affero General Public License version 3, or any later version +# See top-level LICENSE file for more information + + +from django.contrib.auth import get_user_model + +User = get_user_model() + +username = "user" +password = "user" +email = "user@swh-web.org" + +if not User.objects.filter(username=username).exists(): + User.objects.create_user(username, email, password) diff --git a/swh/web/tests/misc/test_origin_save.py b/swh/web/tests/misc/test_origin_save.py --- a/swh/web/tests/misc/test_origin_save.py +++ b/swh/web/tests/misc/test_origin_save.py @@ -8,6 +8,7 @@ import pytest +from swh.auth.django.utils import oidc_user_from_profile from swh.web.common.models import SaveOriginRequest from swh.web.common.origin_save import SAVE_REQUEST_ACCEPTED, SAVE_TASK_SUCCEEDED from swh.web.common.utils import reverse @@ -23,7 +24,7 @@ @pytest.mark.django_db -def test_save_origin_requests_list(client, mocker): +def test_save_origin_requests_list(client, mocker, keycloak_oidc): visit_types = ("git", "svn", "hg") nb_origins_per_type = 10 for visit_type in visit_types: @@ -94,3 +95,57 @@ assert sors["recordsTotal"] == len(visit_types) * nb_origins_per_type assert len(sors["data"]) == nb_origins_per_type assert all(d["visit_type"] == visit_type for d in sors["data"]) + + +@pytest.mark.django_db +def test_save_origin_requests_list_user_filter(client, mocker, keycloak_oidc): + + # anonymous user created a save request + sor = SaveOriginRequest.objects.create( + request_date=datetime.now(tz=timezone.utc), + visit_type="svn", + origin_url="https://svn.example.org/user/project", + status=SAVE_REQUEST_ACCEPTED, + visit_date=datetime.now(tz=timezone.utc) + timedelta(hours=1), + loading_task_id=1, + loading_task_status=SAVE_TASK_SUCCEEDED, + ) + + # authenticated user created a save request + user = oidc_user_from_profile(keycloak_oidc, keycloak_oidc.login()) + client.login(code="", code_verifier="", redirect_uri="") + + sor = SaveOriginRequest.objects.create( + request_date=datetime.now(tz=timezone.utc), + visit_type="git", + origin_url="https://git.example.org/user/project", + status=SAVE_REQUEST_ACCEPTED, + visit_date=datetime.now(tz=timezone.utc) + timedelta(hours=1), + loading_task_id=2, + loading_task_status=SAVE_TASK_SUCCEEDED, + user_ids=f'"{user.id}"', + ) + + # filter save requests according to user id + url = reverse( + "origin-save-requests-list", + url_args={"status": "all"}, + query_params={ + "draw": 1, + "search[value]": "", + "order[0][column]": "0", + "columns[0][name]": "request_date", + "order[0][dir]": "desc", + "length": 10, + "start": "0", + "user_requests_only": "1", + }, + ) + + resp = check_http_get_response( + client, url, status_code=200, content_type="application/json" + ) + sors = json.loads(resp.content.decode("utf-8")) + assert sors["recordsFiltered"] == 1 + assert sors["recordsTotal"] == 2 + assert sors["data"][0] == sor.to_dict() diff --git a/swh/web/tests/test_migrations.py b/swh/web/tests/test_migrations.py --- a/swh/web/tests/test_migrations.py +++ b/swh/web/tests/test_migrations.py @@ -8,6 +8,7 @@ MIGRATION_0008 = "0008_save-code-now_indexes_20210106_1327" MIGRATION_0009 = "0009_saveoriginrequest_visit_status" MIGRATION_0010 = "0010_saveoriginrequest_user_id" +MIGRATION_0011 = "0011_saveoriginrequest_user_ids" def test_migrations_09_add_visit_status_to_sor_model(migrator): @@ -36,3 +37,17 @@ new_model = new_state.apps.get_model(APP_NAME, "SaveOriginRequest") assert hasattr(new_model, "user_id") is True + + +def test_migrations_11_add_user_ids_to_sor_model(migrator): + """Ensures the migration adds the user_id field to SaveOriginRequest table""" + + old_state = migrator.apply_initial_migration((APP_NAME, MIGRATION_0010),) + old_model = old_state.apps.get_model(APP_NAME, "SaveOriginRequest") + + assert hasattr(old_model, "user_ids") is False + + new_state = migrator.apply_tested_migration((APP_NAME, MIGRATION_0011)) + new_model = new_state.apps.get_model(APP_NAME, "SaveOriginRequest") + + assert hasattr(new_model, "user_ids") is True