System.ArgumentException if using the same dictionary object as "extraParameters" argument in the "LoginAsync" function to log in multiple times

[aleksander-ivanovski]

In my application, I have installed the "Auth0.OidcClient.WPF v3.2.4" from nuget.

I have defined my own "extraParameters" as "Dictionary<string,string>", where I have set the "audience" as the only key value pair. After the first login with the "LoginAsync" function, the Auth0Client appends telemetry by default in the same input argument dictionary with key "auth0Client".

When it is time to refresh the access token, my application calls the "LoginAsync" function again (no refresh token was available), and passes the same dictionary as argument. This time, a "System.ArgumentException" with the following message and stack is thrown: Message: An item with the same key has already been added. Stack: at System.ThrowHelper.ThrowArgumentException(ExceptionResource resource) at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add) at Auth0.OidcClient.Auth0ClientBase.AppendTelemetry(Object values) at Auth0.OidcClient.Auth0ClientBase.d__7.MoveNext()--- End of stack trace from previous location where exception was thrown ---

After looking at the source code of the "Auth0ClientBase.cs" in GitHub, I identified the line inside the "AppendTelemetry" private function, where the exception is most probably thrown: https://github.com/auth0/auth0-oidc-client-net/blob/master/src/Auth0.OidcClient.Core/Auth0ClientBase.cs#:~:text=if%20(_options.EnableTelemetry,auth0Client%22%2C%20_userAgent)%3B

if (_options.EnableTelemetry)
    dictionary.Add("auth0Client", _userAgent);

The "auth0Client" key will be added twice, thus throwing the exception.

Fix suggestions:

1. Always create an internal copy to avoid changing the contents of the input argument.
2. Remove the key if exists and add the new value with the same key.
3. Replace the value only: dictionary["auth0Client"] = _userAgent;

As a workaround, I will always create a new instance of the "extraParameters" dictionary on application side, to have a clean state on every login or refresh call.

[frederikprijck]

Thanks for reporting this, looks like it's a bug indeed.

We should be save to ensure we only add the key if it doesn't exist yet.

[aleksander-ivanovski]

Thanks for reporting this, looks like it's a bug indeed.

We should be save to ensure we only add the key if it doesn't exist yet.

That would be a solution, however the input argument "extraParameters" will still be modified. Me as a user, I don't expect a non "out" or "ref" argument to be modified inside a method. But since "out" and "ref" arguments are not allowed in "async" methods, I would suggest to always copy the key value pairs internally. This can be easily achieved by removing the following two lines in the private "ObjectToDictionary" method:

        private Dictionary<string, string> ObjectToDictionary(object values)
        {
            if (values is Dictionary<string, string> dictionary) // Remove
                return dictionary; // Remove

Then you could also add a check before adding the "auth0Client" key in the private "AppendTelemetry" method for safety, or just replace the value, as I suggested above.

[frederikprijck]

This can be easily achieved by removing the following two lines in the private "ObjectToDictionary" method:

Even tho I agree, not modifying the existing dictionary can be considered a breaking change and makes fixing this a bit more complicated.

As much as I want to avoid modifying parameters like is being done here, I believe we can't solve that without breaking people. Making me want to stick with solving the actual issue here first.

That said, let me see what the potential issue is if we do remove that line and if it would be acceptable to not cut a major for it.