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

Sustainsys.Saml2.AspNetCore2.HttpRequestExtensions should be public #1474

Open
magnusj64 opened this issue Oct 15, 2024 · 3 comments
Open

Sustainsys.Saml2.AspNetCore2.HttpRequestExtensions should be public #1474

magnusj64 opened this issue Oct 15, 2024 · 3 comments

Comments

@magnusj64
Copy link

I would like to be able to call Sustainsys.Saml2.AspNetCore2.HttpRequestExtensions.ToHttpRequestData() from my code. As the similar function in the older Sustainsys.Saml2.HttpModule is defined in a public class I assume this omission is just an oversight.

As a workaround, I can copy this class into my own code without any issues. But I would prefer if I could use the existing code from the Sustainsys.Saml2.AspNetCore2 nuget .

@AndersAbel
Copy link
Member

This looks like a reasonable change to do, yes. Would you like to submit a PR?

magnusj64 added a commit to magnusj64/Saml2 that referenced this issue Nov 2, 2024
[Sustainsys/Saml2] Sustainsys.Saml2.AspNetCore2.HttpRequestExtensions should be public (Issue Sustainsys#1474)
@magnusj64
Copy link
Author

I made a PR for the v2 branch. Haven't checked if there's a need in the develop branch.

@rikrak
Copy link

rikrak commented Jan 10, 2025

One word of caution here: I copied the code for ToHttpRequestData() into my code base in order to access the relay data during the authentication process. I noticed an issue whereby, if the ToHttpRequestData() is called outside the scope of an authentication request, an exception can occur when posting a form. In my case, the exception occurred when posting a form that contained a checkbox. The form data contained two values for the same html element name. This caused this line[1] to blow up, as it assumes only a single value will be present for each form element.

        private void InitBasicFields(string httpMethod, Uri url, string applicationPath, IEnumerable<KeyValuePair<string, IEnumerable<string>>> formData)
        {
            HttpMethod = httpMethod;
            Url = url;
            ApplicationUrl = new Uri(url, applicationPath);
            Form = new ReadOnlyDictionary<string, string>(
                (formData ?? Enumerable.Empty<KeyValuePair<string, IEnumerable<string>>>())
                .ToDictionary(kv => kv.Key, kv => kv.Value.Single()));       /////  <--- HERE
            QueryString = QueryStringHelper.ParseQueryString(url.Query);
        }

Naturally this isn't a problem if the class is internal, as there is a great deal of control over where the method can be used. The restriction on where it's safe to use this method is not so obvious for a public class however. Obviously, the quick (and safe?) fix is to use .First() instead of .Single() (that's what I did in my copy of the method)

[1]

.ToDictionary(kv => kv.Key, kv => kv.Value.Single()));

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

No branches or pull requests

3 participants