WebChat "default-user" causes shared user state

[lauren-mills]

I noticed during some testing using UserState that each time I opened webchat it would pull down the same state object in each instance.

Webchat uses "default-user" as the id for all anonymous users which means all users will share the same user state. This seems like a security risk.

Can we change to use a random user id for all anonymous users?

[lauren-mills]

Also, this is my code:

    var bot = window.WebChat.createDirectLine({ secret: 'DIRECT_LINE' });
        window.WebChat.renderWebChat({
            directLine: bot
        }, document.getElementById('webchat'));

        // This code sends an event which triggers the ConversationUpdate message
        bot.postActivity({
            from: { id:"anonymous", role: "user"},
                name: 'startConversation',
                type: 'event',
                value: ''
            })
            .subscribe(function (id) {
                console.log('trigger "startConversation" sent');
            });

[baldrin]

@lauren-mills Where you ever able to come up with a solution to this issue? I've been having the same problem.

Here is what I've found. If I do this in a dialog

UserData userData = await BotAccessors.UserDataAccessor.GetAsync(stepContext.Context, () => new UserData());

And I look in my CosmosDB for the Document that was created, I find this

"id": "directline*2fusers*2fdefault-user",

I'm assuming the last part of the string is the user-id/user-name, which now always defaults to 'default-user' because of the change that was made in V4 from V3.

[compulim]

@lauren-mills @baldrin found the issue in connectSaga.js.

The problem is, if the user is using Direct Line secret (but not access token), it is always using "default-user". We need to change it to accept user ID from renderWebChat call. And the user can provide whatever user ID they want. We might want to put a bit restrictions over there: must be a string and not start with "dl_" (to prevent our "forge-proof" user ID to be mistakingly used).

Note to fixer

• Modify user ID generation logic
  • If user ID is found from access token, use it, otherwise,
    • Give a warning if user ID is specified via Web Chat options, but we overrode it
  • If it is specified on Web Chat options, use it, otherwise,
    • Validate user ID to meet restrictions:
      • Must be a string
      • Must not start with dl_ (our forge-proof user ID)
    • If not meeting the restriction, give a warning and fallback to "default-user"
  • Use "default-user"

[lauren-mills]

Thanks, William!

[compulim]

@lauren-mills sorry was late to response. Feel free to fix it. It will take me ~2 weeks before I can start working on it.

[baldrin]

@compulim , @lauren-mills
Is this something I can contribute to fixing? I don't know the rules regarding your code base. This is probably something I could code and submit.

[compulim]

@baldrin Thanks, sounds great!

We don't set any special rules. And we will fix your code if we could make it better. 😉

• Fork us, create a branch on your fork, fix it
• Format: 2-space indent, bracelets on same-line, no trailing spaces, single trailing line (we will fix them for you if the format is not perfect)
• Sign Microsoft CLA (you will see it when you create your PR)
• We will review/approve your change
• We prefer smaller PR
  • This works (in the "Note to fixer" section of this issue) can fit in a single PR
• We are currently doing manual testing today
  • We have automated test harness setup on Travis CI, but yet to add real tests

[compulim]

@lauren-mills and @baldrin sorry I have poor memory.

When I look at existing code, Web Chat will accept userID props and allow the user to change it. So the actual logic is there and everything looks good to me.

I will add documentation on how to change the user ID. And add some validation rules set above.

One extra note: if you use React and change the userID props, Web Chat will disconnect from Direct Line and restart it, i.e. calling directLine.end(). I doubt the code would work today because Direct Line reconnection story is very weak now.

[compulim]

PR #1447 will add docs to README.md and more validation rules, pending review.

[darrenj]

Whilst the documentation helps this still will create a situation where developers who use webchat "as-is" will end up with users sharing session state as the userId is shared. We've seen examples of this with people in testing.

I would recommend we have some form of fall-back where we use a GUID and/or display a warning message on the webchat canvas.

[compulim]

We could use randomatic, which use (math-random)[https://www.npmjs.com/package/math-random]. And it will try to be cryptographically random if the platform support it (e.g. modern browsers).

[deniercounter]

THIS is a major security issue and has major issues with EU GDPR. I am flabbergasted. Are you aware that Microsoft makes all EU companies using the Microsoft Botframework breach the EU regulations. You have to turn to the general public and inform all companies, that the webchatcontrol is NOT safe to use. This security flaw is known since 13. Nov 2018!