✨(backend) add ServiceProvider #522

qbey · 2024-11-14T16:14:29Z

Purpose

We want to allow Service Providers to fetch our data but we also need a better management of the data available for each one of them.

Proposal

Not included in this PR: allow Service Provider management from the front.

⚠️ This disables the resource server globaly, to only allow it on a per-view basis because we don't want to allow any service provider to be able to query everything without filtering.

add the ServiceProvider model
allow Service Provider management from Django admin
restrict endpoint availability as a resource server
update changelog

This adds the ServiceProvider notion to allow to better manage which teams is available for each service provider.

We don't want every Service Provider to be able to request every endpoint if those are not implementing a filtering on the data returned. To prevent any data leak we enforce the developers to manually "whitelist" each endpoint and add the proper filtering when needed.

When updating an Organization in the Django admin, the validator falsly raises a "duplicated" error because it does not exclude the current object from the database lookup.

This allow to display service providers in the frontend. Not used yet, but will allow to manage organization and teams related service providers.

Warning where raised because of the casing change between `FROM` and `as`.

sampaccoud · 2024-11-16T08:42:09Z

src/backend/core/tests/teams/test_core_api_teams_update.py

+        HTTP_AUTHORIZATION="Bearer b64untestedbearertoken",
+    )
+
+    assert response.status_code == HTTP_404_NOT_FOUND


maybe the 404 here is a symptom of a wrong split between filtering/annotating and permission checks? No big deal but it resonates with our discussions on impress.

sampaccoud · 2024-11-16T08:43:31Z

src/backend/core/tests/teams/test_core_api_teams_retrieve.py

+    client, authenticated_user_with_service_provider
+):
+    """
+    Authenticated users should not be able to delete a team for which they are directly


Suggested change

Authenticated users should not be able to delete a team for which they are directly

Authenticated users should not be able to retrieve a team for which they are directly

sampaccoud · 2024-11-16T08:45:53Z

src/backend/core/tests/teams/test_core_api_teams_retrieve.py

+    }
+
+
+def test_api_teams_retrieve_authenticated_other_resource_server(


should you also test that a team without any service provider can not be retrieved?

sampaccoud · 2024-11-16T08:47:04Z

src/backend/core/tests/teams/test_core_api_teams_list.py

+    """
+    Authenticated users should be able to list teams
+    they are an owner/administrator/member of, and
+    the service provider should be filtered to the one


Suggested change

the service provider should be filtered to the one

the service providers should be filtered to the one

sampaccoud · 2024-11-16T08:56:43Z

src/backend/core/tests/teams/test_core_api_teams_create.py

+    client, resource_server_settings
+):
+    """
+    Authenticated users should be able to create teams and should automatically be declared


Should it be: Users authenticated via a resource server query ?
If yes, update all below tests?

to differentiate from below's test, precise that the service provider should be created on the fly?

sampaccoud · 2024-11-16T09:00:44Z

src/backend/core/tests/service_providers/test_core_api_service_providers_retrieve.py

+def test_api_service_providers_retrieve_authenticated_on_teams(client):
+    """
+    Authenticated users should not be able to retrieve service providers
+    of because of their teams (might change later if needed).


Suggested change

of because of their teams (might change later if needed).

just because it is related to one of their teams if it is not related to their organization (might change later if needed).

sampaccoud · 2024-11-16T09:01:51Z

src/backend/core/tests/teams/test_core_api_teams_update.py

+
+
+@responses.activate
+def test_api_teams_update_authenticated_owner_resource_server(


I was expecting tests about adding/removing service providers from a team for authenticated admin/owners of the team?

sampaccoud · 2024-11-16T09:02:48Z

src/backend/core/tests/conftest.py

+
+
+@pytest.fixture
+def authenticated_user_with_service_provider(client):


Suggested change

def authenticated_user_with_service_provider(client):

def user_authenticated_via_service_provider(client):

sampaccoud · 2024-11-16T09:04:10Z

src/backend/core/tests/conftest.py

@@ -0,0 +1,148 @@
+"""Defines fixtures for the tests."""


hope we don't abuse this pattern. I like tests to be self understandable without having to jump from file to file to try to understand each fixture.

the necessity to define edge cases tends to create complicated code when you try to mutualize test code.

qbey force-pushed the qbey/add-service-provider branch 7 times, most recently from d2f7a96 to a57a31d Compare November 15, 2024 15:48

qbey added 5 commits November 15, 2024 17:31

✨(backend) add ServiceProvider

4a5b75c

This adds the ServiceProvider notion to allow to better manage which teams is available for each service provider.

🐛(admin) fix organization validators

b5c7d03

When updating an Organization in the Django admin, the validator falsly raises a "duplicated" error because it does not exclude the current object from the database lookup.

✨(service_providers) add API endpoints

62da007

This allow to display service providers in the frontend. Not used yet, but will allow to manage organization and teams related service providers.

🎨(dockerfile) fix FromAsCasing warnings

03fc56b

Warning where raised because of the casing change between `FROM` and `as`.

qbey force-pushed the qbey/add-service-provider branch from 04c282f to 03fc56b Compare November 15, 2024 16:31

qbey marked this pull request as ready for review November 15, 2024 16:31

qbey requested a review from sampaccoud November 15, 2024 16:31

qbey self-assigned this Nov 15, 2024

qbey added the backend label Nov 15, 2024

sampaccoud requested a review from lebaudantoine November 16, 2024 08:08

sampaccoud reviewed Nov 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨(backend) add ServiceProvider #522

✨(backend) add ServiceProvider #522

qbey commented Nov 14, 2024 •

edited

Loading

sampaccoud Nov 16, 2024

sampaccoud Nov 16, 2024

sampaccoud Nov 16, 2024

sampaccoud Nov 16, 2024

sampaccoud Nov 16, 2024

sampaccoud Nov 16, 2024

sampaccoud Nov 16, 2024

sampaccoud Nov 16, 2024

sampaccoud Nov 16, 2024

sampaccoud Nov 16, 2024

	Authenticated users should not be able to delete a team for which they are directly
	Authenticated users should not be able to retrieve a team for which they are directly

		}


		def test_api_teams_retrieve_authenticated_other_resource_server(

	the service provider should be filtered to the one
	the service providers should be filtered to the one

	of because of their teams (might change later if needed).
	just because it is related to one of their teams if it is not related to their organization (might change later if needed).



		@responses.activate
		def test_api_teams_update_authenticated_owner_resource_server(



		@pytest.fixture
		def authenticated_user_with_service_provider(client):

	def authenticated_user_with_service_provider(client):
	def user_authenticated_via_service_provider(client):

✨(backend) add ServiceProvider #522

Are you sure you want to change the base?

✨(backend) add ServiceProvider #522

Conversation

qbey commented Nov 14, 2024 • edited Loading

Purpose

Proposal

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qbey commented Nov 14, 2024 •

edited

Loading