ldClient top-level hash value not updated by ldClient.identify()

[andrao]

Describe the bug

Calls to the ldClient.identify() method do not update the ldClient.initialize()-scoped hash variable value, nor the Stream()-scoped hash value.

This causes errors in event streaming after calling ldClient.identify() with a new hash value.

To reproduce

1. Start with 2 users: user1 & user2.
2. Call ldClient.initialize(CLIENT_ID, user1, { hash: user1_hash, streaming: true }. The event stream is successfully initialized.
3. After initialization & readiness confirmed, call ldClient.identify(user2, user2_hash). The new user is successfully identified.
4. Calls to https://clientstream.launchdarkly.com will begin to fail, however. They continue to use the original hash value of user1_hash from the initialize() call in their query string.

Under the hood

The Stream module is initialized in the ldClient.initialize() call with the initial options.hash value as a parameter. ldClient.identify() takes in a hash parameter, but it doesn't update the top-level variable value.

index.js:

export function initialize(env, user, specifiedOptions, platform, extraOptionDefs) {
    // ...
    const options = configuration.validate(specifiedOptions, emitter, extraOptionDefs, logger);
    const hash = options.hash;  // const hash cannot be modified
    // ...

    // Initializes Stream() with the initial `hash` value:
    const stream = Stream(platform, options, environment, diagnosticsAccumulator, hash);
    // ...

    // identify() takes `hash` as a parameter, but does not update the top-level variable
    function identify(user, hash, onDone) {
        // Does not update top-level `hash` value...
    }
    // ...
}

In Stream.js, the initial hash value is used in the openConnection() method, wherein it is appended to the query string:

// Takes a `hash` parameter that never changes
export default function Stream(platform, config, environment, diagnosticsAccumulator, hash) {
    // ...

    // openConnection() always uses the initial `hash` value
    function openConnection() {
        // ...
        query = 'h=' + hash;
        // ...
    }
}

Expected behaviour

The ldClient.initialize()-scoped hash value & the Stream()-scoped hash value need to be updated on a call to ldClient.identify().

An option under the hood

ldClient.identify() does call connectStream() –

index.js:

// Current index.js:

  function identify(user, hash, onDone) {
    // ...
    return utils.wrapPromiseCallback(
      clearFirst
        // ...
        .then(flagValueMap => {
          if (streamActive) {
            connectStream();
          }
          // ...

which itself effects the Stream() module via a stream.connect() call –

index.js:

// Current index.js:

  function connectStream() {
    // ...
    stream.connect(ident.getUser(), {
      ping: function() {
      // ...

Given that stream.connect() updates the Stream-scoped user, perhaps it could also update the Stream-scoped hash.

Instead of (Stream.js):

// Current Stream.js:

export default function Stream(platform, config, environment, diagnosticsAccumulator, hash) {
  // ...
  let user = null;
  let handlers = null;

  stream.connect = function(newUser, newHandlers) {
    user = newUser;
    handlers = {};
     // ...
    tryConnect();
  };
  // ...
}

we could have:

// Potential Stream.js:

// EDIT: Remove `hash` parameter –
export default function Stream(platform, config, environment, diagnosticsAccumulator) {
  // ...
  let user = null;

  // EDIT: Introduce `hash` variable –
  let hash = null;

  let handlers = null;

  // EDIT: Add `newHash` parameter to .connect() –
  stream.connect = function(newUser, newHash, newHandlers) {
    user = newUser;

    // EDIT: Set `hash` value to `newHash` value
    hash = newHash;

    handlers = {};
     // ...
    tryConnect();
  };
  // ...
}

which would require going from, in index.js:

// Current index.js:

  function identify(user, hash, onDone) {
    // ...
    return utils.wrapPromiseCallback(
      clearFirst
        .then(() => userValidator.validateUser(user))
        .then(realUser =>
          requestor
            .fetchFlagSettings(realUser, hash)
            .then(requestedFlags => {
              const flagValueMap = utils.transformVersionedValuesToValues(requestedFlags);
              ident.setUser(realUser);
              // ...
            })
        )
        .then(flagValueMap => {
          if (streamActive) {
            connectStream();
          }
          // ...
  }
  // ...

  function connectStream() {
    streamActive = true;
    if (!ident.getUser()) {
      return;
    }
    stream.connect(ident.getUser(), {
      ping: function() {
      // ...
    }
  }

to:

// Potential index.js:

  // EDIT: Rename `hash` param to `newHash` –
  function identify(user, newHash, onDone) {
    // ...
    return utils.wrapPromiseCallback(
      clearFirst
        .then(() => userValidator.validateUser(user))
        .then(realUser =>
          requestor
            .fetchFlagSettings(realUser, hash)
            .then(requestedFlags => {
              const flagValueMap = utils.transformVersionedValuesToValues(requestedFlags);
              ident.setUser(realUser);

              // EDIT: Update hash value –
              hash = newHash;

              // ...
            })
        )
        .then(flagValueMap => {
          if (streamActive) {
            connectStream();
          }
          // ...
  }
  // ...

  function connectStream() {
    streamActive = true;
    if (!ident.getUser()) {
      return;
    }

    // EDIT: Pass `hash` to `stream.connect()` –
    stream.connect(ident.getUser(), hash, {
      ping: function() {
      // ...
    }
  }

SDK version

3.1.1

[eli-darkly]

This is trickier than it might seem, because the hash is not and cannot be generated by the front-end code. That's the whole point of secure mode: the back end has to compute the hash based on a private credential that is not exposed to the front end. And given the massive variations in how web apps and their endpoints are set up, there isn't any standard mechanism that the front end could use to request a new hash value from the back end.

So I'm not sure if there is much we can do for this, other than to document that using secure mode is not compatible with switching users on the fly in JS.

[andrao]

Hi @eli-darkly – thanks for the prompt response.

In our case we call identify() only after an in-app, post-intial-load query to the backend returns a hash value. A single session, using a single LD client, can switch between namespaces – hence the usage of identify() after the initial client initialization.

So no hash is being generated on the frontend.

If using identify() is equivocal with switching users on the fly, what purpose is its hash parameter meant to serve?

[eli-darkly]

Ah, sorry, I misunderstood what you were getting at - I didn't realize you already had a value and were passing it, and not seeing the correct result. Yes, this looks like a straightforward bug and we'll put a high priority on fixing it.

[eli-darkly]

[tracked internally as 65728]

[andrao]

Amazing. Thank you Eli!

[eli-darkly]

Btw, thanks for the very detailed code walkthrough - this really could've been a PR, it's pretty much all here.

[andrao]

For what it's worth, it looks like I did forget at least one thing:

In identify(),

.fetchFlagSettings(realUser, hash)

should be:

.fetchFlagSettings(realUser, newHash)

[eli-darkly]

The fix will be in the next releases of all of the client-side JS-based SDKs, which should be pretty soon. As always, please don't try to update launchdarkly-js-sdk-common separately in an application.