Skip to content

Commit

Permalink
LTI overhaul to support Sakai 22.x
Browse files Browse the repository at this point in the history
* Change var to const for js constants being injected into the page from the server
* Allows the before_play_start event to determine if the lti content item picker should be displayed
* Move picker display function into a common controller
* Add an api method that’ll validate and sign a content item selection lti request
* injects LTI_MESSAGE_TYPE and LTI_KEY js constants into the picker page template
* LTILaunch gains a method to load the lti config that is associated with a lti key
* Updated a few strings that specifically mention canvas
* Moves all classes, tests, migrations, and configs from fuel/app/modules/lti into fuel/app
* overhaul lti test provider to simplify
* simplify lti launch picker code when sending content selection messages to lti/assignment
* fixed issue with starting plays with referrer urls longer than 255 characters fixes #1418
* updates oauth validation in fuel by ensuring the request uri is not masked by FuelPHP
* Overhaul lit test provider a bit - mostly to simplify and use lessons
* fix code sniff errors
* add deps to docker image
* attempt to fix yarn cache issue
* Changes docker build process to ensure the built image has admin:make_paths_writable run to avoid having to run it every startup
  • Loading branch information
iturgeon committed Dec 31, 2022
1 parent 45efbf6 commit 702747b
Show file tree
Hide file tree
Showing 50 changed files with 1,035 additions and 976 deletions.
2 changes: 1 addition & 1 deletion docker/run_build_assets.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,4 @@ docker run \
--mount type=bind,source="$(pwd)"/../,target=/build \
--mount source=materia-asset-build-vol,target=/build/node_modules \
node:12.11.1-alpine \
/bin/ash -c "apk add --no-cache git && cd build && yarn install --frozen-lockfile --non-interactive --production --silent --pure-lockfile --force"
/bin/ash -c "apk add --no-cache git && cd build && yarn install --frozen-lockfile --non-interactive --production --silent --pure-lockfile --force --check-files --cache-folder .ycache && rm -rf .ycache"
2 changes: 1 addition & 1 deletion docker/run_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ DCTEST="docker-compose -f docker-compose.yml -f docker-compose.override.test.yml
set -e
set -o xtrace

$DCTEST run --rm app /wait-for-it.sh mysql:3306 -t 20 -- composer run testci -- "$@"
$DCTEST run -T --rm app /wait-for-it.sh mysql:3306 -t 20 -- composer run testci -- "$@"
2 changes: 1 addition & 1 deletion docker/run_tests_lint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ DCTEST="docker-compose -f docker-compose.yml -f docker-compose.override.test.yml
set -e
set -o xtrace

$DCTEST run --rm --no-deps app composer sniff-ci
$DCTEST run -T --rm --no-deps app composer sniff-ci
45 changes: 36 additions & 9 deletions fuel/app/classes/basetest.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ protected function tearDown(): void
}
}

protected static function remove_all_roles_for_user($user_id)
{
\DB::delete('perm_role_to_user')
->where('user_id', $user_id)
->execute();
}

protected static function clear_fuel_input()
{
// reset fuelphp's input class
Expand Down Expand Up @@ -196,6 +203,10 @@ protected function _as_student()
\Fuel\Tasks\Admin::new_user($uname, 'test', 'd', 'student', '[email protected]', $pword);
$user = \Model_User::find_by_username($uname);
}
else
{
static::remove_all_roles_for_user($user->id);
}

$login = \Service_User::login($uname, $pword);
$this->assertTrue($login);
Expand All @@ -215,13 +226,16 @@ protected function _as_author()
{
require_once(APPPATH.'/tasks/admin.php');
\Fuel\Tasks\Admin::new_user($uname, 'Prof', 'd', 'Author', '[email protected]', $pword);
\Fuel\Tasks\Admin::give_user_role($uname, 'basic_author');
$user = \Model_User::find_by_username($uname);
}
else
{
static::remove_all_roles_for_user($user->id);
}

\Materia\Perm_Manager::add_users_to_roles_system_only([$user->id], [\Materia\Perm_Role::AUTHOR]);
$login = \Service_User::login($uname, $pword);
$this->assertTrue($login);

$this->users_to_clean[] = $user;
return $user;
}
Expand All @@ -238,10 +252,14 @@ protected function _as_author_2()
{
require_once(APPPATH.'/tasks/admin.php');
\Fuel\Tasks\Admin::new_user($uname, 'test', 'd', 'author', '[email protected]', $pword);
\Fuel\Tasks\Admin::give_user_role($uname, 'basic_author');
$user = \Model_User::find_by_username($uname);
}
else
{
static::remove_all_roles_for_user($user->id);
}

\Materia\Perm_Manager::add_users_to_roles_system_only([$user->id], [\Materia\Perm_Role::AUTHOR]);
$login = \Service_User::login($uname, $pword);
$this->assertTrue($login);
$this->users_to_clean[] = $user;
Expand All @@ -260,10 +278,14 @@ protected function _as_author_3()
{
require_once(APPPATH.'/tasks/admin.php');
\Fuel\Tasks\Admin::new_user($uname, 'test', 'd', 'author', '[email protected]', $pword);
\Fuel\Tasks\Admin::give_user_role($uname, 'basic_author');
$user = \Model_User::find_by_username($uname);
}
else
{
static::remove_all_roles_for_user($user->id);
}

\Materia\Perm_Manager::add_users_to_roles_system_only([$user->id], [\Materia\Perm_Role::AUTHOR]);
$login = \Service_User::login($uname, $pword);
$this->assertTrue($login);
$this->users_to_clean[] = $user;
Expand All @@ -282,12 +304,14 @@ protected function _as_super_user()
{
require_once(APPPATH.'/tasks/admin.php');
\Fuel\Tasks\Admin::new_user($uname, 'test', 'd', 'su', '[email protected]', $pword);
// TODO: super_user should get all these rights inherently right??????!!!!
\Fuel\Tasks\Admin::give_user_role($uname, 'super_user');
\Fuel\Tasks\Admin::give_user_role($uname, 'basic_author');
$user = \Model_User::find_by_username($uname);
}
else
{
static::remove_all_roles_for_user($user->id);
}

\Materia\Perm_Manager::add_users_to_roles_system_only([$user->id], [\Materia\Perm_Role::AUTHOR, \Materia\Perm_Role::SU]);
$login = \Service_User::login($uname, $pword);
$this->assertTrue($login);
$this->users_to_clean[] = $user;
Expand All @@ -305,11 +329,14 @@ protected function _as_noauth()
{
require_once(APPPATH.'/tasks/admin.php');
\Fuel\Tasks\Admin::new_user($uname, 'test', 'd', 'noauth', '[email protected]', $pword);
// TODO: super_user should get all these rights inherently right??????!!!!
\Fuel\Tasks\Admin::give_user_role($uname, 'no_author');
$user = \Model_User::find_by_username($uname);
}
else
{
static::remove_all_roles_for_user($user->id);
}

\Materia\Perm_Manager::add_users_to_roles_system_only([$user->id], [\Materia\Perm_Role::NOAUTH]);
$login = \Service_User::login($uname, $pword);
$this->assertTrue($login);
$this->users_to_clean[] = $user;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
* License outlined in licenses folder
*/

namespace Lti;

class Controller_Lti extends \Controller
{
use \Trait_Analytics;
Expand All @@ -20,7 +18,7 @@ public function before()
*/
public function action_index()
{
$cfg = \Config::get('lti::lti.consumers.default');
$cfg = \Config::get('lti.consumers.default');
// TODO: this is hard coded for Canvas, figure out if the request carries any info we can use to figure this out
$this->theme->set_template('partials/config_xml');
$this->theme->get_template()
Expand Down Expand Up @@ -58,8 +56,8 @@ public function action_login()
$this->theme->set_partial('content', 'partials/post_login');
$this->insert_analytics();

\Js::push_inline('var BASE_URL = "'.\Uri::base().'";');
\Js::push_inline('var STATIC_CROSSDOMAIN = "'.\Config::get('materia.urls.static').'";');
\Js::push_inline('const BASE_URL = "'.\Uri::base().'";');
\Js::push_inline('const STATIC_CROSSDOMAIN = "'.\Config::get('materia.urls.static').'";');

\Css::push_group('core');

Expand All @@ -77,8 +75,10 @@ public function action_picker(bool $authenticate = true)
$launch = LtiLaunch::from_request();
if ($authenticate && ! LtiUserManager::authenticate($launch)) return \Response::redirect('/lti/error/unknown_user');

$system = ucfirst(\Input::post('tool_consumer_info_product_family_code', 'this system'));
$is_selector_mode = \Input::post('selection_directive') === 'select_link' || \Input::post('lti_message_type') === 'ContentItemSelectionRequest';
$system = \Input::post('tool_consumer_info_product_family_code', 'this system');
$lti_message_type = \Input::post('lti_message_type', 'none');
$lti_key = \Input::post('oauth_consumer_key', '');
$is_selector_mode = \Input::post('selection_directive') === 'select_link' || $lti_message_type === 'ContentItemSelectionRequest';
$return_url = \Input::post('launch_presentation_return_url') ?? \Input::post('content_item_return_url');

\Materia\Log::profile(['action_picker', \Input::post('selection_directive'), $system, $is_selector_mode ? 'yes' : 'no', $return_url], 'lti');
Expand All @@ -89,17 +89,18 @@ public function action_picker(bool $authenticate = true)
\Js::push_inline('var BASE_URL = "'.\Uri::base().'";');
\Js::push_inline('var WIDGET_URL = "'.\Config::get('materia.urls.engines').'";');
\Js::push_inline('var STATIC_CROSSDOMAIN = "'.\Config::get('materia.urls.static').'";');
\Js::push_inline($this->theme->view('partials/select_item_js')
->set('system', $system));
\Css::push_group(['core', 'lti']);

\Js::push_inline('var LTI_MESSAGE_TYPE = "'.$lti_message_type.'"');
\Js::push_inline('var system = "'.htmlentities($system).'"');
\Js::push_inline('const LTI_KEY = "'.$lti_key.'"');
if ($is_selector_mode && ! empty($return_url))
{
\Js::push_inline('var RETURN_URL = "'.$return_url.'"');
}

\Css::push_group(['core', 'lti']);

$this->theme->get_template()
->set('title', 'Select a Widget for Use in '.$system)
->set('title', 'Select a Widget for Use in '.ucfirst($system))
->set('page_type', 'lti-select');

$this->theme->set_partial('content', 'partials/select_item');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@
* License outlined in licenses folder
*/

namespace Lti;

class Controller_Error extends \Controller
class Controller_Lti_Error extends \Controller
{
use \Trait_Analytics;
protected $_content_partial = 'partials/error_general';
Expand Down
Loading

0 comments on commit 702747b

Please sign in to comment.