Don't parse html with xml parser #53
Conversation
djangosaml2/views.py
Outdated
| params = get_hidden_form_inputs(result['data'][3]) | ||
| else: | ||
| # manually get request XML to build our own template | ||
| request_id, request_xml = client.create_authn_request( |
There was a problem hiding this comment.
I don't like doing this :-(.
We already have a result from prepare_for_authenticate, now we should disregard it and ask for a new, just because we can "parse it better"?
I take it that you're trying to fine-tune the code and I appreciate it, but this is ain't broke (it works!) and I prefer stability here. There's really no need to do this in my opinion.
There was a problem hiding this comment.
Oh no, it's still very broken if you have any characters in your IDP SSO url that choke an xml parser. I only fixed the & because the was one we had happen in the field, and I intended it only as a temporary fix until I dug more into pysaml2.
I can change it so it does only one or the other instead of both in that case if you like, but explicitly getting the parameters instead of parsing them from html that pysaml2 generates is going to be far less bug-prone. Do you want me to go ahead and have it only do one or the other?
…d don't create two requests in the case where we use a custom template for http-post binding.
|
Updated to include the code from #51, don't create two requests when using a custom template for http-post binding, and pass logger variables as arguments. |
2 participants
This replaces the "encode & symbols" hack from my previous PR, and instead of parsing html with an xml parser, we get the necessary data straight from pysaml2.