Merge pull request #16304 from CartoDB/feature/ch160203/review-oauth-application-restriction-to-organization

[ch160203] OauthApps restricted by default

NEWS.md

@@ -16,6 +16,7 @@ Development
 - Guard code for Users and Visualizations [#16265](https://github.com/CartoDB/cartodb/pull/16265)
 - Use the organization user's data while editing a user from organization settings [#16280](https://github.com/CartoDB/cartodb/pull/16280)
 - Limit start parameter of Dropbox connector [#16264](https://github.com/CartoDB/cartodb/pull/16264)
+- OauthApps restricted by default [#16304](https://github.com/CartoDB/cartodb/pull/16304)
 - Support staging hostname in the catalog [#16258](https://github.com/CartoDB/cartodb/pull/16258)
 - Fix user migration export/import logs [#16298](https://github.com/CartoDB/cartodb/pull/16298)
 - Allow the usage of WMTS URLs with parameters to create custom basemaps [#16271](https://github.com/CartoDB/cartodb/pull/16271)

app/models/carto/oauth_app.rb

@@ -24,6 +24,7 @@ module Carto
 
     before_validation :ensure_keys_generated
 
+    before_create :restrict_app_to_organization_users, if: ->(app) { app.user.try(:organization_user?) }
     after_create :create_central, if: :sync_with_central?
     after_update :update_central, if: :sync_with_central?
     after_destroy :delete_central, if: :sync_with_central?
@@ -88,6 +89,14 @@ module Carto
       errors.add(:redirect_uris, "must be valid")
     end
 
+    def restrict_app_to_organization_users
+      self.restricted = true
+      oauth_app_organizations.new(
+        organization: user.organization,
+        seats: user.organization.seats
+      )
+    end
+
     def create_central
       cartodb_central_client.create_oauth_app(user.username, sync_attributes)
     end

spec/models/carto/oauth_app_spec.rb

@@ -31,6 +31,44 @@ module Carto
         expect(app.errors[:icon_url]).to(include("must be a valid URL"))
       end
 
+      describe 'restriction' do
+        let(:organization_owner) do
+          create(:organization, :with_owner, owner: @user)
+          @user.reload
+        end
+
+        it 'restrict the access to the user\'s organization if it exists' do
+          app = described_class.new(user: organization_owner,
+                                    name: 'name',
+                                    redirect_uris: ['https://re.dir'],
+                                    website_url: 'http://localhost')
+          expect(app).to(be_valid)
+
+          app.save!
+
+          expect(app.restricted).to(be_true)
+          expect(app.oauth_app_organizations).not_to(be_empty)
+
+          oauth_app_organization = app.oauth_app_organizations.take
+
+          expect(oauth_app_organization.organization_id).to eq(organization_owner.organization_id)
+          expect(oauth_app_organization.seats).to eq(organization_owner.organization.seats)
+        end
+
+        it 'doesn\'t add restrictions if the user has no organization' do
+          app = described_class.new(user: @user,
+                                    name: 'name',
+                                    redirect_uris: ['https://re.dir'],
+                                    website_url: 'http://localhost')
+          expect(app).to(be_valid)
+
+          app.save!
+
+          expect(app.restricted).to(be_false)
+          expect(app.oauth_app_organizations).to(be_empty)
+        end
+      end
+
       describe 'redirection uri' do
         it 'rejects if empty' do
           app = OauthApp.new

spec/requests/carto/api/public/oauth_apps_controller_spec.rb

@@ -164,9 +164,6 @@ describe Carto::Api::Public::OauthAppsController do
       @app2 = create(:oauth_app, user_id: @carto_org_user_2.id, name: 'ABC', restricted: true)
       @app3 = create(:oauth_app, user_id: @carto_org_user_2.id)
 
-      @app1.oauth_app_organizations.create!(organization: @carto_organization, seats: 1)
-      @app2.oauth_app_organizations.create!(organization: @carto_organization, seats: 1)
-
       Carto::OauthAppUser.create!(user: @carto_org_user_1, oauth_app: @app1, scopes: ['user:profile'])
       Carto::OauthAppUser.create!(user: @carto_org_user_1, oauth_app: @app2)
     end
@@ -684,7 +681,6 @@ describe Carto::Api::Public::OauthAppsController do
   describe 'revoke' do
     before(:each) do
       @app = create(:oauth_app, user_id: @carto_org_user_2.id)
-      @app.oauth_app_organizations.create!(organization: @carto_organization, seats: 1)
       @oauth_app_user = Carto::OauthAppUser.create!(user: @carto_org_user_1, oauth_app: @app)
 
       @params = { id: @app.id, api_key: @carto_org_user_1.api_key }