summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNathan Kinder <nkinder@redhat.com>2015-04-01 17:36:22 -0700
committerRob Crittenden <rcritten@redhat.com>2015-04-01 23:18:39 -0400
commit8ffd2629c6554124e7e3b15dce10275a2efd8261 (patch)
tree32212efa6f6884ba357955dbcaab93d955744b90
parent9600cff7a3f93529ab56044968b489030f74b76c (diff)
downloadipsilon-8ffd2629c6554124e7e3b15dce10275a2efd8261.zip
ipsilon-8ffd2629c6554124e7e3b15dce10275a2efd8261.tar.gz
ipsilon-8ffd2629c6554124e7e3b15dce10275a2efd8261.tar.xz
Validate SP names for admin pages and REST
We were previously only validating the SP name in the admin pages for SP creation and update. The REST API would allow a SP to be created with an invalid name, which would break the ability to manage that SP in the admin pages. This patch moves the SP name validation logic out of the admin page code and centralizes it in the provider creation code. This ensures that validation will occur regardless of the interface that is used. In addition, a helper method is added to allow the admin page to check if a name is valid during update operations. https://fedorahosted.org/ipsilon/ticket/102 Signed-off-by: Nathan Kinder <nkinder@redhat.com> Reviewed-by: Rob Crittenden <rcritten@redhat.com>
-rw-r--r--ipsilon/providers/saml2/admin.py12
-rw-r--r--ipsilon/providers/saml2/provider.py13
-rwxr-xr-xtests/testrest.py35
3 files changed, 49 insertions, 11 deletions
diff --git a/ipsilon/providers/saml2/admin.py b/ipsilon/providers/saml2/admin.py
index 0ab2a41..2503be1 100644
--- a/ipsilon/providers/saml2/admin.py
+++ b/ipsilon/providers/saml2/admin.py
@@ -23,13 +23,9 @@ from ipsilon.admin.common import ADMIN_STATUS_WARN
from ipsilon.providers.saml2.provider import ServiceProvider
from ipsilon.providers.saml2.provider import ServiceProviderCreator
from ipsilon.providers.saml2.provider import InvalidProviderId
-import re
import requests
-VALID_IN_NAME = r'[^\ a-zA-Z0-9]'
-
-
class NewSPAdminPage(AdminPage):
def __init__(self, site, parent):
@@ -68,12 +64,6 @@ class NewSPAdminPage(AdminPage):
cherrypy.request.content_type,))
for key, value in kwargs.iteritems():
if key == 'name':
- if re.search(VALID_IN_NAME, value):
- message = "Invalid name!" \
- " Use only numbers and letters"
- message_type = ADMIN_STATUS_ERROR
- return self.form_new(message, message_type)
-
name = value
elif key == 'metatext':
if len(value) > 0:
@@ -156,7 +146,7 @@ class SPAdminPage(AdminPage):
return False
if self.user.is_admin or self.user.name == self.sp.owner:
- if re.search(VALID_IN_NAME, value):
+ if not self.sp.is_valid_name(value):
err = "Invalid name! Use only numbers and letters"
raise InvalidValueFormat(err)
diff --git a/ipsilon/providers/saml2/provider.py b/ipsilon/providers/saml2/provider.py
index 4439a0d..d1c7b42 100644
--- a/ipsilon/providers/saml2/provider.py
+++ b/ipsilon/providers/saml2/provider.py
@@ -19,6 +19,10 @@ from ipsilon.providers.common import ProviderException
from ipsilon.tools.saml2metadata import SAML2_NAMEID_MAP
from ipsilon.util.log import Log
import lasso
+import re
+
+
+VALID_IN_NAME = r'[^\ a-zA-Z0-9]'
class InvalidProviderId(ProviderException):
@@ -136,6 +140,11 @@ class ServiceProvider(Log):
return username.split('@', 1)[0]
return username
+ def is_valid_name(self, value):
+ if re.search(VALID_IN_NAME, value):
+ return False
+ return True
+
def is_valid_nameid(self, value):
if value in SAML2_NAMEID_MAP:
return True
@@ -153,6 +162,10 @@ class ServiceProviderCreator(object):
def create_from_buffer(self, name, metabuf):
'''Test and add data'''
+ if re.search(VALID_IN_NAME, name):
+ raise InvalidProviderId("Name must contain only "
+ "numbers and letters")
+
test = lasso.Server()
test.addProviderFromBuffer(lasso.PROVIDER_ROLE_SP, metabuf)
newsps = test.get_providers()
diff --git a/tests/testrest.py b/tests/testrest.py
index bf16b8b..24a7092 100755
--- a/tests/testrest.py
+++ b/tests/testrest.py
@@ -56,6 +56,18 @@ sp2_a = {'hostname': '${ADDRESS}:${PORT}',
'saml_auth': '/sp',
'httpd_user': '${TEST_USER}'}
+sp3_g = {'HTTPDCONFD': '${TESTDIR}/${NAME}/conf.d',
+ 'SAML2_TEMPLATE': '${TESTDIR}/templates/install/saml2/sp.conf',
+ 'SAML2_CONFFILE': '${TESTDIR}/${NAME}/conf.d/ipsilon-saml.conf',
+ 'SAML2_HTTPDIR': '${TESTDIR}/${NAME}/saml2'}
+
+
+sp3_a = {'hostname': '${ADDRESS}:${PORT}',
+ 'saml_idp_metadata': 'http://127.0.0.10:45080/idp1/saml2/metadata',
+ 'saml_secure_setup': 'False',
+ 'saml_auth': '/sp',
+ 'httpd_user': '${TEST_USER}'}
+
def fixup_sp_httpd(httpdir, alias):
location = """
@@ -116,18 +128,31 @@ class IpsilonTest(IpsilonTestBase):
print "Starting SP's httpd server"
self.start_http_server(conf, env)
+ print "Installing third SP server"
+ name = 'sp3.invalid'
+ addr = '127.0.0.10'
+ port = '45083'
+ sp3 = self.generate_profile(sp3_g, sp3_a, name, addr, port)
+ conf = self.setup_sp_server(sp3, name, addr, port, env)
+ fixup_sp_httpd(os.path.dirname(conf), name)
+
+ print "Starting SP's httpd server"
+ self.start_http_server(conf, env)
+
if __name__ == '__main__':
idpname = 'idp1'
spname = 'sp1'
sp2name = 'sp2'
+ sp3name = 'sp3.invalid'
user = pwd.getpwuid(os.getuid())[0]
sess = HttpSessions()
sess.add_server(idpname, 'http://127.0.0.10:45080', user, 'ipsilon')
sess.add_server(spname, 'http://127.0.0.11:45081')
sess.add_server(sp2name, 'http://127.0.0.10:45082')
+ sess.add_server(sp3name, 'http://127.0.0.10:45083')
print "testrest: Authenticate to IDP ...",
try:
@@ -213,6 +238,16 @@ if __name__ == '__main__':
# Now for some negative testing
+ print "testrest: Add illegally named Service Provider via REST ...",
+ try:
+ sess.add_sp_metadata(idpname, sp3name, rest=True)
+ except ValueError, e:
+ print " SUCCESS"
+ else:
+ print >> sys.stderr, "ERROR: " \
+ "Adding SP with invalid name should have failed and it didn't"
+ sys.exit(1)
+
print "testrest: Fetch non-existent REST endpoint ...",
try:
result = sess.fetch_rest_page(