ably-js icon indicating copy to clipboard operation
ably-js copied to clipboard

How can I handle signed out users?

Open nandorojo opened this issue 1 year ago • 16 comments

I am using the authCallback option. What if a user isn't signed in? Should I just call the callback with an error? I'm doing this right now:

import { AblyProvider as Provider } from 'ably/react'
import * as Ably from 'ably'
import { useMemo } from 'react'

export function AblyProvider({ children }) {
  const auth = useAuth().user
  return (
    <Provider
      client={useMemo(() => {
        return new Ably.Realtime.Promise({
          useTokenAuth: true,
          async authCallback(data, callback) {
            if (!auth) {
              return callback('No auth', null)
            }
            try {
              const result = await getAblyAuth()

              return callback(null, result)
            } catch (e) {
              return callback(e, null)
            }
          },
        })
      }, [auth])}
    >
      {children}
    </Provider>
  )
}

Is this right?

┆Issue is synchronized with this Jira Task by Unito

nandorojo avatar Oct 04 '23 15:10 nandorojo

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-3887

sync-by-unito[bot] avatar Oct 04 '23 15:10 sync-by-unito[bot]

@nandorojo as per documentation -> https://github.com/ably/ably-js/blob/89589f1d5a0ef43aec380f615966ee924584e4d0/src/common/lib/client/auth.ts#L411

   *
   * - authCallback:  (optional) a JavaScript callback to be called to get auth information.
   *                  authCallback should be a function of (tokenParams, callback) that calls
   *                  the callback with (err, result), where result is any of:
   *                  - a tokenRequest object (ie the result of a rest.auth.createTokenRequest call),
   *                  - a tokenDetails object (ie the result of a rest.auth.requestToken call),
   *                  - a token string

So yes, non-null argument in error should work. You might like to pass an error object in instead of string error though.

sacOO7 avatar Oct 04 '23 15:10 sacOO7

Screenshot 2023-10-04 at 1 07 56 PM

Thanks for the response @sacOO7.

This solution is a bit undesireable though, as it's throwing an error even when I'm wrapping it with try, and even when it isn't actually an error. Signed out users is an intended state.

nandorojo avatar Oct 04 '23 17:10 nandorojo

@nandorojo try passing standard js error object instead

sacOO7 avatar Oct 04 '23 18:10 sacOO7

image

This appears to fail types (unless I did this incorrectly).

nandorojo avatar Oct 04 '23 18:10 nandorojo

I also notice that that solution:

  1. continues to throw errors
  2. Continually throws on React Native, even after someone has signed in

nandorojo avatar Oct 04 '23 20:10 nandorojo

image

For reference.

nandorojo avatar Oct 04 '23 20:10 nandorojo

I tried using the authUrl too, like this:

authUrl: [OpenAPI.BASE, '/api/v1/rest/chat/ably/auth'].join(''),
authHeaders: {
  ...(token && {
    Authorization: `Bearer ${token}`,
  }),
},

But of course, this throws when there is no token. How can I tell ably to not hit the auth endpoint until there is a token?

nandorojo avatar Oct 04 '23 20:10 nandorojo

@sacOO7 should I be using the key prop as a fallback? It was my understanding that this was a server-only secret. Is the key safe to use on the client?

nandorojo avatar Oct 04 '23 21:10 nandorojo

I tried this approach too with autoConnect, but it still hits the server when there is no token.

return new Ably.Realtime.Promise({
  useTokenAuth: true,
  authUrl: [OpenAPI.BASE, '/api/v1/rest/chat/ably/auth'].join(''),
  autoConnect: Boolean(token),
  authHeaders: {
    ...(token && {
      Authorization: `Bearer ${token}`,
    }),
  },

nandorojo avatar Oct 04 '23 23:10 nandorojo

Hey, just wondering if there's a solution here. I tried using a fake token when users are logged out but this throws an error. How can I just tell ably that I don't want to connect yet?

nandorojo avatar Oct 10 '23 15:10 nandorojo

Hey @nandorojo, thanks for reaching out!

Your usage of authCallback looks correct to me, and calling the provided callback with an error or the 'no auth' string will never throw an exception (this is why the try/catch block doesn't do anything). Furthermore the client shouldn't automatically connect when the autoConnect prop is falsy.

For the initial runtime error, it sounds like somewhere in your app a separate call which requires authentication is being made which, in turn, triggers the client to call the authCallback. For this, the stack trace of the error should hopefully give some indication of what is happening. If you believe this to be caused by the library itself please share the stack trace here and I can have a look.

As for what to do when the client app does not yet have a token, you're correct that using basic auth on the client side is not safe. The best pattern for not connecting if you don't have a token is probably to just not create a client unless you already have a token ready.

I hope this helps, let me know if you have any further questions 🙂

owenpearson avatar Oct 12 '23 08:10 owenpearson

Hey thanks for the response.

For some reason, 'no auth' was always throwing an error. I was trying but ultimately failed to get it working with the Ably hooks/provider, so I forked them and made my own, where I conditionally pass an ably instance to the provider, and only create the connection in the useChannel hook if it's ready.

I wanted to call useChannel even before the user has signed in, since I am showing existing data while the user signs in / connects to authentication. And then I want it to ignore starting up if the user is not signed in. This is useful for a chat app that supports offline mode too.

Here's how I solved this for our case. I tried all the options from the SDK but just couldn't get an optional ably instance to work without throwing (or getting my auth to work conditionally).

import { Realtime, type Types } from 'ably'
import { createContext, useContext, useMemo } from 'react'

const context = createContext<Types.RealtimePromise | undefined>(undefined)

export function AblyProvider({ children }) {
  const hasOnboarded = useHasOnboarded() // pseudo hook
  const ably = useMemo(() => {
    if (hasOnboarded) { // conditional ably instance passed to content
      return new Realtime.Promise({
        async authCallback(data, callback) {
          try {
            const result = await ablyAuthentication()

            return callback(null, result)
          } catch (e) {
            try {
              return callback(e, null)
            } catch {}
          }
        },
      })
    }
  }, [hasOnboarded])
  return <context.Provider value={ably}>{children}</context.Provider>
}

export function useAblyContext() {
  return useContext(context)
}

Then I forked useChannel to allow for an optional instance of ably:

// to allow optional ably instance from custom context for signed out users
import { useEffect, useMemo, useRef } from 'react'
import { ChannelParameters } from 'ably/src/platform/react-hooks/src/AblyReactHooks'
import { Types } from 'ably/ably'
import { useAblyContext } from './context' // ⚠️ custom hook just created

export type AblyMessageCallback = Types.messageCallback<Types.Message>

export interface ChannelResult {
  channel: Types.RealtimeChannelPromise
  ably: Types.RealtimePromise
  connectionError: Types.ErrorInfo | null
  channelError: Types.ErrorInfo | null
}

type SubscribeArgs = [string, AblyMessageCallback] | [AblyMessageCallback]

export function useAblyChannel(
  channelNameOrNameAndOptions: ChannelParameters,
  callbackOnMessage?: AblyMessageCallback
): ChannelResult
export function useAblyChannel(
  channelNameOrNameAndOptions: ChannelParameters,
  event: string,
  callbackOnMessage?: AblyMessageCallback
): ChannelResult

export function useAblyChannel(
  channelNameOrNameAndOptions: ChannelParameters,
  eventOrCallback?: string | AblyMessageCallback,
  callback?: AblyMessageCallback
) {
  const channelHookOptions =
    typeof channelNameOrNameAndOptions === 'object'
      ? channelNameOrNameAndOptions
      : { channelName: channelNameOrNameAndOptions }

  const ably = useAblyContext()

  const { channelName, options: channelOptions, skip } = channelHookOptions

  const channelEvent =
    typeof eventOrCallback === 'string' ? eventOrCallback : null
  const ablyMessageCallback =
    typeof eventOrCallback === 'string' ? callback : eventOrCallback

  const channelOptionsRef = useRef(channelOptions)
  const ablyMessageCallbackRef = useRef(ablyMessageCallback)

  const channel = useMemo(
   // ⚠️ let the channel be optional!
    () => ably?.channels.get(channelName, channelOptionsRef.current),
    [ably, channelName]
  )

  useEffect(() => {
    if (!channel) return

    if (channelOptionsRef.current !== channelOptions && channelOptions) {
      channel.setOptions(channelOptions)
    }
    channelOptionsRef.current = channelOptions
  }, [channel, channelOptions])

  useEffect(() => {
    ablyMessageCallbackRef.current = ablyMessageCallback
  }, [ablyMessageCallback])

  useEffect(() => {
    if (!channel) return // ⚠️ bail out if the channel is undefined

    const listener: AblyMessageCallback | null = ablyMessageCallbackRef.current
      ? (message) => {
          ablyMessageCallbackRef.current &&
            ablyMessageCallbackRef.current(message)
        }
      : null

    const subscribeArgs: SubscribeArgs | null = listener
      ? channelEvent === null
        ? [listener]
        : [channelEvent, listener]
      : null

    if (!skip && subscribeArgs) {
      handleChannelMount(channel, ...subscribeArgs)
    }

    return () => {
      !skip && subscribeArgs && handleChannelUnmount(channel, ...subscribeArgs)
    }
  }, [channelEvent, channel, skip])

  return { channel, ably }
}

async function handleChannelMount(
  channel: Types.RealtimeChannelPromise,
  ...subscribeArgs: SubscribeArgs
) {
  await (channel.subscribe as any)(...subscribeArgs)
}

async function handleChannelUnmount(
  channel: Types.RealtimeChannelPromise,
  ...subscribeArgs: SubscribeArgs
) {
  await (channel.unsubscribe as any)(...subscribeArgs)

  setTimeout(async () => {
    if (channel.listeners.length === 0) {
      await channel.detach()
    }
  }, 2500)
}

nandorojo avatar Oct 12 '23 15:10 nandorojo

Hey @nandorojo, thanks for providing the code!

It's hard to say without seeing a full repro or logs but I'm relatively certain that calling useChannel will be what causes the 'no auth' error to be thrown. Ideally it would be best to ensure that a client isn't created until it's ready to authenticate, however if you do need to call useChannel without an authenticated client we provide a skip parameter to make it so that you can conditionally skip execution of the hook, example usage would be:

useChannel({
  channelName: 'my_channel',
  skip: !token,
}, (msg) => {
  console.log(msg);
});

If you do this you may still encounter the "token request signing call returned error" console warning. I know console warnings can be a bit intrusive in react-native so it might be worth also turning off logging from the client:

const client = new Ably.Realtime.Promise({
  ...options,
  log: {
    level: 0,
  },
});

owenpearson avatar Oct 12 '23 21:10 owenpearson

Yeah I originally used the skip feature but got tons of warnings so I opted for this instead. So maybe I could just disable logs like you mentioned (though I'd want them for the useful cases)

Maybe skip could be used to avoid creating the channel instance inside of useChannel too?

nandorojo avatar Oct 12 '23 21:10 nandorojo

oh interesting, what were the warnings? creating the channel instance doesn't do any network i/o and won't throw an error or anything so avoiding that shouldn't change the behaviour

owenpearson avatar Oct 12 '23 21:10 owenpearson