Merge pull request #2447 from somiaj/add-lti-config-items

Alex-Jordan · web-flow · commit bf11f9809098 · 2024-08-06T12:15:26.000-07:00
Add more configurable LTI variables to config page.
diff --git a/conf/authen_LTI.conf.dist b/conf/authen_LTI.conf.dist
@@ -169,19 +169,43 @@ $LTIMassUpdateInterval = 86400;    #in seconds
 # the tab will not be shown.  Note that the default values for the variables that will be shown
 # in the LTI tab are the values that are set above.  Further note that only the commented out
 # variables listed below may be added to the LTI config tab.  In addition, only the variables that
-# pertain to the active LTI version will be shown in the tab.
+# pertain to the active LTI version will be shown in the tab. Warning: Allowing users to modify
+# the BasicConsumerSecret for LTI 1.1 or the IDs, URLs, etc for LTI 1.3 can expose the values
+# of the variables and allow users to lock themselves out of logging in via an LMS.
 @LTIConfigVariables = (
 	#'LTI{v1p1}{LMS_name}',
 	#'LTI{v1p3}{LMS_name}',
 	#'LTI{v1p1}{LMS_url}',
 	#'LTI{v1p3}{LMS_url}',
 	#'external_auth',
 	#'LTIGradeMode',
+	#'LTIGradeOnSubmit',
+	#'LTIMassUpdateInterval',
 	#'LMSManageUserData',
+	#'LTI{v1p1}{BasicConsumerSecret}',
+	#'LTI{v1p3}{PlatformID}',
+	#'LTI{v1p3}{ClientID}',
+	#'LTI{v1p3}{DeploymentID}',
+	#'LTI{v1p3}{PublicKeysetURL}',
+	#'LTI{v1p3}{AccessTokenURL}',
+	#'LTI{v1p3}{AccessTokenAUD}',
+	#'LTI{v1p3}{AuthReqURL}',
 	#'debug_lti_parameters',
 	#'lms_context_id'
 );
 
+# By default only admin users can modify the LTI secrets and lms_context_id. The following
+# permissions need to be modified to allow other users the permission to modify the values.
+#$permissionLevels{'change_config_LTI{v1p1}{BasicConsumerSecret}'} = "admin",
+#$permissionLevels{'change_config_LTI{v1p3}{PlatformID}'}          = "admin",
+#$permissionLevels{'change_config_LTI{v1p3}{ClientID}'}            = "admin",
+#$permissionLevels{'change_config_LTI{v1p3}{DeploymentID}'}        = "admin",
+#$permissionLevels{'change_config_LTI{v1p3}{PublicKeysetURL}'}     = "admin",
+#$permissionLevels{'change_config_LTI{v1p3}{AccessTokenURL}'}      = "admin",
+#$permissionLevels{'change_config_LTI{v1p3}{AccessTokenAUD}'}      = "admin",
+#$permissionLevels{'change_config_LTI{v1p3}{AuthReqURL}'}          = "admin",
+#$permissionLevels{'change_config_lms_context_id'}                 = "admin",
+
 # Note that the lms_context_id is actually a database setting. It must be set for a course in
 # order for the instructor to utilize LTI content selection. This can also be set in the admin
 # course.
diff --git a/conf/defaults.config b/conf/defaults.config
@@ -874,7 +874,15 @@ $authen{admin_module} = ['WeBWorK::Authen::Basic_TheLastOption'];
 	# sufficient to change a configuration setting.
 
 	#change_config_courseTitle    => "admin",
-	change_config_lms_context_id => "admin",
+	change_config_lms_context_id                   => "admin",
+	'change_config_LTI{v1p1}{BasicConsumerSecret}' => "admin",
+	'change_config_LTI{v1p3}{PlatformID}'          => "admin",
+	'change_config_LTI{v1p3}{ClientID}'            => "admin",
+	'change_config_LTI{v1p3}{DeploymentID}'        => "admin",
+	'change_config_LTI{v1p3}{PublicKeysetURL}'     => "admin",
+	'change_config_LTI{v1p3}{AccessTokenURL}'      => "admin",
+	'change_config_LTI{v1p3}{AccessTokenAUD}'      => "admin",
+	'change_config_LTI{v1p3}{AuthReqURL}'          => "admin",
 
 	# Do not confuse the permission to change a configuration permission with
 	# the actual permission as in the following example. If this us uncommented,
diff --git a/lib/WeBWorK/ConfigObject.pm b/lib/WeBWorK/ConfigObject.pm
@@ -2,6 +2,7 @@ package WeBWorK::ConfigObject;
 use Mojo::Base -signatures;
 
 # Base object class for all config objects
+use constant SECRET_STRING => '*****';
 
 sub new ($class, $data, $c) {
 	return bless {
@@ -13,7 +14,7 @@ sub new ($class, $data, $c) {
 
 # Only input is a value to display, and should produce an html string.
 sub display_value ($self, $val) {
-	return $val;
+	return $self->{secret} ? SECRET_STRING : $val;
 }
 
 # This should return the value to compare to the new value.  This is *not* what is displayed.
@@ -47,16 +48,16 @@ sub convert_newval_source ($self, $use_current) {
 # will be the value of the perl variable, and newval will be whatever an entry widget produces.
 sub save_string ($self, $oldval, $use_current = 0) {
 	my $newval = $self->convert_newval_source($use_current);
-	return '' if $self->comparison_value($oldval) eq $newval;
+	return '' if $self->comparison_value($oldval) eq $newval || ($self->{secret} && $newval eq SECRET_STRING);
 
 	$newval =~ s/['"`]//g;
 	return "\$$self->{var} = '$newval';\n";
 }
 
 # A widget to interact with the user
-sub entry_widget ($self, $default) {
+sub entry_widget ($self, $default, $is_secret = 0) {
 	return $self->{c}->text_field(
-		$self->{name} => $default,
+		$self->{name} => $is_secret ? SECRET_STRING : $default,
 		id            => $self->{name},
 		size          => $self->{width} || 15,
 		class         => 'form-control form-control-sm'
diff --git a/lib/WeBWorK/ConfigObject/boolean.pm b/lib/WeBWorK/ConfigObject/boolean.pm
@@ -29,7 +29,7 @@ sub save_string ($self, $oldval, $use_current = 0) {
 	return "\$$self->{var} = $newval;\n";
 }
 
-sub entry_widget ($self, $default) {
+sub entry_widget ($self, $default, $is_secret = 0) {
 	return $self->{c}->select_field(
 		$self->{name} => [
 			[ $self->{c}->maketext('True')  => 1, $default == 1 ? (selected => undef) : () ],
diff --git a/lib/WeBWorK/ConfigObject/checkboxlist.pm b/lib/WeBWorK/ConfigObject/checkboxlist.pm
@@ -44,7 +44,7 @@ sub comparison_value ($self, $val) {
 	return join(',', @{ $val // [] });
 }
 
-sub entry_widget ($self, $default) {
+sub entry_widget ($self, $default, $is_secret = 0) {
 	my $c = $self->{c};
 	return $c->c(
 		map {
diff --git a/lib/WeBWorK/ConfigObject/list.pm b/lib/WeBWorK/ConfigObject/list.pm
@@ -43,7 +43,7 @@ sub save_string ($self, $oldval, $use_current = 0) {
 	return "\$$self->{var} = [" . join(',', map {"'$_'"} map { $_ =~ s/['"`]//gr } split(',', $newval)) . "];\n";
 }
 
-sub entry_widget ($self, $default) {
+sub entry_widget ($self, $default, $is_secret = 0) {
 	my $str = join(', ', @{ $default // [] });
 	return $self->{c}->text_area(
 		$self->{name} => $str =~ /\S/ ? $str : '',
diff --git a/lib/WeBWorK/ConfigObject/lms_context_id.pm b/lib/WeBWorK/ConfigObject/lms_context_id.pm
@@ -101,7 +101,7 @@ sub save_string ($self, $oldval, $use_current = 0) {
 
 # This ensures that the input for this setting always shows what is in the database. If the form is submitted, and the
 # requested context id is rejected above, then that rejected value should not be shown when the page reloads.
-sub entry_widget ($self, $default) {
+sub entry_widget ($self, $default, $is_secret = 0) {
 	$self->{c}->param($self->{name}, $default);
 	return $self->SUPER::entry_widget($default);
 }
diff --git a/lib/WeBWorK/ConfigObject/permission.pm b/lib/WeBWorK/ConfigObject/permission.pm
@@ -34,7 +34,7 @@ sub save_string ($self, $oldval, $use_current = 0) {
 	return "\$$self->{var} = '$newval';\n";
 }
 
-sub entry_widget ($self, $default) {
+sub entry_widget ($self, $default, $is_secret = 0) {
 	my $c = $self->{c};
 
 	# The value of a permission can be undefined (for nobody), a standard permission number, or some other number
diff --git a/lib/WeBWorK/ConfigObject/permission_checkboxlist.pm b/lib/WeBWorK/ConfigObject/permission_checkboxlist.pm
@@ -51,7 +51,7 @@ sub comparison_value ($self, $val) {
 	return join(',', @{ $val // [] });
 }
 
-sub entry_widget ($self, $default) {
+sub entry_widget ($self, $default, $is_secret = 0) {
 	my $c = $self->{c};
 	$default = role_and_above($self->{c}->ce->{userRoles}, $default) unless ref($default) eq 'ARRAY';
 	return $c->c(
diff --git a/lib/WeBWorK/ConfigObject/popuplist.pm b/lib/WeBWorK/ConfigObject/popuplist.pm
@@ -29,7 +29,7 @@ sub save_string ($self, $oldval, $use_current = 0) {
 	return ("\$$self->{var} = '$newval';\n");
 }
 
-sub entry_widget ($self, $default) {
+sub entry_widget ($self, $default, $is_secret = 0) {
 	my $c = $self->{c};
 	return $c->select_field(
 		$self->{name} => [
diff --git a/lib/WeBWorK/ConfigValues.pm b/lib/WeBWorK/ConfigValues.pm
@@ -899,6 +899,25 @@ sub getConfigValues ($ce) {
 			labels => { '' => 'None', 'course' => 'Course', 'homework' => 'Homework' },
 			type   => 'popuplist'
 		},
+		LTIGradeOnSubmit => {
+			var  => 'LTIGradeOnSubmit',
+			doc  => x('Update LMS Grade Each Submit'),
+			doc2 => x(
+				'Sets if webwork sends grades back to the LMS every time a user submits an answer. '
+					. 'This keeps students grades up to date, but can cause additional server load.'
+			),
+			type => 'boolean'
+		},
+		LTIMassUpdateInterval => {
+			var  => 'LTIMassUpdateInterval',
+			doc  => x('Time in seconds to periodically update LMS grades (-1 to disable)'),
+			doc2 => x(
+				'Sets the time in seconds to periodically update the LMS grades. WeBWorK will update all grades on '
+					. 'the LMS if it has been longer than this time since the completion of the last update. This is '
+					. 'only an approximate time. 86400 seconds is one day. -1 disables periodic updates.'
+			),
+			type => 'number'
+		},
 		LMSManageUserData => {
 			var  => 'LMSManageUserData',
 			doc  => x('Allow the LMS to update user account data'),
@@ -911,6 +930,65 @@ sub getConfigValues ($ce) {
 			),
 			type => 'boolean'
 		},
+		'LTI{v1p1}{BasicConsumerSecret}' => {
+			var  => 'LTI{v1p1}{BasicConsumerSecret}',
+			doc  => x('LMS shared secret for LTI 1.1 authentication'),
+			doc2 => x(
+				'This secret word is used to validate logins from an LMS using LTI 1.1. '
+					. 'This secret word must match the word configured in the LMS.'
+			),
+			type   => 'text',
+			secret => 1
+		},
+		'LTI{v1p3}{PlatfromID}' => {
+			var    => 'LTI{v1p3}{PlatformID}',
+			doc    => x('LMS platform ID for LTI 1.3'),
+			doc2   => x('LMS platform ID used to validate logins from an LMS using LTI 1.3.'),
+			type   => 'text',
+			secret => 1
+		},
+		'LTI{v1p3}{ClientID}' => {
+			var    => 'LTI{v1p3}{ClientID}',
+			doc    => x('LMS client ID for LTI 1.3'),
+			doc2   => x('LMS client ID used to validate logins from an LMS using LTI 1.3.'),
+			type   => 'text',
+			secret => 1
+		},
+		'LTI{v1p3}{DeploymentID}' => {
+			var    => 'LTI{v1p3}{DeploymentID}',
+			doc    => x('LMS deployment ID for LTI 1.3'),
+			doc2   => x('LMS deployment ID used to validate logins from an LMS using LTI 1.3.'),
+			type   => 'text',
+			secret => 1
+		},
+		'LTI{v1p3}{PublicKeysetURL}' => {
+			var    => 'LTI{v1p3}{PublicKeysetURL}',
+			doc    => x('LMS public keyset URL for LTI 1.3'),
+			doc2   => x('LMS public keyset URL used to validate logins from an LMS using LTI 1.3.'),
+			type   => 'text',
+			secret => 1
+		},
+		'LTI{v1p3}{AccessTokenURL}' => {
+			var    => 'LTI{v1p3}{AccessTokenURL}',
+			doc    => x('LMS access token URL for LTI 1.3'),
+			doc2   => x('LMS access token URL used to validate logins from an LMS using LTI 1.3.'),
+			type   => 'text',
+			secret => 1
+		},
+		'LTI{v1p3}{AccessTokenAUD}' => {
+			var    => 'LTI{v1p3}{AccessTokenAUD}',
+			doc    => x('LMS access token AUD for LTI 1.3'),
+			doc2   => x('LMS access token AUD used to validate logins from an LMS using LTI 1.3.'),
+			type   => 'text',
+			secret => 1
+		},
+		'LTI{v1p3}{AuthReqURL}' => {
+			var    => 'LTI{v1p3}{AuthReqURL}',
+			doc    => x('LMS authorization request URL for LTI 1.3'),
+			doc2   => x('LMS authorization request URL used to validate logins from an LMS using LTI 1.3.'),
+			type   => 'text',
+			secret => 1
+		},
 		debug_lti_parameters => {
 			var  => 'debug_lti_parameters',
 			doc  => x('Show LTI parameters (for debugging)'),
@@ -997,6 +1075,15 @@ sub getConfigValues ($ce) {
 				{ %{ delete $LTIConfigValues->{'LTI{v1p1}{LMS_name}'} }, var => 'LTI{v1p3}{LMS_name}' };
 			$LTIConfigValues->{'LTI{v1p3}{LMS_url}'} =
 				{ %{ delete $LTIConfigValues->{'LTI{v1p1}{LMS_url}'} }, var => 'LTI{v1p3}{LMS_url}' };
+			delete $LTIConfigValues->{'LTI{v1p1}{BasicConsumerSecret}'};
+		} else {
+			for my $key (
+				'PlatformID',     'ClientID',       'DeploymentID', 'PublicKeysetURL',
+				'AccessTokenURL', 'AccessTokenAUD', 'AuthReqURL'
+				)
+			{
+				delete $LTIConfigValues->{"LTI{v1p3}{$key}"};
+			}
 		}
 
 		push(
diff --git a/templates/ContentGenerator/Instructor/Config.html.ep b/templates/ContentGenerator/Instructor/Config.html.ep
@@ -56,12 +56,16 @@
 								% defined $ce->{permissionLevels}{"change_config_$conobject->{var}"}
 								% && !$authz->hasPermissions(param('user'), "change_config_$conobject->{var}")
 							% );
+							% # Hide sensitive variables from being displayed.
+							% my $default_value = $conobject->get_value($default_ce);
+							% my $current_value = $conobject->get_value($ce3);
+							% my $is_secret = $conobject->{secret} && $default_value eq $current_value ? 1 : 0;
 							<tr>
 								<td><%= $conobject->what_string %></td>
 								<td class="text-center">
 									<%= $conobject->display_value($conobject->get_value($default_ce)) %>
 								</td>
-								<td><%= $conobject->entry_widget($conobject->get_value($ce3)) =%></td>
+								<td><%= $conobject->entry_widget($conobject->get_value($ce3), $is_secret) =%></td>
 							</tr>
 						% }
 					</table>

Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ sub save_string ($self, $oldval, $use_current = 0) {`
`29`	`29`	`return "\$$self->{var} = $newval;\n";`
`30`	`30`	`}`
`31`	`31`
`32`		`-sub entry_widget ($self, $default) {`
	`32`	`+sub entry_widget ($self, $default, $is_secret = 0) {`
`33`	`33`	`return $self->{c}->select_field(`
`34`	`34`	`$self->{name} => [`
`35`	`35`	`[ $self->{c}->maketext('True') => 1, $default == 1 ? (selected => undef) : () ],`
Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ sub comparison_value ($self, $val) {`
`44`	`44`	`return join(',', @{ $val // [] });`
`45`	`45`	`}`
`46`	`46`
`47`		`-sub entry_widget ($self, $default) {`
	`47`	`+sub entry_widget ($self, $default, $is_secret = 0) {`
`48`	`48`	`my $c = $self->{c};`
`49`	`49`	`return $c->c(`
`50`	`50`	`map {`
Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ sub save_string ($self, $oldval, $use_current = 0) {`
`43`	`43`	return "\$$self->{var} = [" . join(',', map {"'$_'"} map { $_ =~ s/['"`]//gr } split(',', $newval)) . "];\n";
`44`	`44`	`}`
`45`	`45`
`46`		`-sub entry_widget ($self, $default) {`
	`46`	`+sub entry_widget ($self, $default, $is_secret = 0) {`
`47`	`47`	`my $str = join(', ', @{ $default // [] });`
`48`	`48`	`return $self->{c}->text_area(`
`49`	`49`	`$self->{name} => $str =~ /\S/ ? $str : '',`
Original file line number	Diff line number	Diff line change
`@@ -101,7 +101,7 @@ sub save_string ($self, $oldval, $use_current = 0) {`
`101`	`101`
`102`	`102`	`# This ensures that the input for this setting always shows what is in the database. If the form is submitted, and the`
`103`	`103`	`# requested context id is rejected above, then that rejected value should not be shown when the page reloads.`
`104`		`-sub entry_widget ($self, $default) {`
	`104`	`+sub entry_widget ($self, $default, $is_secret = 0) {`
`105`	`105`	`$self->{c}->param($self->{name}, $default);`
`106`	`106`	`return $self->SUPER::entry_widget($default);`
`107`	`107`	`}`
Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,7 @@ sub save_string ($self, $oldval, $use_current = 0) {`
`34`	`34`	`return "\$$self->{var} = '$newval';\n";`
`35`	`35`	`}`
`36`	`36`
`37`		`-sub entry_widget ($self, $default) {`
	`37`	`+sub entry_widget ($self, $default, $is_secret = 0) {`
`38`	`38`	`my $c = $self->{c};`
`39`	`39`
`40`	`40`	`# The value of a permission can be undefined (for nobody), a standard permission number, or some other number`
Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,7 @@ sub comparison_value ($self, $val) {`
`51`	`51`	`return join(',', @{ $val // [] });`
`52`	`52`	`}`
`53`	`53`
`54`		`-sub entry_widget ($self, $default) {`
	`54`	`+sub entry_widget ($self, $default, $is_secret = 0) {`
`55`	`55`	`my $c = $self->{c};`
`56`	`56`	`$default = role_and_above($self->{c}->ce->{userRoles}, $default) unless ref($default) eq 'ARRAY';`
`57`	`57`	`return $c->c(`