Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upprettify XML string output by registering default namespace prefixes #326
Conversation
|
See also #249 |
|
This is nice to have. It breaks tests as they depend on the exact output which includes the namespaces. We should look for ways to avoid depending on that. |
|
This is an important PR, we shouldn't forget it! |
as @spaceone already proposed here: IdentityPython#326
as @spaceone already proposed here: IdentityPython#326
…ML:2.0:assertion"')
…ML:2.0:assertion"')
…ML:2.0:assertion"')
…AML:2.0:protocol"')
…ML:2.0:assertion"')
…AML:2.0:protocol"')
|
I replaced/fixed the tests with the following script: import pipes
import subprocess
NAMESPACE = 'urn:oasis:names:tc:SAML:2.0:assertion'
SAMLP_NAMESPACE = 'urn:oasis:names:tc:SAML:2.0:protocol'
XSI_NAMESPACE = 'http://www.w3.org/2001/XMLSchema-instance'
XS_NAMESPACE = 'http://www.w3.org/2001/XMLSchema'
DS_NAMESPACE = 'http://www.w3.org/2000/09/xmldsig#'
MDUI_NAMESPACE = "urn:oasis:names:tc:SAML:metadata:ui"
MD_NAMESPACE = "urn:oasis:names:tc:SAML:2.0:metadata"
DEFAULT_NS_PREFIXES = {'saml': NAMESPACE, 'samlp': SAMLP_NAMESPACE, 'ds': DS_NAMESPACE, 'xsi': XSI_NAMESPACE, 'xs': XS_NAMESPACE, 'mdui': MD_NAMESPACE, 'md': MD_NAMESPACE}
for ns, xmlns in DEFAULT_NS_PREFIXES.items():
for i in range(6):
code = '''sed -i 's/ns%d/%s/g' $(git grep -l 'xmlns:ns%d="%s"')''' % (i, ns, i, xmlns)
if 0 == subprocess.call(code, shell=True):
subprocess.call('git ci -a -m %s' % (pipes.quote(code),), shell=True)But unfortionately still 7 tests are failing. |
|
Hi @spaceone, I also extended your code here https://github.com/IdentityPython/pysaml2/pull/625/files before your latest commits. If you agree we can work all together to make it to work and get it to be merged. I don't think that an hard-coded subprocess call, with an embedded shell env in it, could be considered as a good solution, despite this I understand your contribution logic and your personal need to get it to work. If this particular feature will be considered by lead project mantainers as important, as I also do, I'd like to make this proposals:
I consider this feature important because friends from Federation Service pointed me out that there are some SP that could not understand well those metadata created in this way and this could be a problem for my new pysaml2 IDP validation (but I also don't think strictly the same). Thank you all, hope the best thing for this PR |
|
In my last commit I get all the test passes without any alchemies :) The key is ElementTree. As I also read into SAML standard documents, those namespace will not create functional problems if they should changed, or if they would follow ElementTree's automatic assignment, but I also think that is very important to follow OASIS conventions to remove all doubts to other collegues and operators in the identity federations. I agree about the fact that this is only a cosmetic taste, but help us to give to pysaml2 an even better posture in comparison to leading softwares like Shibboleth. The even better news is, that a user can register his preferred ns_prefixes in his general config. |
|
@peppelinux Could you add the |
|
@spaceone in my PR BaseSaml.register_prefix Is a stati method and It can call register_namespace, globally, runtime. You can change those ns each time in differenti time, also in the settings of your project. Anyone can register his own namespace globally, calling BaseSaml.register_prefix runtime. The test that checks this behaviour Is test_88. Each xml string in the tests can be generalized with the defaults, taken by the default ns dict, with .format(). I Will. I'd like to have OASIS default namespaces in pysaml2 as default, @cookiemonster what do you think? Thank you for the reply and for your PR, your idea made me see something important |
|
@spaceone , @peppelinux : Can your team fix and merge this commit as we are totally dependent on this pysaml2 package for generating SAML response which is by default giving me xml namespaces as ns0, ns1, ns2. We are nearing our deployment to PROD. please let me know if any other way to include custom namespace mapping via CONFIG, or any other attribute to pas to - self.server.create_authn_response() |
|
@Vinod83GH this is the current implementation that is going to be merged: |
The namespace-prefix names do not matter; they are purely cosmetic. Why do you think For this reason, this patch is low-priority. |
|
@c00kiemon5ter I would really appreciate if this patch will be merged as there is an issue with signed responses, when saml namespaces required |
|
No description provided.