disallow sis enrollment roles as custom role names

to avoid ambiguity, don't allow custom roles named 'student',
'teacher', 'ta', 'observer', or 'designer'

also fix the error message from the api not to assume that
the base role type is invalid

test plan:
 - using the api, try to create a custom role with one of the
   above names
   -> it should tell you the name is reserved
 - also, try to create a custom role with a valid name
   but an invalid base role type
   -> it should tell you the base role type is invalid

Change-Id: Ic7f91a56e79d2929a64780115553ed38aec5f592
Reviewed-on: https://gerrit.instructure.com/15978
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Cody Cutrer <cody@instructure.com>
QA-Review: Adam Phillipps <adam@instructure.com>

app/controllers/role_overrides_controller.rb

@@ -242,7 +242,7 @@ class RoleOverridesController < ApplicationController
     role.workflow_state = 'active'
     role.deleted_at = nil
     if !role.save
-      render :json => {:message => "invalid base role type"}, :status => :bad_request
+      render :json => { :message => role.errors.full_messages.to_sentence }, :status => :bad_request
       return
     end
     # remove old role overrides that were associated with this role name

app/models/role.rb

@@ -22,8 +22,8 @@ class Role < ActiveRecord::Base
   attr_accessible :name
   before_validation :infer_root_account_id
   validates_presence_of :name
-  validates_inclusion_of :base_role_type, :in => RoleOverride::BASE_ROLE_TYPES
-  validates_exclusion_of :name, :in => RoleOverride::KNOWN_ROLE_TYPES
+  validates_inclusion_of :base_role_type, :in => RoleOverride::BASE_ROLE_TYPES, :message => 'is invalid'
+  validates_exclusion_of :name, :in => RoleOverride::KNOWN_ROLE_TYPES + Enrollment::SIS_TYPES.values
   validates_uniqueness_of :name, :scope => :account_id
   validate :ensure_no_name_conflict_with_different_base_role_type
 

spec/apis/v1/roles_api_spec.rb

@@ -240,7 +240,15 @@ describe "Roles API", :type => :integration do
         { :controller => 'role_overrides', :action => 'add_role', :format => 'json', :account_id => @admin.account.id.to_s },
         { :role => @role, :base_role_type => "notagoodbaserole" })
       response.status.should == '400 Bad Request'
-      JSON.parse(response.body).should == {"message" => "invalid base role type"}
+      JSON.parse(response.body).should == {"message" => "Base role type is invalid"}
+    end
+
+    it "should fail for a course role with a reserved name" do
+      raw_api_call(:post, "/api/v1/accounts/#{@admin.account.id}/roles",
+                   { :controller => 'role_overrides', :action => 'add_role', :format => 'json', :account_id => @admin.account.id.to_s },
+                   { :role => 'student', :base_role_type => "StudentEnrollment" })
+      response.status.should == '400 Bad Request'
+      JSON.parse(response.body).should == {"message" => "Name is reserved"}
     end
 
     it "should not create an override for course role for account-only permissions" do

spec/models/role_spec.rb

@@ -98,6 +98,29 @@ describe Role do
       role.should be_valid
     end
 
+    it "should disallow names that match base sis enrollment role names" do
+      role = @account.roles.create
+      role.base_role_type = 'StudentEnrollment'
+
+      role.name = 'student'
+      role.should_not be_valid
+
+      role.name = 'teacher'
+      role.should_not be_valid
+
+      role.name = 'ta'
+      role.should_not be_valid
+
+      role.name = 'designer'
+      role.should_not be_valid
+
+      role.name = 'observer'
+      role.should_not be_valid
+
+      role.name = 'cheater'
+      role.should be_valid
+    end
+
     it "should infer the root account id" do
       role = create_role('StudentEnrollment', "1337 Student")
       role.root_account_id.should == @account.id