Skip to content

Commit

Permalink
XWIKI-20851: Validate CSRF token before running job scheduling actions
Browse files Browse the repository at this point in the history
* Add a mandatory CSRF token for job management
* Add a page test for Scheduler.WebHome that checks for the proper use of the CSRF token
* Refactor scheduling actions URL generation
* Refactor Scheduler.WebHome to improve escaping

(cherry picked from commit f16ca4e)
  • Loading branch information
pjeanjean authored and michitux committed Oct 24, 2023
1 parent fa6846c commit f30d9c6
Show file tree
Hide file tree
Showing 8 changed files with 300 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2889,6 +2889,7 @@ xe.scheduler.job.scriptexplanation=The script is the code that will be executed
xe.scheduler.job.backtolist=Back to the job list
xe.scheduler.job.object=This sheet must be applied to a page that holds a scheduler job object.
xe.scheduler.updateJobClassComment=Created/Updated Scheduler Job Class definition
xe.scheduler.invalidToken=Invalid token, please try again by clicking on the desired action below.

### Statistics application
xe.statistics.activity=Activity Statistics
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,11 @@ void verifyScheduler(TestUtils setup)
// because we want to remain on the same page in case of a test failure so that our TestDebugger rule can
// collect accurate information about the failure. It's not a problem if the job remains scheduled because it
// does nothing. Other tests should not rely on the number of scheduler jobs though.
setup.deletePage("Scheduler", "SchedulerTestJob");
setup.deletePage("Scheduler", "Scheduler]]TestJob");

// Create Job
SchedulerHomePage schedulerHomePage = SchedulerHomePage.gotoPage();
schedulerHomePage.setJobName("SchedulerTestJob");
schedulerHomePage.setJobName("Scheduler]]TestJob");
SchedulerEditPage schedulerEdit = schedulerHomePage.clickAdd();

String jobName = "Tester problem";
Expand Down Expand Up @@ -107,7 +107,7 @@ void verifyScheduler(TestUtils setup)
assertFalse(setup.getDriver().hasElementWithoutWaiting(By.linkText(jobName)));
// Note: since the page doesn't exist, we need to disable the space redirect feature so that we end up on the
// terminal page that was removed.
setup.gotoPage("Scheduler", "SchedulerTestJob", "view", "spaceRedirect=false");
setup.gotoPage("Scheduler", "Scheduler]]TestJob", "view", "spaceRedirect=false");
setup.getDriver().findElement(By.linkText("Restore")).click();
schedulerPage = new SchedulerPage();
schedulerPage.backToHome();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,27 @@
<version>${project.version}</version>
<scope>runtime</scope>
</dependency>
<!-- Test dependencies. -->
<dependency>
<groupId>org.xwiki.platform</groupId>
<artifactId>xwiki-platform-test-page</artifactId>
<version>${project.version}</version>
<scope>test</scope>
</dependency>
<!-- Provides the component list for RenderingScriptService. -->
<dependency>
<groupId>org.xwiki.platform</groupId>
<artifactId>xwiki-platform-rendering-xwiki</artifactId>
<version>${project.version}</version>
<type>test-jar</type>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.xwiki.platform</groupId>
<artifactId>xwiki-platform-rendering-configuration-default</artifactId>
<version>${project.version}</version>
<type>test-jar</type>
<scope>test</scope>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,4 @@
<hidden>true</hidden>
<content>scheduler.applicationsPanelEntryLabel=Planer
</content>
</xwikidoc>
</xwikidoc>
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,4 @@
<hidden>true</hidden>
<content>scheduler.applicationsPanelEntryLabel=Планировщик
</content>
</xwikidoc>
</xwikidoc>
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,4 @@
<hidden>true</hidden>
<content>scheduler.applicationsPanelEntryLabel=Планувальник
</content>
</xwikidoc>
</xwikidoc>
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
-->

<xwikidoc version="1.1">
<xwikidoc version="1.5" reference="Scheduler.WebHome" locale="">
<web>Scheduler</web>
<name>WebHome</name>
<language/>
Expand Down Expand Up @@ -51,7 +51,13 @@
#set ($tJobHolder = $request.which)
#set ($jobDoc = $xwiki.getDocument($tJobHolder))
#set ($jobObj = $jobDoc.getObject('XWiki.SchedulerJobClass'))
#if ($request.do == 'schedule')
#if (!$services.csrf.isTokenValid($request.form_token))
##
## Check that the CSRF token matches the user before any operation
##
{{error}}$services.localization.render('xe.scheduler.invalidToken'){{/error}}

#elseif ($request.do == 'schedule')
##
## Schedule a job
##
Expand Down Expand Up @@ -143,7 +149,6 @@ $services.localization.render('xe.scheduler.welcome')
##
|=(%scope="col"%)$services.localization.render('xe.scheduler.jobs.name')|=(%scope="col"%)$services.localization.render('xe.scheduler.jobs.status')|=(%scope="col"%)$services.localization.render('xe.scheduler.jobs.next')|=(%scope="col"%)$services.localization.render('xe.scheduler.jobs.actions')
#foreach ($docName in $services.query.xwql('from doc.object(XWiki.SchedulerJobClass) as jobs where doc.fullName &lt;&gt; ''XWiki.SchedulerJobTemplate''').execute())
#set ($actions = {})
#set ($jobHolder = $xwiki.getDocument($docName))
#set ($job = $jobHolder.getObject('XWiki.SchedulerJobClass'))
#set ($status = $scheduler.getJobStatus($job).value)
Expand All @@ -156,18 +161,16 @@ $services.localization.render('xe.scheduler.welcome')
#else
#set ($firetime = $services.localization.render('xe.scheduler.jobs.next.undefined'))
#end
#set ($ok = $!actions.put('trigger', $doc.getURL('view', "do=trigger&amp;which=${jobHolder.fullName}")))
#set ($actions = ['trigger'])
#if ($status == 'None')
#set ($ok = $!actions.put('schedule', $doc.getURL('view', "do=schedule&amp;which=${jobHolder.fullName}")))
#set ($ok = $actions.add('schedule'))
#elseif($status == 'Normal')
#set ($ok = $!actions.put('pause', $doc.getURL('view', "do=pause&amp;which=${jobHolder.fullName}")))
#set ($ok = $!actions.put('unschedule', $doc.getURL('view', "do=unschedule&amp;which=${jobHolder.fullName}")))
#set ($ok = $actions.addAll(['pause', 'unschedule']))
#elseif ($status == 'Paused')
#set ($ok = $!actions.put('resume', $doc.getURL('view', "do=resume&amp;which=${jobHolder.fullName}")))
#set ($ok = $!actions.put('unschedule', $doc.getURL('view', "do=unschedule&amp;which=${jobHolder.fullName}")))
#set ($ok = $actions.addAll(['resume', 'unschedule']))
#end
#set ($ok = $!actions.put('delete', $doc.getURL('view', "do=delete&amp;which=${jobHolder.fullName}")))
|$job.get('jobName')|$status|$firetime|**$services.localization.render('xe.scheduler.jobs.actions.access')** [[$services.localization.render('xe.scheduler.jobs.actions.view')&gt;&gt;$jobHolder.fullName]]#if($jobHolder.hasAccessLevel('programming')) [[$services.localization.render('xe.scheduler.jobs.actions.edit')&gt;&gt;path:${jobHolder.getURL('edit')}]]#end **$services.localization.render('xe.scheduler.jobs.actions.manage')**#foreach($action in $actions.entrySet()) [[$services.localization.render("xe.scheduler.jobs.actions.${action.key}")&gt;&gt;path:${action.value}]]#end
#set ($ok = $actions.add('delete'))
|$job.get('jobName')|$status|$firetime|**$services.localization.render('xe.scheduler.jobs.actions.access')** [[$services.localization.render('xe.scheduler.jobs.actions.view')&gt;&gt;$services.rendering.escape($jobHolder.fullName, 'xwiki/2.1')]]#if($jobHolder.hasAccessLevel('programming')) [[$services.localization.render('xe.scheduler.jobs.actions.edit')&gt;&gt;path:${jobHolder.getURL('edit')}]]#end **$services.localization.render('xe.scheduler.jobs.actions.manage')**#foreach($action in $actions) [[$services.localization.render("xe.scheduler.jobs.actions.$action")&gt;&gt;path:$doc.getURL('view', $escapetool.url({'do': $action, 'which': $jobHolder.fullName, 'form_token': $services.csrf.token}))]]#end

#end
#if ($doc.hasAccessLevel('programming'))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,257 @@
/*
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
*
* This is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as
* published by the Free Software Foundation; either version 2.1 of
* the License, or (at your option) any later version.
*
* This software is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this software; if not, write to the Free
* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
*/
package org.xwiki.scheduler.ui;

import java.util.List;
import java.util.stream.Stream;

import org.jsoup.nodes.Document;
import org.jsoup.nodes.Element;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.Mock;
import org.quartz.Trigger;
import org.xwiki.csrf.script.CSRFTokenScriptService;
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.query.internal.ScriptQuery;
import org.xwiki.query.script.QueryManagerScriptService;
import org.xwiki.rendering.RenderingScriptServiceComponentList;
import org.xwiki.rendering.internal.configuration.DefaultRenderingConfigurationComponentList;
import org.xwiki.rendering.internal.macro.message.ErrorMessageMacro;
import org.xwiki.rendering.internal.macro.message.InfoMessageMacro;
import org.xwiki.rendering.internal.macro.message.WarningMessageMacro;
import org.xwiki.script.service.ScriptService;
import org.xwiki.test.annotation.ComponentList;
import org.xwiki.test.page.HTML50ComponentList;
import org.xwiki.test.page.PageTest;
import org.xwiki.test.page.TestNoScriptMacro;
import org.xwiki.test.page.XWikiSyntax21ComponentList;

import com.xpn.xwiki.XWikiContext;
import com.xpn.xwiki.api.Object;
import com.xpn.xwiki.doc.XWikiDocument;
import com.xpn.xwiki.objects.BaseObject;
import com.xpn.xwiki.plugin.scheduler.JobState;
import com.xpn.xwiki.plugin.scheduler.SchedulerPluginApi;
import com.xpn.xwiki.plugin.scheduler.internal.SchedulerJobClassDocumentInitializer;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

/**
* Page tests for {@code Scheduler.WebHome}.
*
* @version $Id$
*/
@ComponentList({
InfoMessageMacro.class,
ErrorMessageMacro.class,
SchedulerJobClassDocumentInitializer.class,
TestNoScriptMacro.class,
WarningMessageMacro.class
})
@RenderingScriptServiceComponentList
@DefaultRenderingConfigurationComponentList
@HTML50ComponentList
@XWikiSyntax21ComponentList
class SchedulerPageTest extends PageTest
{
private static final String WIKI_NAME = "xwiki";

private static final String XWIKI_SPACE = "Scheduler";

private static final DocumentReference SCHEDULER_WEB_HOME =
new DocumentReference(WIKI_NAME, XWIKI_SPACE, "WebHome");

private static final String CSRF_TOKEN = "a0a0a0a0";

private QueryManagerScriptService queryService;

private CSRFTokenScriptService tokenService;

private SchedulerPluginApi schedulerPluginApi;

@Mock
private ScriptQuery query;

private Object testJobObjectApi;

@BeforeEach
void setUp() throws Exception
{
// Mock the Query Service to return a job.
this.queryService = this.oldcore.getMocker().registerMockComponent(ScriptService.class, "query",
QueryManagerScriptService.class,
true);
when(this.queryService.xwql(anyString())).thenReturn(this.query);
when(this.query.execute()).thenReturn(List.of("Scheduler.TestJob"));

// Mock the Token Service to get a consistent CSRF token throughout the tests.
this.tokenService = this.oldcore.getMocker().registerMockComponent(ScriptService.class, "csrf",
CSRFTokenScriptService.class, true);
when(this.tokenService.getToken()).thenReturn(CSRF_TOKEN);
when(this.tokenService.isTokenValid(CSRF_TOKEN)).thenReturn(true);

// Spy the Scheduler Plugin to obtain a mocked API.
this.schedulerPluginApi = mock(SchedulerPluginApi.class);
doReturn(this.schedulerPluginApi).when(this.oldcore.getSpyXWiki()).getPluginApi(eq("scheduler"),
any(XWikiContext.class));

this.xwiki.initializeMandatoryDocuments(this.context);

// Create a new job and keep a reference to its API.
XWikiDocument testJob = new XWikiDocument(new DocumentReference("xwiki", "Scheduler", "TestJob"));
BaseObject testJobObject = testJob.newXObject(SchedulerJobClassDocumentInitializer.XWIKI_JOB_CLASSREFERENCE,
this.context);
this.xwiki.saveDocument(testJob, this.context);
this.testJobObjectApi = new Object(testJobObject, this.context);

// Fake programming access level to display the complete page.
when(this.oldcore.getMockRightService().hasAccessLevel(eq("programming"), anyString(), anyString(),
any(XWikiContext.class))).thenReturn(true);
}

/**
* Verify that the trigger operation is not called in the Scheduler Plugin API when the CSRF token is invalid, and
* that the corresponding error message is properly displayed.
*/
@Test
void checkInvalidCSRFToken() throws Exception
{
String wrongToken = "wrong token";

this.request.put("do", "trigger");
this.request.put("which", "Scheduler.TestJob");
this.request.put("form_token", wrongToken);
Document result = renderHTMLPage(SCHEDULER_WEB_HOME);

verify(this.schedulerPluginApi, never()).triggerJob(any(Object.class));
verify(this.tokenService).isTokenValid(wrongToken);
assertEquals("xe.scheduler.invalidToken", result.getElementsByClass("errormessage").text());
}

/**
* Verify that the trigger operation is correctly called in the Scheduler Plugin API when the CSRF token is valid,
* and that no error displays.
*/
@Test
void checkValidCSRFToken() throws Exception
{
when(this.schedulerPluginApi.triggerJob(this.testJobObjectApi)).thenReturn(true);

this.request.put("do", "trigger");
this.request.put("which", "Scheduler.TestJob");
this.request.put("form_token", CSRF_TOKEN);
Document result = renderHTMLPage(SCHEDULER_WEB_HOME);

verify(this.schedulerPluginApi).triggerJob(this.testJobObjectApi);
verify(this.tokenService).isTokenValid(CSRF_TOKEN);
assertTrue(result.getElementsByClass("errormessage").isEmpty());
}

/**
* List every possible action that can be applied to a job depending on its current status.
*
* @return a {@link Stream} of {@link org.junit.jupiter.params.provider.Arguments} for every combination of job
* status and action
*/
static Stream<Arguments> jobStatusAndActionProvider()
{
return Stream.of(
Arguments.of(new JobState(Trigger.TriggerState.NONE), "trigger"),
Arguments.of(new JobState(Trigger.TriggerState.NONE), "schedule"),
Arguments.of(new JobState(Trigger.TriggerState.NORMAL), "pause"),
Arguments.of(new JobState(Trigger.TriggerState.NORMAL), "unschedule"),
Arguments.of(new JobState(Trigger.TriggerState.PAUSED), "resume"),
Arguments.of(new JobState(Trigger.TriggerState.PAUSED), "unschedule"),
Arguments.of(new JobState(Trigger.TriggerState.NONE), "delete"));
}

/**
* Verify that each action URL on the page contains the right CSRF token.
*
* @param status the status of the job
* @param action the action to verify
*/
@ParameterizedTest
@MethodSource("jobStatusAndActionProvider")
void checkCSRFTokenPresenceInActionURL(JobState status, String action) throws Exception
{
// Set the status of the displayed job to control which action URLs will be rendered.
when(this.schedulerPluginApi.getJobStatus(this.testJobObjectApi)).thenReturn(status);

Document result = renderHTMLPage(SCHEDULER_WEB_HOME);
verify(this.schedulerPluginApi).getJobStatus(this.testJobObjectApi);
Element actionLink = result.selectFirst(String.format("td a:contains(actions.%s)", action));
assertNotNull(actionLink);

// Check the presence of the CSRF token for the given action.
assertEquals(String.format("path:/xwiki/bin/view/Scheduler/?do=%s&which=Scheduler"
+ ".TestJob&form_token=%s", action, CSRF_TOKEN), actionLink.attr("href"));
}

/**
* Verify that the names of jobs are properly escaped in each action URL.
*
* @param status the status of the job
* @param action the action to verify
*/
@ParameterizedTest
@MethodSource("jobStatusAndActionProvider")
void checkEscapingInJobNames(JobState status, String action) throws Exception
{
// Use the `noscript` macro to make sure that no code injection occurs.
String jobName = "\">]]{{/html}}{{noscript /}}";
String escapedJobName = "%22%3E%5D%5D%7B%7B%2Fhtml%7D%7D%7B%7Bnoscript%20%2F%7D%7D";

// Create a new job with a name that needs escaping and get a reference to its API.
XWikiDocument escapedJob = new XWikiDocument(new DocumentReference("xwiki", "Scheduler", jobName));
BaseObject escapedJobObject =
escapedJob.newXObject(SchedulerJobClassDocumentInitializer.XWIKI_JOB_CLASSREFERENCE, this.context);
Object escapedJobObjectApi = new Object(escapedJobObject, this.context);
this.xwiki.saveDocument(escapedJob, this.context);

// Return the name of the new job through the Query Service.
when(this.query.execute()).thenReturn(List.of("Scheduler." + jobName));

// Set the status of the new job to control which action URLs will be rendered.
when(this.schedulerPluginApi.getJobStatus(escapedJobObjectApi)).thenReturn(status);

Document result = renderHTMLPage(SCHEDULER_WEB_HOME);
Element actionLink = result.selectFirst(String.format("td a:contains(actions.%s)", action));
assertNotNull(actionLink);

// Check the proper escaping of the job name for the given action.
assertEquals(String.format("path:/xwiki/bin/view/Scheduler/?do=%s&which=Scheduler"
+ ".%s&form_token=%s", action, escapedJobName, CSRF_TOKEN), actionLink.attr("href"));
}
}

0 comments on commit f30d9c6

Please sign in to comment.