Merge pull request #700 from openstax/fine-grained-app-access

Add fine grained app permissions

app/access_policies/message_access_policy.rb

@@ -3,8 +3,8 @@ class MessageAccessPolicy
   # with which Message objects.
 
   def self.action_allowed?(action, requestor, msg)
-    # Only trusted apps can access this API
-    return false unless requestor.is_application? && requestor.trusted
+    # Only selected apps can access this API
+    return false unless requestor.is_application? && requestor.can_message_users
 
     # Create is currently the only available action
     [:create].include?(action)

app/access_policies/user_access_policy.rb

@@ -16,8 +16,8 @@ def self.action_allowed?(action, requestor, user)
       requestor == user # Temp or unclaimed users only
     when :unclaimed
       # find-or-create accounts that are a stand-in for a person who's not yet signed up
-      # only trusted applications can access this via client credentials
-      Rails.env.development? || (requestor.is_application? && requestor.trusted?)
+      # only selected applications can access this via client credentials
+      Rails.env.development? || (requestor.is_application? && requestor.can_find_or_create_accounts?)
     end
   end
 end

app/controllers/api/v1/application_users_controller.rb

@@ -91,7 +91,7 @@ def index
     app = current_api_user.application
     respond_with outputs, represent_with: Api::V1::UserSearchRepresenter,
                           user_options: {
-                            include_private_data: app && app.trusted?
+                            include_private_data: app && app.can_access_private_user_data?
                           },
                           location: nil
   end
@@ -185,7 +185,7 @@ def updates
     outputs = GetUpdatedApplicationUsers.call(current_application, params[:limit]).outputs
     respond_with outputs[:application_users],
                  represent_with: Api::V1::ApplicationUsersRepresenter,
-                 user_options: { include_private_data: current_application && current_application.trusted? },
+                 user_options: { include_private_data: current_application && current_application.can_access_private_user_data? },
                  location: nil
   end
 

app/controllers/api/v1/messages_controller.rb

@@ -7,7 +7,7 @@ class Api::V1::MessagesController < Api::V1::ApiController
     description <<-EOS
       All actions in this controller require an Oauth token
       obtained through the Client Credentials flow.
-      Only trusted applications can currently access this API.
+      Only selected applications can currently access this API.
 
       Messages belong to applications.
       They represent messages sent to Users through their ContactInfos.
@@ -44,7 +44,7 @@ class Api::V1::MessagesController < Api::V1::ApiController
   api :POST, '/messages', 'Creates and sends a new Message.'
   description <<-EOS
     Creates and sends a new Message to the given users.
-    Can only be called by a trusted application, using a token
+    Can only be called by a selected application, using a token
     obtained via the Client Credentials Oauth flow.
     Returns a JSON representation of the sent message.
 

app/controllers/api/v1/users_controller.rb

@@ -95,7 +95,7 @@ def index
     respond_with outputs, represent_with: Api::V1::UserSearchRepresenter,
                           location: nil,
                           user_options: {
-                            include_private_data: current_application.try(:trusted?)
+                            include_private_data: current_application.try(:can_access_private_user_data?)
                           }
   end
 

app/controllers/oauth/applications_controller.rb

@@ -111,8 +111,13 @@ def set_user
     def app_params
       params.require(:doorkeeper_application).permit(
         :name, :redirect_uri, :scopes, :email_subject_prefix, :lead_application_source,
-        :trusted, :email_from_address, :confidential
+        :trusted, :email_from_address, :confidential,
+        :can_access_private_user_data, :can_find_or_create_accounts, :can_message_users,
+        :can_skip_oauth_screen,
       )
     end
   end
 end
+
+
+

app/views/doorkeeper/applications/_form.html.erb

@@ -78,15 +78,25 @@
       </div>
     <% end %>
 
+    <% [
+      :trusted,
+      :can_access_private_user_data,
+      :can_find_or_create_accounts,
+      :can_message_users,
+      :can_skip_oauth_screen,
+    ].each do |priviledge| %>
+
     <%= content_tag :div,
-        class: "form-group#{' has-error' if application.errors[:trusted].present?}" do %>
-      <%= f.label :trusted?, class: 'col-sm-2 control-label',
-                             for: 'doorkeeper_application_trusted' %>
+        class: "form-group#{' has-error' if application.errors[priviledge].present?}" do %>
+      <%= f.label priviledge.to_s.humanize + "?", class: 'col-sm-2 control-label',
+                             for: "doorkeeper_application_#{priviledge}" %>
       <div class="col-sm-10">
-        <%= f.check_box :trusted %>
-        <%= doorkeeper_errors_for application, :trusted %>
+        <%= f.check_box priviledge %>
+        <%= doorkeeper_errors_for application, priviledge %>
       </div>
     <% end %>
+
+    <% end %>
   <% end %>
 
   <div class="form-group">

app/views/doorkeeper/applications/show.html.erb

@@ -44,9 +44,20 @@
 
     <p><%= @application.lead_application_source %></p>
 
-    <h4>Trusted?</h4>
+  <% [
+    :trusted,
+    :can_access_private_user_data,
+    :can_find_or_create_accounts,
+    :can_message_users,
+    :can_skip_oauth_screen,
+  ].each do |priviledge| %>
+
+    <h4><%= priviledge.to_s.humanize %>?</h4>
+
+    <p><%= @application.send(priviledge) ? 'Yes' : 'No' %></p>
+
+  <% end %>
 
-    <p><%= @application.trusted ? 'Yes' : 'No' %></p>
   </div>
 
   <div class="col-md-4">

config/initializers/doorkeeper.rb

@@ -192,7 +192,7 @@
   # For example if dealing with a trusted application.
   #
   skip_authorization do |resource_owner, client|
-    client.application.trusted
+    client.application.can_skip_oauth_screen
   end
 
   # WWW-Authenticate Realm (default "Doorkeeper").

config/initializers/doorkeeper_models.rb

@@ -22,6 +22,30 @@ def is_redirect_url?(url)
     # Let doorkeeper do the work of checking the URL against the app's redirect_uris
     Doorkeeper::OAuth::Helpers::URIChecker.valid_for_authorization?(url, redirect_uri)
   end
+
+  ## TODO REMOVE THE FOLLOWING AFTER THESE NEW PERMISSIONS FIELDS ARE DEPLOYED
+
+  def respond_to_missing?(method, include_private=false)
+    [
+      :can_access_private_user_data,
+      :can_find_or_create_accounts,
+      :can_message_users,
+      :can_skip_oauth_screen,
+    ].include?(method.to_s.gsub(/\?/,'').to_sym) || super
+  end
+
+  def method_missing(method, *args, &block)
+    if [
+      :can_access_private_user_data,
+      :can_find_or_create_accounts,
+      :can_message_users,
+      :can_skip_oauth_screen,
+    ].include?(method.to_s.gsub(/\?/,'').to_sym)
+      trusted
+    else
+      super
+    end
+  end
 end
 
 Doorkeeper::AccessToken.class_eval do

db/migrate/20190911171944_add_fine_grained_access_to_oauth_applications.rb

@@ -0,0 +1,24 @@
+class AddFineGrainedAccessToOauthApplications < ActiveRecord::Migration[5.2]
+  def up
+    add_column :oauth_applications, :can_access_private_user_data, :boolean, default: false, null: false
+    add_column :oauth_applications, :can_find_or_create_accounts, :boolean, default: false, null: false
+    add_column :oauth_applications, :can_message_users, :boolean, default: false, null: false
+    add_column :oauth_applications, :can_skip_oauth_screen, :boolean, default: false, null: false
+
+    Doorkeeper::Application.reset_column_information
+
+    Doorkeeper::Application.where(trusted: true).update_all(
+      can_access_private_user_data: true,
+      can_find_or_create_accounts: true,
+      can_message_users: true,
+      can_skip_oauth_screen: true
+    )
+  end
+
+  def down
+    remove_column :oauth_applications, :can_access_private_user_data
+    remove_column :oauth_applications, :can_find_or_create_accounts
+    remove_column :oauth_applications, :can_message_users
+    remove_column :oauth_applications, :can_skip_oauth_screen
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_02_06_185243) do
+ActiveRecord::Schema.define(version: 2019_09_11_171944) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "citext"
@@ -247,6 +247,10 @@
     t.string "scopes", default: "", null: false
     t.string "lead_application_source", default: "", null: false
     t.boolean "confidential", default: true, null: false
+    t.boolean "can_access_private_user_data", default: false
+    t.boolean "can_find_or_create_accounts", default: false
+    t.boolean "can_message_users", default: false
+    t.boolean "can_skip_oauth_screen", default: false
     t.index ["owner_id", "owner_type"], name: "index_oauth_applications_on_owner_id_and_owner_type"
     t.index ["uid"], name: "index_oauth_applications_on_uid", unique: true
   end

spec/controllers/api/v1/application_users_controller_spec.rb

@@ -3,7 +3,7 @@
 RSpec.describe Api::V1::ApplicationUsersController, type: :controller, api: true, version: :v1 do
 
   let!(:untrusted_application) { FactoryBot.create :doorkeeper_application }
-  let!(:trusted_application)   { FactoryBot.create :doorkeeper_application, :trusted }
+  let!(:trusted_application)   { FactoryBot.create :doorkeeper_application, can_access_private_user_data: true }
 
   let!(:untrusted_application_token) do
     FactoryBot.create :doorkeeper_access_token, application: untrusted_application,

spec/controllers/api/v1/messages_controller_spec.rb

@@ -7,7 +7,7 @@
                        email_from_address: '[email protected]'
   }
   let!(:trusted_application)   {
-    FactoryBot.create :doorkeeper_application, :trusted,
+    FactoryBot.create :doorkeeper_application, can_message_users: true,
                        email_from_address: '[email protected]'
   }
   let!(:user_1)                { FactoryBot.create :user }

spec/controllers/api/v1/users_controller_spec.rb

@@ -62,6 +62,19 @@
       api_get :index, trusted_application_token, params: {q: 'first_name:bob last_name:Michaels'}
       expect(response.code).to eq('200')
 
+      expected_response = {
+        total_count: 1,
+        items: [ user_matcher(user_2, include_private_data: false) ]
+      }
+
+      expect(response.body_as_hash).to match(expected_response)
+    end
+
+    it "returns a single result well with private data" do
+      trusted_application.update_attribute(:can_access_private_user_data, true)
+      api_get :index, trusted_application_token, params: {q: 'first_name:bob last_name:Michaels'}
+      expect(response.code).to eq('200')
+
       expected_response = {
         total_count: 1,
         items: [ user_matcher(user_2, include_private_data: true) ]
@@ -249,10 +262,16 @@
   end
 
   context "find or create" do
+    let!(:foc_trusted_application)   { FactoryBot.create :doorkeeper_application, can_find_or_create_accounts: true }
+    let!(:foc_trusted_application_token) do
+      FactoryBot.create :doorkeeper_access_token, application: foc_trusted_application,
+                                                   resource_owner_id: nil
+    end
+
     it "should create a new user for an app" do
       expect{
         api_post :find_or_create,
-                 trusted_application_token,
+                 foc_trusted_application_token,
                  body: {email: '[email protected]', first_name: 'Ezekiel', last_name: 'Jones'}
       }.to change{User.count}.by(1)
       expect(response.code).to eq('201')
@@ -265,7 +284,7 @@
     it 'creates a new user with first name, last name and full name if given' do
       expect {
         api_post :find_or_create,
-                 trusted_application_token,
+                 foc_trusted_application_token,
                  body: {
                    email: '[email protected]',
                    first_name: 'Sarah',
@@ -279,7 +298,7 @@
       expect(new_user.last_name).to eq 'Test'
       expect(new_user.full_name).to eq 'Sarah Test'
       expect(new_user.role).to eq 'instructor'
-      expect(new_user.applications).to eq [ trusted_application ]
+      expect(new_user.applications).to eq [ foc_trusted_application ]
       expect(new_user.uuid).not_to be_blank
     end
 
@@ -303,7 +322,7 @@
 
     context "should return only IDs for a user" do
       it "does so for unclaimed users" do
-        api_post :find_or_create, trusted_application_token,
+        api_post :find_or_create, foc_trusted_application_token,
                  body: {email: unclaimed_user.contact_infos.first.value}
         expect(response.code).to eq('201')
         expect(response.body_as_hash).to eq(
@@ -314,7 +333,7 @@
       end
       it "does so for claimed users" do
         api_post :find_or_create,
-                 trusted_application_token,
+                 foc_trusted_application_token,
                  body: {email: user_2.contact_infos.first.value}
         expect(response.code).to eq('201')
         expect(response.body_as_hash).to eq(

spec/handlers/messages_create_spec.rb

@@ -3,7 +3,7 @@
 describe MessagesCreate, type: :handler do
 
   let!(:trusted_application)   {
-    FactoryBot.create :doorkeeper_application, :trusted,
+    FactoryBot.create :doorkeeper_application, can_message_users: true,
                        email_from_address: '[email protected]'
   }
   let!(:trusted_application_token) { FactoryBot.create :doorkeeper_access_token,

spec/support/feature_helpers.rb

@@ -88,7 +88,9 @@ def link_in_last_email
 end
 
 def create_application(skip_terms: false)
-  app = FactoryBot.create(:doorkeeper_application, :trusted, skip_terms: skip_terms)
+  app = FactoryBot.create(:doorkeeper_application, skip_terms: skip_terms,
+                          can_access_private_user_data: true,
+                          can_skip_oauth_screen: true)
 
   # We want to provide a local "external" redirect uri so our specs aren't actually
   # making HTTP calls against real external URLs like "example.com"