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

[Bug] The use of "+" instead of "%20" in URLs is not supported by some third party IDPs #5061

Open
lgiuliani80 opened this issue Jan 7, 2025 · 1 comment
Labels

Comments

@lgiuliani80
Copy link

lgiuliani80 commented Jan 7, 2025

Library version used

4.66.2

.NET version

.NET Framework 4.8

Scenario

PublicClient - desktop app

Is this a new or an existing app?

The app is in production, I haven't upgraded MSAL, but started seeing this issue

Issue description and reproduction steps

Some third party Identity Providers [namely Broadcom Siteminder] do NOT allow the usage of "+" to represent a space in URLs, in particular to separate scopes in the "authorize" endpoint.
So the default scope string "openid profile offline_access", being encoded in the URL string as "openid+profile+offline_access", is interpreted by the IDP as a SINGLE scope "openid+profile+offline_access", which is of course rejected.

Steps to reproduce: try to log in into Siteminder.

Relevant code snippets

No response

Expected behavior

No response

Identity provider

Other

Regression

No response

Solution and workarounds

The solution I propose requires a minimal change: just comment out the line 34 in CoreHelpers.cs:

public static string UrlEncode(string message)
{
    if (string.IsNullOrEmpty(message))
    {
        return message;
    }

    message = Uri.EscapeDataString(message);
    //message = message.Replace("%20", "+");  // THIS IS NOT NEEDED

    return message;
}

The replacement of "%20" with "+" is NOT needed and this action actually harms compatibility.

There is NO workaround for this issue. In order to support SiteMinder I had to fork MSAL.NET to apply this change.

@lgiuliani80 lgiuliani80 added needs attention Delete label after triage untriaged Do not delete. Needed for Automation labels Jan 7, 2025
@lgiuliani80
Copy link
Author

Version of Siteminder used: 12.8

@ashok672 ashok672 added tracked-ado and removed untriaged Do not delete. Needed for Automation labels Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants