Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues/single logout #83

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Issues/single logout #83

wants to merge 4 commits into from

Conversation

nightBaker
Copy link

#80
After logout CAS sends post request message=<samlp:LogoutRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" ID="LR-177-X8BLQyvy76SFszPwCMYwpFeF" Version="2.0" IssueInstant="2018-04-12T15:07:21Z"><saml:NameID xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">@NOT_USED@</saml:NameID>samlp:SessionIndexST-2171-iZF3BlhCp9VdarvPV-tJ1GEebO0-kaspi-portsso1</samlp:SessionIndex></samlp:LogoutRequest>,asynchronous=false,contentType=application/x-www-form-urlencoded

And dotnet-cas-client tryes get logoutRequest from HttpContext.Current.Request.Form

internal static void ProcessSingleSignOutRequest()
        {
            HttpContext context = HttpContext.Current;
            HttpRequest request = context.Request;
            HttpResponse response = context.Response;
            protoLogger.Debug("Examining request for single sign-out signature");

            if (request.HttpMethod == "POST" && request.Form["logoutRequest"] != null)
            {

getting value is causing exception - context.Request.Form["logoutRequest"] 'context.Request.Form["logoutRequest"]' threw an exception of type 'System.Web.HttpRequestValidationException' string {System.Web.HttpRequestValidationException}

@phantomtypist
Copy link
Contributor

Uh, @nightBaker why do you keep closing and re-opening pull requests? You can just keep continuing your changes on a single pull request and push the changes to it as you go along.

TL;DR: no need to create a new pull request and close the previous one for each commit/change you make.

@phantomtypist
Copy link
Contributor

@nightBaker You'll also need to tell us when you are done making changes to the PR... otherwise I have no clue when you are done making changes... which is why I'll suggest that you don't normally submit a PR in any source control system until you are done working on the code on an issue.

Granted though, it's okay to have a discussion in the PR once it is submitted... which might lead to further changes on your part. TL;DR: Get what you want to get done in its fullest and then submit a PR and we can discuss as a group.

@nightBaker
Copy link
Author

nightBaker commented Apr 13, 2018

I have done making changes)
There are some places where it tries to get value from HttpContext.Current.Request.Form["logoutRequest"]
what contains xml, so ASP.NET throws exception System.Web.HttpRequestValidationException
so I made changes for .net 4.5 using Unvalidated property which gives access to values without triggering ASP.NET request validation
HttpContext.Current.Request.Unvalidated.Form["logoutRequest"]
what solved the issue

@phantomtypist
Copy link
Contributor

@nightBaker

You say you fixed the problem for the .NET 4.x code path.

Question: does the problem exist in the .NET 2/3.x code path? If yes, then have you fixed that as well in this PR?

@nightBaker
Copy link
Author

@phantomtypist
Problem still exists in the .Net 2/3. It is not possible to fix in same way, because HttpContext.Current.Request.Unvalidated is not available for thus versions of framework

@phantomtypist
Copy link
Contributor

@nightBaker I understand how it is not fixable in 2/3.x in the same manner you did for 4.x, but do you think you'd feel a little adventurous to see if you can come up with a fix for the 2/3.x side of things?... no pressure though ;)

@nightBaker
Copy link
Author

@phantomtypist, problem for below versions of .NET is solved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants