From 5f0a9072c96bfe708197939e3cce639efd597777 Mon Sep 17 00:00:00 2001
From: Martijn Vermaat <martijn@vermaat.name>
Date: Wed, 19 Jan 2011 15:02:28 +0000
Subject: [PATCH] Various security related fixes to url routing. Renamed SOAP
webservice from MutalyzerService to Mutalyzer. Updated copyright years in
HTML template. Added test checking availability of WSDL file.
git-svn-id: https://humgenprojects.lumc.nl/svn/mutalyzer/trunk@160 eb6bd6ab-9ccd-42b9-aceb-e2899b4a52f1
---
src/tests/test_webservice.py | 15 +++++++--
src/tests/test_wsgi.py | 4 ++-
src/webservice.py | 13 +++-----
src/wsgi.py | 65 ++++++++++++++++++++++++++++--------
templates/client-mono.cs | 8 ++---
templates/menu.html | 2 +-
6 files changed, 77 insertions(+), 30 deletions(-)
diff --git a/src/tests/test_webservice.py b/src/tests/test_webservice.py
index da36ba2c..8c6eeaa6 100755
--- a/src/tests/test_webservice.py
+++ b/src/tests/test_webservice.py
@@ -2,17 +2,28 @@
"""
Tests for the SOAP interface to Mutalyzer.
-
-@todo: Test availability of the WSDL file.
"""
import logging; logging.raiseExceptions = 0
+import urllib2
from suds.client import Client
from suds import WebFault
import unittest
WSDL_URL = 'http://mutalyzer.martijn/services/?wsdl'
+class TestWSDL(unittest.TestCase):
+ """
+ Test the Mutalyzer SOAP interface WSDL description.
+ """
+ def test_wsdl(self):
+ """
+ Test if the WSDL is available and looks somewhat sensible.
+ """
+ wsdl = urllib2.urlopen(WSDL_URL).read()
+ self.assertTrue(wsdl.startswith("<?xml version='1.0' encoding='UTF-8'?>"))
+ self.assertTrue('name="Mutalyzer"' in wsdl)
+
class TestWebservice(unittest.TestCase):
"""
Test the Mutalyzer SOAP interface.
diff --git a/src/tests/test_wsgi.py b/src/tests/test_wsgi.py
index 9ea5f2e1..fae2b040 100755
--- a/src/tests/test_wsgi.py
+++ b/src/tests/test_wsgi.py
@@ -8,6 +8,8 @@ Uses WebTest, see:
http://blog.ianbicking.org/2010/04/02/webtest-http-testing/
I just installed webtest by 'easy_install webtest'.
+
+@todo: Tests for /upload, /getGS, and /Variant_info.
"""
import re
@@ -293,7 +295,7 @@ class TestWSGI(unittest.TestCase):
"""
r = self.app.get('/documentation')
self.assertEqual(r.content_type, 'text/html')
- r.mustcontain('Web Service: MutalyzerService')
+ r.mustcontain('Web Service: Mutalyzer')
if __name__ == '__main__':
unittest.main()
diff --git a/src/webservice.py b/src/webservice.py
index f83d1151..f17e080f 100644
--- a/src/webservice.py
+++ b/src/webservice.py
@@ -29,10 +29,6 @@ import logging; logging.basicConfig()
# $ cd soaplib
# $ sudo python setup.py install
-# Other tree:
-# https://github.com/cuker/soaplib/
-# https://github.com/cuker/soaplib/commit/1a248ba1421c57738c6d30333036114c3ed42022
-
from soaplib.core import Application
from soaplib.core.service import soap
from soaplib.core.service import DefinitionBase
@@ -227,8 +223,8 @@ class MutalyzerService(DefinitionBase) :
return ret
#getTranscriptsByGene
- @soap(Mandatory.String, Mandatory.String, Mandatory.Integer, Mandatory.Integer, Mandatory.Integer,
- _returns = List.String)
+ @soap(Mandatory.String, Mandatory.String, Mandatory.Integer,
+ Mandatory.Integer, Mandatory.Integer, _returns = List.String)
def getTranscriptsRange(self, build, chrom, pos1, pos2, method) :
"""Get all the transcripts that overlap with a range on a chromosome.
@@ -304,7 +300,8 @@ class MutalyzerService(DefinitionBase) :
#getGeneName
- @soap(Mandatory.String, Mandatory.String, Mandatory.String, Mandatory.String, _returns = Mapping)
+ @soap(Mandatory.String, Mandatory.String, Mandatory.String,
+ Mandatory.String, _returns = Mapping)
def mappingInfo(self, LOVD_ver, build, accNo, variant) :
"""Search for an NM number in the MySQL database, if the version
number matches, get the start and end positions in a variant and
@@ -631,7 +628,7 @@ class MutalyzerService(DefinitionBase) :
# WSGI application for use with e.g. Apache/mod_wsgi
soap_application = Application([MutalyzerService],
'http://mutalyzer.nl/2.0/services', # namespace
- 'MutalyzerService')
+ 'Mutalyzer')
application = wsgi.Application(soap_application)
# We can also use the built-in webserver by executing this file directly
diff --git a/src/wsgi.py b/src/wsgi.py
index 2f211c28..a8c91c63 100644
--- a/src/wsgi.py
+++ b/src/wsgi.py
@@ -41,6 +41,8 @@ port 8080):
VERSION = '2.0 β-6'
NOMENCLATURE_VERSION = '2.0'
RELEASE_DATE = '12 Jan 2011'
+WEBSERVICE_LOCATION = '/services'
+WSDL_VIEWER = 'templates/wsdl-viewer.xsl'
# Log exceptions to stdout
@@ -51,7 +53,6 @@ import os
import bz2
import web
import urllib
-import urllib2
import site
from lxml import etree
@@ -111,8 +112,8 @@ urls = (
'/Results_(\d+)\.txt', 'BatchResult',
'/download/([a-zA-Z-]+\.(?:py|cs))', 'Download',
'/downloads/([a-zA-Z\._-]+)', 'Downloads',
- '/upload', 'Uploader',
- '/Reference/(.*)', 'Reference'
+ '/Reference/([\da-zA-Z\._-]+)', 'Reference',
+ '/upload', 'Uploader'
)
@@ -215,12 +216,18 @@ session = web.session.Session(app,
class Download:
"""
Download file from template directory, processing it with TAL first.
-
- @todo: This is a potential security hole.
"""
def GET(self, file):
"""
@arg file: Filename to download.
+ @type file: string
+
+ Be very careful to not call this with anything but an ordinary
+ filename. A possible security issue is allowing this method to be
+ called with file='../mutalyzer.conf' for example.
+
+ The url routing currently makes sure to only call this with filenames
+ of the form [a-zA-Z-]+\.(?:py|cs).
"""
# Process the file with TAL and return the content as a downloadable file.
if not os.path.isfile("templates/" + file):
@@ -236,12 +243,18 @@ class Download:
class Downloads:
"""
Download plain text files from /templates/downloads directory.
-
- @todo: This is a potential security hole.
"""
def GET(self, file):
"""
@arg file: Filename to download.
+ @type file: string
+
+ Be very careful to not call this with anything but an ordinary
+ filename. A possible security issue is allowing this method to be
+ called with file='../../mutalyzer.conf' for example.
+
+ The url routing currently makes sure to only call this with filenames
+ of the form [a-zA-Z\._-]+.
"""
if not os.path.isfile("templates/downloads/" + file):
raise web.notfound()
@@ -260,6 +273,14 @@ class Reference:
def GET(self, file):
"""
@arg file: Filename to download from cache.
+ @type file: string
+
+ Be very careful to not call this with anything but an ordinary
+ filename. A possible security issue is allowing this method to be
+ called with file='../../mutalyzer.conf' for example.
+
+ The url routing currently makes sure to only call this with filenames
+ of the form [a-zA-Z\._-]+.
"""
fileName = "%s/%s.bz2" % (C.Retriever.cache, file)
if not os.path.isfile(fileName):
@@ -346,7 +367,8 @@ class SyntaxCheck:
i = web.input()
variant = i.variant
if variant.find(',') >= 0:
- O.addMessage(__file__, 2, "WCOMMASYNTAX", "Comma's are not allowed in the syntax, autofixed")
+ O.addMessage(__file__, 2, "WCOMMASYNTAX",
+ "Comma's are not allowed in the syntax, autofixed")
variant = variant.replace(',', '')
#args["variant"]=variant
P = Parser.Nomenclatureparser(O)
@@ -399,7 +421,8 @@ class Snp:
O.addMessage(__file__, -1, "INFO", "Received rs%s" % rsId)
R = Retriever.Retriever(C.Retriever, O, None)
R.snpConvert(rsId)
- O.addMessage(__file__, -1, "INFO", "Finished processing rs%s" % rsId)
+ O.addMessage(__file__, -1, "INFO",
+ "Finished processing rs%s" % rsId)
#if
args = {
@@ -655,6 +678,8 @@ class Check:
class CheckForward:
"""
Set the given variant in the session and redirect to the name checker.
+
+ @todo: Cleaner solution (one without using a session variable).
"""
def GET(self):
"""
@@ -681,16 +706,16 @@ class BatchProgress:
"""
Progress for a batch job.
- @todo: The 'progress' template does not exist.
-
Parameters:
- jobID: ID of the job to return progress for.
- totalJobs: Total number of entries in this job.
- ajax: If set, return plain text result.
+
+ @todo: The 'progress' template does not exist.
"""
O = Output.Output(__file__, C.Output)
- attr = {"percentage" : 0}
+ attr = {"percentage": 0}
i = web.input(ajax=None)
try:
@@ -828,6 +853,14 @@ class BatchResult:
Return raw content (for batch checker results).
@arg result: Result identifier.
+ @type result: string
+
+ Be very careful to not call this with anything but an ordinary
+ filename. A possible security issue is allowing this method to be
+ called with file='../../mutalyzer.conf' for example.
+
+ The url routing currently makes sure to only call this with filenames
+ of the form \d+.
"""
file = 'Results_%s.txt' % result
handle = open(os.path.join(C.Scheduler.resultsDir, file))
@@ -1015,9 +1048,9 @@ class Documentation:
@todo: Use configuration value for .xsl location.
@todo: Cache this transformation.
"""
- url = web.ctx.homedomain + web.ctx.homepath + '/services'
+ url = web.ctx.homedomain + web.ctx.homepath + WEBSERVICE_LOCATION
wsdl_handle = StringIO(webservice.soap_application.get_wsdl(url))
- xsl_handle = open('templates/wsdl-viewer.xsl', 'r')
+ xsl_handle = open(WSDL_VIEWER, 'r')
wsdl_doc = etree.parse(wsdl_handle)
xsl_doc = etree.parse(xsl_handle)
transform = etree.XSLT(xsl_doc)
@@ -1038,6 +1071,10 @@ class Static:
exist. Special case is a page of None, having the same
effect as 'index'.
@type page: string
+
+ Be careful to only call this method with an argument that is a simple
+ template name. For example, make sure this is not called with page
+ value '../forbidden'. This check is implemented in the url routing.
"""
if not page:
page = 'index'
diff --git a/templates/client-mono.cs b/templates/client-mono.cs
index f3521754..6281a7ae 100644
--- a/templates/client-mono.cs
+++ b/templates/client-mono.cs
@@ -2,8 +2,8 @@
Compilation instructions:
wsdl '<tal tal:replace = "path"></tal>/services/?wsdl'
- gmcs /target:library MutalyzerService.cs -r:System.Web.Services
- gmcs /r:MutalyzerService.dll client-mono.cs
+ gmcs /target:library Mutalyzer.cs -r:System.Web.Services
+ gmcs /r:Mutalyzer.dll client-mono.cs
Usage:
@@ -12,7 +12,7 @@ Usage:
using System;
-class Mutalyzer {
+class MutalyzerClient {
public static void Main(string [] args) {
@@ -21,7 +21,7 @@ class Mutalyzer {
Console.WriteLine("Checking " + c.variant);
- MutalyzerService mutalyzer = new MutalyzerService();
+ Mutalyzer mutalyzer = new Mutalyzer();
checkSyntaxResponse result = mutalyzer.checkSyntax(c);
diff --git a/templates/menu.html b/templates/menu.html
index ab3c0361..0e6f7f9e 100644
--- a/templates/menu.html
+++ b/templates/menu.html
@@ -553,7 +553,7 @@
</td>
<td align="middle">
<img src = "base/images/LUMC_24x24.png" align = "middle">
- © 2010 <a href = "http://www.lumc.nl">LUMC</a>
+ © 2007-2011 <a href = "http://www.lumc.nl">LUMC</a>
</td>
--
GitLab