Rethink permissions in CodeOcean for usage in schools and adopt views

app/controllers/application_controller.rb

@@ -33,6 +33,7 @@ class ApplicationController < ActionController::Base
   private :set_locale
 
   def welcome
+    # Show root page
   end
 
   def allow_iframe_requests

app/controllers/exercises_controller.rb

@@ -7,8 +7,8 @@ class ExercisesController < ApplicationController
 
   before_action :handle_file_uploads, only: [:create, :update]
   before_action :set_execution_environments, only: [:create, :edit, :new, :update]
-  before_action :set_exercise, only: MEMBER_ACTIONS + [:clone, :implement, :working_times, :intervention, :search, :run, :statistics, :submit, :reload, :feedback]
+  before_action :set_exercise_and_authorize, only: MEMBER_ACTIONS + [:clone, :implement, :working_times, :intervention, :search, :run, :statistics, :submit, :reload, :feedback]
-  before_action :set_external_user, only: [:statistics]
+  before_action :set_external_user_and_authorize, only: [:statistics]
   before_action :set_file_types, only: [:create, :edit, :new, :update]
   before_action :set_course_token, only: [:implement]
 
@@ -291,19 +291,19 @@ class ExercisesController < ApplicationController
   end
   private :set_execution_environments
 
-  def set_exercise
+  def set_exercise_and_authorize
     @exercise = Exercise.find(params[:id])
     authorize!
   end
-  private :set_exercise
+  private :set_exercise_and_authorize
 
-  def set_external_user
+  def set_external_user_and_authorize
     if params[:external_user_id]
       @external_user = ExternalUser.find(params[:external_user_id])
       authorize!
     end
   end
-  private :set_exercise
+  private :set_external_user_and_authorize
 
   def set_file_types
     @file_types = FileType.all.order(:name)
@@ -321,10 +321,11 @@ class ExercisesController < ApplicationController
   private :collect_set_and_unset_exercise_tags
 
   def show
+    # Show exercise details for teachers and admins
   end
 
-  #we might want to think about auth here
   def reload
+    # Returns JSON with original file content
   end
 
   def statistics

app/controllers/proxy_exercises_controller.rb

@@ -1,7 +1,7 @@
 class ProxyExercisesController < ApplicationController
   include CommonBehavior
 
-  before_action :set_exercise, only: MEMBER_ACTIONS + [:clone, :reload]
+  before_action :set_exercise_and_authorize, only: MEMBER_ACTIONS + [:clone, :reload]
 
   def authorize!
     authorize(@proxy_exercise || @proxy_exercises)
@@ -56,11 +56,11 @@ class ProxyExercisesController < ApplicationController
     authorize!
   end
 
-  def set_exercise
+  def set_exercise_and_authorize
     @proxy_exercise = ProxyExercise.find(params[:id])
     authorize!
   end
-  private :set_exercise
+  private :set_exercise_and_authorize
 
   def show
     @search = @proxy_exercise.exercises.search

app/policies/application_policy.rb

@@ -9,22 +9,28 @@ class ApplicationPolicy
   end
   private :teacher?
 
+  def author?
+    @user == @record.author
+  end
+  private :author?
+
   def everyone
+    # As the ApplicationController forces to have any authorization, `everyone` here means `every user logged in`
     true
   end
   private :everyone
 
+  def no_one
+    false
+  end
+  private :no_one
+
   def initialize(user, record)
     @user = user
     @record = record
     require_user!
   end
 
-  def no_one
-    false
-  end
-  private :no_one
-
   def require_user!
     fail Pundit::NotAuthorizedError unless @user
   end

app/policies/comment_policy.rb

@@ -1,9 +1,4 @@
 class CommentPolicy < ApplicationPolicy
-  def author?
-    @user == @record.author
-  end
-  private :author?
-
   def create?
     everyone
   end

app/policies/consumer_policy.rb

@@ -1,5 +1,3 @@
 class ConsumerPolicy < AdminOnlyPolicy
-  def show?
+
-    super || @user.consumer == @record
-  end
 end

app/policies/execution_environment_policy.rb

@@ -1,14 +1,9 @@
 class ExecutionEnvironmentPolicy < AdminOnlyPolicy
-  def author?
-    @user == @record.author
-  end
-  private :author?
-
   [:execute_command?, :shell?, :statistics?].each do |action|
     define_method(action) { admin? || author? }
   end
 
-  [:create?, :index?, :new?].each do |action|
+  [:show?, :index?, :new?].each do |action|
     define_method(action) { admin? || teacher? }
   end
 end

app/policies/exercise_policy.rb

@@ -1,9 +1,4 @@
 class ExercisePolicy < AdminOrAuthorPolicy
-  def author?
-    @user == @record.author
-  end
-  private :author?
-
   def batch_update?
     admin?
   end

app/policies/file_template_policy.rb

@@ -1,7 +1,11 @@
 class FileTemplatePolicy < AdminOnlyPolicy
 
+  def index?
+    admin? || teacher?
+  end
+
   def show?
-    everyone
+    admin? || teacher?
   end
 
   def by_file_type?

app/policies/file_type_policy.rb

@@ -1,10 +1,5 @@
 class FileTypePolicy < AdminOnlyPolicy
-  def author?
+  [:index?, :show?].each do |action|
-    @user == @record.author
-  end
-  private :author?
-
-  [:create?, :index?, :new?].each do |action|
     define_method(action) { admin? || teacher? }
   end
 

app/policies/intervention_policy.rb

@@ -1,9 +1,4 @@
 class InterventionPolicy < AdminOrAuthorPolicy
-  def author?
-    @user == @record.author
-  end
-  private :author?
-
   def batch_update?
     admin?
   end

app/policies/proxy_exercise_policy.rb

@@ -1,9 +1,4 @@
 class ProxyExercisePolicy < AdminOrAuthorPolicy
-  def author?
-    @user == @record.author
-  end
-  private :author?
-
   def batch_update?
     admin?
   end

app/policies/request_for_comment_policy.rb

@@ -1,9 +1,4 @@
 class RequestForCommentPolicy < ApplicationPolicy
-  def author?
-    @user == @record.author
-  end
-  private :author?
-
   def create?
     everyone
   end

app/policies/search_policy.rb

@@ -1,9 +1,4 @@
 class SearchPolicy < AdminOrAuthorPolicy
-  def author?
-    @user == @record.author
-  end
-  private :author?
-
   def batch_update?
     admin?
   end

app/policies/submission_policy.rb

@@ -1,9 +1,4 @@
 class SubmissionPolicy < ApplicationPolicy
-  def author?
-    @user == @record.author
-  end
-  private :author?
-
   def create?
     everyone
   end
@@ -16,4 +11,15 @@ class SubmissionPolicy < ApplicationPolicy
   def index?
     admin?
   end
+
+  def everyone_in_study_group
+    users_in_same_study_group = @record.study_groups.users
+    users_in_same_study_group.include? @user
+  end
+  private :everyone_in_study_group
+
+  def teacher_in_study_group
+    teacher? && everyone_in_study_group
+  end
+  private :teacher_in_study_group
 end

app/policies/tag_policy.rb

@@ -1,9 +1,4 @@
 class TagPolicy < AdminOrAuthorPolicy
-  def author?
-    @user == @record.author
-  end
-  private :author?
-
   def batch_update?
     admin?
   end

app/views/file_types/index.html.slim

@@ -11,12 +11,12 @@ h1 = FileType.model_name.human(count: 2)
     tbody
       - @file_types.each do |file_type|
         tr
-          td = link_to(file_type.name, file_type)
+          td = link_to_if(policy(file_type).show?, file_type.name, file_type)
-          td = link_to(file_type.author, file_type.author)
+          td = link_to_if(policy(file_type.author).show?, file_type.author, file_type.author)
           td = file_type.file_extension
-          td = link_to(t('shared.show'), file_type)
+          td = link_to(t('shared.show'), file_type) if policy(file_type).show?
-          td = link_to(t('shared.edit'), edit_file_type_path(file_type))
+          td = link_to(t('shared.edit'), edit_file_type_path(file_type)) if policy(file_type).edit?
-          td = link_to(t('shared.destroy'), file_type, data: {confirm: t('shared.confirm_destroy')}, method: :delete)
+          td = link_to(t('shared.destroy'), file_type, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(file_type).destroy?
 
 = render('shared/pagination', collection: @file_types)
 p = render('shared/new_button', model: FileType)