[SILO-396] Modify OAuth flow to move app installation out of token generate (#3694)

* refactor: oauth flow to move app installation out of create token + get installation from token

* added tests for app installation flow and webhook fix

* replace app_installation_id with id as query param

apps/api/plane/authentication/models/oauth.py

@@ -16,7 +16,7 @@ from oauth2_provider.models import (
 from plane.app.permissions.base import ROLE
 from plane.authentication.bgtasks.app_webhook_url_updates import app_webhook_url_updates
 from plane.db.mixins import SoftDeleteModel, UserAuditModel
-from plane.db.models import BaseModel, Project, ProjectMember, User, WorkspaceMember
+from plane.db.models import BaseModel, Project, ProjectMember, User, WorkspaceMember, Webhook
 from plane.db.models.user import BotTypeEnum
 from plane.authentication.bgtasks.send_app_uninstall_webhook import (
     send_app_uninstall_webhook,
@@ -274,6 +274,8 @@ class WorkspaceAppInstallation(BaseModel):
               ],
               ignore_conflicts=True,
           )
+          # create the webhook for the app installation
+          self._create_webhook()
         super(WorkspaceAppInstallation, self).save(*args, **kwargs)
 
     def delete(self, *args, **kwargs):
@@ -328,6 +330,29 @@ class WorkspaceAppInstallation(BaseModel):
             # Call the parent delete method
             super().delete(*args, **kwargs)
 
+    def _create_webhook(self):
+          if self.application.webhook_url:
+              is_new_webhook = True
+              webhook = Webhook()
+              if self.webhook:
+                  webhook = self.webhook
+                  is_new_webhook = False
+
+              webhook.url = self.application.webhook_url
+              webhook.is_active = True
+              # In future, below config comes from the app installation screen
+              webhook.project = True
+              webhook.issue = True
+              webhook.module = True
+              webhook.cycle = True
+              webhook.issue_comment = True
+              webhook.workspace_id = self.workspace_id
+              webhook.created_by_id = self.installed_by_id
+              webhook.updated_by_id = self.installed_by_id
+              webhook.save(disable_auto_set_user=True)
+
+              if is_new_webhook:
+                  self.webhook = webhook
 
 class ApplicationAttachment(BaseModel):
     application = models.ForeignKey(

apps/api/plane/authentication/views/oauth/application.py

@@ -24,11 +24,20 @@ class OAuthApplicationInstalledWorkspacesEndpoint(BaseAPIView):
     def get(self, request: Request, *args: Any, **kwargs: Any) -> Response:
         application: Application = request.auth.application
 
-        filters = {**request.query_params.dict()}
+        filters = {}
+        if request.query_params.get("id"):
+            filters["id"] = request.query_params.get("id")
 
         workspace_applications = WorkspaceAppInstallation.objects.filter(
             application=application, **filters
         )
+        
+        # Always filter those workspaces where user is a member
+        if request.auth.grant_type == "authorization_code":
+          workspace_applications = workspace_applications.filter(
+              workspace__workspace_member__member=request.auth.user
+          )
+        
         workspace_applications_serializer = WorkspaceAppInstallationSerializer(
             workspace_applications, many=True
         )

apps/api/plane/authentication/views/oauth/auth.py

@@ -61,67 +61,39 @@ class OAuthTokenEndpoint(TokenView):
         # Set the bot user on the token
         access_token = token_data.get("access_token")
         if access_token:
+            # get the token
             token = AccessToken.objects.get(token=access_token)
             application_id = token.application_id
             app_installation_id = request.POST.get("app_installation_id")
             grant_type = request.POST.get("grant_type")
 
-            if app_installation_id:
-                workspace_app_installation = WorkspaceAppInstallation.objects.filter(
-                    id=app_installation_id, application_id=application_id
-                ).first()
-
-                if not workspace_app_installation:
+            # if grant type is client credentials, we need to set the bot user
+            if grant_type == "client_credentials":
+                # app installation id is required for client credentials
+                if not app_installation_id:
                     token.delete()
-                    raise exceptions.ValidationError("Workspace application not found")
+                    raise exceptions.ValidationError("App installation ID is required")
+                else:
+                    # get the workspace app installation
+                    workspace_app_installation = WorkspaceAppInstallation.objects.filter(
+                        id=app_installation_id,
+                        application_id=application_id,
+                        status=WorkspaceAppInstallation.Status.INSTALLED,
+                    ).first()
+                    # if the workspace app installation is not found, delete the token
+                    if not workspace_app_installation:
+                        token.delete()
+                        raise exceptions.ValidationError("Workspace application not found")
 
-                if (
-                    workspace_app_installation.status
-                    == WorkspaceAppInstallation.Status.PENDING
-                ):
-                    workspace_app_installation.status = (
-                        WorkspaceAppInstallation.Status.INSTALLED
-                    )
-                    workspace_app_installation.save()
-                    self._create_webhook(workspace_app_installation)
-
-                if grant_type == "client_credentials":
+                    # set the bot user on the token
                     token.user = workspace_app_installation.app_bot
 
+            # set the grant type on the token
             token.grant_type = grant_type
             token.save()
 
         return token_response
 
-    def _create_webhook(self, workspace_app_installation: WorkspaceAppInstallation):
-        try:
-            if workspace_app_installation.application.webhook_url:
-                is_new_webhook = True
-                webhook = Webhook()
-                if workspace_app_installation.webhook:
-                    webhook = workspace_app_installation.webhook
-                    is_new_webhook = False
-
-                webhook.url = workspace_app_installation.application.webhook_url
-                webhook.is_active = True
-                # In future, below config comes from the app installation screen
-                webhook.project = True
-                webhook.issue = True
-                webhook.module = True
-                webhook.cycle = True
-                webhook.issue_comment = True
-                webhook.workspace_id = workspace_app_installation.workspace_id
-                webhook.created_by_id = workspace_app_installation.installed_by_id
-                webhook.updated_by_id = workspace_app_installation.installed_by_id
-                webhook.save(disable_auto_set_user=True)
-
-                if is_new_webhook:
-                    workspace_app_installation.webhook = webhook
-                    workspace_app_installation.save()
-        except Exception as e:
-            log_exception(e)
-
-
 AUTHORIZE_RATE_LIMIT = "10/m"
 
 

apps/api/plane/ee/views/app/oauth/application.py

@@ -176,7 +176,8 @@ class OAuthApplicationEndpoint(BaseAPIView):
             # Single application case
             application = (
                 Application.objects.filter(
-                    id=pk, application_owners__workspace__slug=slug
+                    Q(id=pk, application_owners__workspace__slug=slug)
+                    | Q(published_at__isnull=False, id=pk)
                 )
                 .select_related(
                     "logo_asset",
@@ -279,6 +280,12 @@ class OAuthApplicationInstallEndpoint(BaseAPIView):
             return Response(
                 {"error": "App ID is required"}, status=status.HTTP_400_BAD_REQUEST
             )
+        
+        application = Application.objects.filter(id=pk).first()
+        if not application:
+            return Response(
+                {"error": "Application not found"}, status=status.HTTP_404_NOT_FOUND
+            )
 
         # create or update workspace installation
         workspace_application = WorkspaceAppInstallation.objects.filter(
@@ -290,6 +297,7 @@ class OAuthApplicationInstallEndpoint(BaseAPIView):
                     "workspace": workspace.id,
                     "application": pk,
                     "installed_by": request.user.id,
+                    "status": WorkspaceAppInstallation.Status.INSTALLED,
                 }
             )
         else:
@@ -297,7 +305,7 @@ class OAuthApplicationInstallEndpoint(BaseAPIView):
                 workspace_application,
                 data={
                     "installed_by": request.user.id,
-                    "status": WorkspaceAppInstallation.Status.PENDING,
+                    "status": WorkspaceAppInstallation.Status.INSTALLED,
                 },
                 partial=True,
             )
@@ -309,7 +317,6 @@ class OAuthApplicationInstallEndpoint(BaseAPIView):
                 workspace_application_serialiser.errors,
                 status=status.HTTP_400_BAD_REQUEST,
             )
-
         return Response(
             workspace_application_serialiser.data, status=status.HTTP_200_OK
         )

apps/api/plane/tests/factories.py

@@ -102,6 +102,7 @@ class ApplicationFactory(factory.django.DjangoModelFactory):
     client_secret = factory.Sequence(lambda n: f"test-client-secret-{n}")
     client_type = Application.CLIENT_CONFIDENTIAL
     authorization_grant_type = Application.GRANT_AUTHORIZATION_CODE
+    webhook_url = factory.Sequence(lambda n: f"https://test-webhook-url-{n}")
 
 
 class ApplicationOwnerFactory(factory.django.DjangoModelFactory):

apps/api/plane/tests/unit/ee/test_oauth_application_install.py

@@ -0,0 +1,198 @@
+from django.urls import reverse
+from rest_framework import status
+from plane.authentication.models import WorkspaceAppInstallation
+from plane.db.models import WorkspaceMember, ProjectMember
+
+class TestOAuthApplicationInstallEndpoint:
+    """Test cases for OAuthApplicationInstallEndpoint"""
+
+    def test_install_application_success(
+        self,
+        session_client,
+        workspace,
+        oauth_application,
+        project,
+    ):
+        """Test that installation properly sets up related data and webhook"""
+        url = reverse(
+            "application-install",
+            kwargs={
+                "slug": workspace.slug,
+                "pk": oauth_application.id,
+            },
+        )
+
+        response = session_client.post(url)
+        if response.status_code != status.HTTP_200_OK:
+            print(f"Response status: {response.status_code}")
+            print(f"Response data: {response.data}")
+        assert response.status_code == status.HTTP_200_OK
+
+        # Verify the workspace app installation is created
+        assert WorkspaceAppInstallation.objects.filter(
+            application=oauth_application,
+            workspace=workspace,
+            status=WorkspaceAppInstallation.Status.INSTALLED,
+        ).exists()
+
+        from plane.db.models import Webhook, User
+        from plane.app.permissions import ROLE
+
+        # Verify the webhook is created
+        assert Webhook.objects.filter(
+            url=oauth_application.webhook_url,
+            workspace=workspace,
+            is_active=True,
+            deleted_at__isnull=True,
+        ).exists()
+
+        # Verify the bot user is created
+        user = User.objects.get(
+            username=f"{workspace.slug}_{oauth_application.slug}_bot",
+        )
+        assert user is not None
+        assert user.is_bot is True
+        assert user.is_active is True
+
+        # Verify the bot user is added to the workspace members
+        assert WorkspaceMember.objects.filter(
+            member=user,
+            workspace=workspace,
+            role=ROLE.MEMBER.value,
+        ).exists()
+
+        # Verify the bot user is added to the project members
+        assert ProjectMember.objects.filter(
+            member=user,
+            project=project,
+            workspace=workspace,
+            role=ROLE.MEMBER.value,
+        ).exists()
+
+    def test_install_application_not_found(self, session_client, workspace):
+        """Test installation when application doesn't exist"""
+        import uuid
+
+        non_existent_id = uuid.uuid4()
+        url = reverse(
+            "application-install",
+            kwargs={
+                "slug": workspace.slug,
+                "pk": non_existent_id,
+            },
+        )
+
+        response = session_client.post(url)
+
+        assert response.status_code == status.HTTP_404_NOT_FOUND
+        assert response.data["error"] == "Application not found"
+
+    def test_install_application_workspace_not_found(
+        self,
+        session_client,
+        workspace_app_installation,
+    ):
+        """Test installation when workspace doesn't exist"""
+        url = reverse(
+            "application-install",
+            kwargs={
+                "slug": "non-existent-workspace",
+                "pk": workspace_app_installation.id,
+            },
+        )
+
+        response = session_client.post(url)
+
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+
+    def test_install_application_unauthenticated(
+        self, api_client, workspace, workspace_app_installation
+    ):
+        """Test installation without authentication"""
+        url = reverse(
+            "application-install",
+            kwargs={
+                "slug": workspace.slug,
+                "pk": workspace_app_installation.id,
+            },
+        )
+
+        response = api_client.post(url)
+
+        assert response.status_code == status.HTTP_401_UNAUTHORIZED
+
+    def test_install_application_insufficient_permissions(
+        self, session_client, workspace, workspace_app_installation, create_user
+    ):
+        """Test installation with insufficient permissions"""
+        # Update user role to member (not admin)
+        WorkspaceMember.objects.filter(
+            workspace=workspace,
+            member=create_user,
+        ).update(role=15)
+
+        url = reverse(
+            "application-install",
+            kwargs={
+                "slug": workspace.slug,
+                "pk": workspace_app_installation.id,
+            },
+        )
+
+        response = session_client.post(url)
+
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+
+    def test_install_application_different_http_methods(
+        self,
+        session_client,
+        workspace,
+        oauth_application,
+    ):
+        """Test that only POST method is allowed"""
+        url = reverse(
+            "application-install",
+            kwargs={
+                "slug": workspace.slug,
+                "pk": oauth_application.id,
+            },
+        )
+
+        # Test GET method
+        response = session_client.get(url)
+        assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED
+
+        # Test PUT method
+        response = session_client.put(url)
+        assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED
+
+        # Test PATCH method
+        response = session_client.patch(url)
+        assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED
+
+        # Test POST method
+        response = session_client.delete(url)
+        assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED
+
+    def test_install_application_idempotency(
+        self, session_client, workspace, oauth_application
+    ):
+        """Test that workspace app installation is reused when reinstalling the same app"""
+        # Install the application
+        url = reverse(
+            "application-install",
+            kwargs={
+                "slug": workspace.slug,
+                "pk": oauth_application.id,
+            },
+        )
+
+        installation1 = session_client.post(url)
+        assert installation1.status_code == status.HTTP_200_OK
+
+        # re install the same application
+        installation2 = session_client.post(url)
+        assert installation2.status_code == status.HTTP_200_OK
+
+        # Verify it used the same workspace app installation
+        assert installation2.data["id"] == installation1.data["id"]

apps/web/ee/components/marketplace/applications/installation/details.tsx

@@ -76,6 +76,7 @@ export const ApplicationInstallationDetails: React.FC<ApplicationInstallationDet
         redirect_uri: redirectUri,
         response_type: "code",
         scope: "read write",
+        workspace_slug: currentWorkspace?.slug,
       });
       window.location.assign(authorizationUrl);
     } else {

apps/web/ee/components/marketplace/oauth/app-consent.tsx

@@ -73,9 +73,17 @@ export const AppConsent = observer(({ application, consentParams, workspaceSlug
     router.back();
   };
 
+  const fetchCsrfToken = async () => {
+    const data = await authService.requestCSRFToken();
+    if (data?.csrf_token) {
+      setCsrfToken(data.csrf_token);
+    }
+  };
+
   useEffect(() => {
-    if (csrfToken === undefined)
-      authService.requestCSRFToken().then((data) => data?.csrf_token && setCsrfToken(data.csrf_token));
+    if (csrfToken === undefined) {
+      fetchCsrfToken();
+    }
   }, [csrfToken]);
 
   return (