diff --git a/NEWS.md b/NEWS.md index fadc08891f..73b82cd5a1 100644 --- a/NEWS.md +++ b/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) diff --git a/app/models/carto/oauth_app.rb b/app/models/carto/oauth_app.rb index 36536316aa..b7530e9be2 100644 --- a/app/models/carto/oauth_app.rb +++ b/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 diff --git a/spec/models/carto/oauth_app_spec.rb b/spec/models/carto/oauth_app_spec.rb index b24d6d1b08..844dd2e346 100644 --- a/spec/models/carto/oauth_app_spec.rb +++ b/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 diff --git a/spec/requests/carto/api/public/oauth_apps_controller_spec.rb b/spec/requests/carto/api/public/oauth_apps_controller_spec.rb index 4b7ffd4743..30f3cd5bc2 100644 --- a/spec/requests/carto/api/public/oauth_apps_controller_spec.rb +++ b/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 }