feat: (WIP) getting through Guilhem's comments

Signed-off-by: Léo-Paul HAUET <[email protected]>

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- New UserAwaitingApproval (base user with no channel) ([#680](https://github.com/Substra/substra-backend/pull/680))
+
 ## [0.39.0](https://github.com/Substra/substra-backend/releases/tag/0.39.0) 2023-06-27
 
 ### Added
@@ -26,7 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Removed
 
-- references to `substra` cli commands in `localdev.md`  ([#667](https://github.com/Substra/substra-backend/pull/667))
+- references to `substra` cli commands in `localdev.md` ([#667](https://github.com/Substra/substra-backend/pull/667))
 
 ## [0.37.0](https://github.com/Substra/substra-backend/releases/tag/0.37.0) 2023-05-11
 

backend/api/tests/views/test_views_computeplan.py

@@ -325,9 +325,8 @@ def test_computeplan_list_success(self):
         )
 
     def test_computeplan_list_wrong_channel(self):
-        extra = {"HTTP_ACCEPT": "application/json;version=0.0"}
         self.client.channel = "yourchannel"
-        response = self.client.get(self.url, **extra)
+        response = self.client.get(self.url, **self.extra)
         self.assertEqual(response.json(), {"count": 0, "next": None, "previous": None, "results": []})
 
     @internal_server_error_on_exception()

backend/api/tests/views/test_views_datamanager.py

@@ -195,9 +195,8 @@ def test_datamanager_list_success(self):
         )
 
     def test_datamanager_list_wrong_channel(self):
-        extra = {"HTTP_ACCEPT": "application/json;version=0.0"}
         self.client.channel = "yourchannel"
-        response = self.client.get(self.url, **extra)
+        response = self.client.get(self.url, **self.extra)
         self.assertEqual(response.json(), {"count": 0, "next": None, "previous": None, "results": []})
 
     @internal_server_error_on_exception()
@@ -503,9 +502,8 @@ def test_datamanager_retrieve_with_tasks(self):
 
     def test_datamanager_retrieve_wrong_channel(self):
         url = reverse("api:data_manager-detail", args=[self.expected_results[0]["key"]])
-        extra = {"HTTP_ACCEPT": "application/json;version=0.0"}
         self.client.channel = "yourchannel"
-        response = self.client.get(url, **extra)
+        response = self.client.get(url, **self.extra)
         self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
 
     def test_datamanager_retrieve_storage_addresses_update(self):

backend/api/tests/views/test_views_datasample.py

@@ -3,7 +3,6 @@
 import ntpath
 import os
 import shutil
-import sys
 import tempfile
 from unittest import mock
 
@@ -111,12 +110,8 @@ def test_datasample_retrieve(self):
 
     def test_datasample_retrieve_wrong_channel(self):
         url = reverse("api:data_sample-detail", args=[self.expected_results[0]["key"]])
-        extra = {"HTTP_ACCEPT": "application/json;version=0.0"}
         self.client.channel = "yourchannel"
-        sys.stderr.write(f"{self.client.channel}")
-        sys.stderr.write(f"{type(self.client.channel)}")
-
-        response = self.client.get(url, **extra)
+        response = self.client.get(url, **self.extra)
         self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
 
     @internal_server_error_on_exception()
@@ -139,9 +134,8 @@ def test_datasample_list_success(self):
         )
 
     def test_datasample_list_wrong_channel(self):
-        extra = {"HTTP_ACCEPT": "application/json;version=0.0"}
         self.client.channel = "yourchannel"
-        response = self.client.get(self.url, **extra)
+        response = self.client.get(self.url, **self.extra)
         self.assertEqual(response.json(), {"count": 0, "next": None, "previous": None, "results": []})
 
     @internal_server_error_on_exception()

backend/users/tests/test_user.py

@@ -105,7 +105,6 @@ def test_oidc_username_generation(self):
 
 class TestUserEndpoints:
     url = None
-    extra = None
 
     @pytest.fixture(autouse=True)
     def use_dummy_channels(self, settings):

backend/users/views/user.py

@@ -95,8 +95,7 @@ def has_permission(self, request, view):
 
 class IsAdmin(permissions.BasePermission):
     def has_permission(self, request, view):
-        if request.user.is_authenticated:
-            return request.user.channel.role == UserChannel.Role.ADMIN
+        return request.user.channel.role == UserChannel.Role.ADMIN
 
 
 class IsSelf(permissions.BasePermission):
@@ -276,19 +275,21 @@ def put(self, request, *args, **kwargs):
         d = json.loads(request.body)
         try:
             user = User.objects.get(username=request.GET.get("username"))
-        except User.DoesNotExist or User.MultipleObjectsReturned:
+        except User.DoesNotExist:
             return ApiResponse(data={"message": "User not found"}, status=status.HTTP_404_NOT_FOUND)
+        except User.MultipleObjectsReturned:
+            return ApiResponse(
+                data={"message": "Multiple instance of the same user found"}, status=status.HTTP_409_CONFLICT
+            )
 
         channel_name = get_channel_name(request)
-        channel_data = {"channel_name": channel_name}
-        channel_data["role"] = _validate_role(d.get("role"))
-        channel_data["user"] = user
-        UserChannel.objects.create(**channel_data)
-        user.refresh_from_db()
+        channel_name = get_channel_name(request)
+        role = _validate_role(d.get("role"))
+        UserChannel.objects.create(channel_name=channel_name, role=role, user=user)
         data = UserSerializer(instance=user).data
-        return ApiResponse(data=data, status=status.HTTP_200_OK)  # get success header ?
+        return ApiResponse(data=data, status=status.HTTP_200_OK)
 
-    # TODO THIS SHOULD NOT BE IN THE PULL REQUEST
+    # TODO THIS SHOULD NOT BE MERGED
     def post(self, request, *args, **kwargs):
         d = json.loads(request.body)
         user, created = User.objects.get_or_create(username=d.get("username"), email=d.get("email"))

charts/substra-backend/CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## [Unreleased]
+
+### Added
+
+- New `requireApproval` field, that triggers the User Awaiting Approval functionnality ([#680](https://github.com/Substra/substra-backend/pull/680))
+
 ## [22.5.2] - 2023-06-27
 
 ### Changed

charts/substra-backend/Chart.yaml

@@ -1,7 +1,7 @@
 apiVersion: v2
 name: substra-backend
 home: https://github.com/Substra
-version: 22.5.2
+version: 22.6.0
 appVersion: 0.39.0
 kubeVersion: ">= 1.19.0-0"
 description: Main package for Substra

charts/substra-backend/templates/configmap-oidc.yaml

@@ -8,6 +8,7 @@ data:
   OIDC_ENABLED: {{ .Values.oidc.enabled | quote }}
   OIDC_USERS_APPEND_DOMAIN: {{ .Values.oidc.users.appendDomain | quote }}
   OIDC_USERS_DEFAULT_CHANNEL: {{ .Values.oidc.users.channel | default "" | quote }}
+  OIDC_USERS_MUST_BE_APPROVED: {{ .Values.oidc.users.requireApproval | default "" | quote }}
   OIDC_USERS_LOGIN_VALIDITY_DURATION: {{ .Values.oidc.users.loginValidityDuration | default "" | quote }}
   OIDC_USERS_USE_REFRESH_TOKEN: {{ .Values.oidc.users.useRefreshToken | quote }}
   OIDC_RP_SIGN_ALGO: {{ .Values.oidc.signAlgo | default "" | quote }}

charts/substra-backend/values.yaml

@@ -12,7 +12,6 @@ organizationName: owkin
 ##
 DataSampleStorageInServerMedia: false
 
-
 privateCa:
   ## @param privateCa.enabled Run the init container injecting the private CA certificate
   ##
@@ -427,7 +426,6 @@ schedulerWorker:
     runAsGroup: 1001
     fsGroup: 1001
 
-
 ## @section Celery task scheduler settings
 scheduler:
   ## @param scheduler.enabled Enable scheduler service
@@ -654,7 +652,7 @@ addAccountOperator:
 ## @descriptionStart Uses the authorization code flow.
 ##
 ## By default, `oidc.users.useRefreshToken` is enabled. This makes sure the user still has an account at the identity provider, without damaging user experience.
-## 
+##
 ## The way it works is that a OIDC user that spent more than `oidc.users.loginValidityDuration` since their last login must undergo a refresh to keep using their access tokens -- but these refreshes are done in the background if `oidc.users.useRefreshToken` is enabled (otherwise a new manual authorization is necessary). The identity provider must support `offline_access` and configuration discovery.
 ##
 ## With this option active, you can set `oidc.users.loginValidityDuration` to low values (minutes).
@@ -666,10 +664,10 @@ oidc:
   ## @param oidc.enabled Whether to enable OIDC authentication
   ##
   enabled: false
-  
+
   ## @param oidc.clientSecretName The name of a secret containing the keys `OIDC_RP_CLIENT_ID` and `OIDC_RP_CLIENT_SECRET` (client ID and secret, typically issued by the provider)
   clientSecretName: null
-  
+
   provider:
     ## @param oidc.provider.url The identity provider URL (with scheme).
     url: null
@@ -683,10 +681,10 @@ oidc:
       token: null
       ## @param oidc.provider.endpoints.user Typically https://provider/me
       user: null
-    
+
     ## @param oidc.provider.jwksUri Typically https://provider/jwks. Only required for public-key-based signing algorithms. If not given, read from `/.well-known/openid-configuration` at startup.
     jwksUri: null
-  
+
   ## @param oidc.signAlgo Either RS256 or HS256
   signAlgo: RS256
   users:
@@ -696,6 +694,8 @@ oidc:
     loginValidityDuration: 3600
     ## @param oidc.users.channel The channel to assign OIDC users to (mandatory)
     channel: null
+    ## @param oidc.users.requireApproval Is not compatible with default channel
+    requireApproval: false
     ## @param oidc.users.appendDomain As usernames are assigned based on e-mail address, whether to suffix user names with the email domain ([email protected] would then be `john-doe-example`)
     appendDomain: false
 
@@ -708,17 +708,16 @@ database:
     username: &psql-username postgres
     ## @param database.auth.password what password to use for connecting
     password: &psql-password postgres
-    
+
     ## @param database.auth.credentialsSecretName An alternative to giving username and password; must have `DATABASE_USERNAME` and `DATABASE_PASSWORD` keys.
     ##
     credentialsSecretName: null
-  
+
   ## @param database.host Hostname of the database to connect to (defaults to local)
   host: null
   ## @param database.port Port of an external database to connect to
   port: 5432
 
-
 ## @section PostgreSQL settings
 ## @descriptionStart
 ## Database included as a subchart used by default.