New Auth Hook: `useSigninCheck()` to replace `AuthCheck` and `ClaimsCheck` components

[jhuleatt]

Fixes #325

Description

Deprecate AuthCheck and ClaimsCheck in favor of a hook that would allow easy implementation of either of those components: useSigninCheck()

Code sample

Simple use case:

function UserFavorites() {
   const {status, data: signInCheckResult} = useSigninCheck();

   if (status === 'loading') {
     return <LoadingSpinner />
   }

   if (signInCheckResult.signedIn === true) {
     return <FavoritesList />
   } else {
     return <SignInForm />
   }
}

Advanced: You can also optionally check custom claims. Example:

function ProductPricesAdminPanel() {
  const { status, data: signInCheckResult } = useSigninCheck({
    requiredClaims: { admin: true, canModifyPrices: true },
  });

  if (status === "loading") {
    return <LoadingSpinner />;
  }

  if (signInCheckResult.signedIn && signInCheckResult.hasRequiredClaims) {
    return <FavoritesList />;
  } else {
    console.warn(
      `User ${signInCheckResult.user.displayname} is missing claims:`,
      Object.keys(signInCheckResult.errors)
    );
    return <SignInForm />;
  }
}

Why deprecate AuthCheck and ClaimsCheck?

They worked well in the previous version of ReactFire when ReactFire only supported concurrent mode, but are awkward now that we support both concurrent mode and non-concurrent mode (added in https://github.com/FirebaseExtended/reactfire/pull/255). Moving to a hook allows for the same easy checks, but leaves loading states to the developer, instead of AuthCheck and ClaimsCheck trying to handle both concurrent mode and non-concurrent mode loading strategies.

[jhuleatt]

Open question: Should the SigninCheckResult object returned by useSigninCheck contain the current user too? Currently it has:

interface MissingClaims {
  [key: string]: { expected: string; actual: string };
}

export type SigninCheckResult =
  | {
      signedIn: false;
      hasRequiredClaims: false;
    }
  | {
      signedIn: true;
      hasRequiredClaims: boolean;
      missingClaims?: MissingClaims;
    };

With the user, it could look like:

interface MissingClaims {
  [key: string]: { expected: string; actual: string };
}

export type SigninCheckResult =
  | {
      signedIn: false;
      hasRequiredClaims: false;
    }
  | {
      signedIn: true;
      hasRequiredClaims: boolean;
      missingClaims?: MissingClaims;
      user: firebase.User;
    };

Or should we leave that to useUser?

[jhuleatt]

Open question: Should the SigninCheckResult object returned by useSigninCheck contain the current user too?

Or should we leave that to useUser?

After a chat with @thomasmburke, I think the answer is that we should add it. useSigninCheck gets the current user anyway so that it can check claims, so it will be trivial to implement. And there may be some use case where it's nice to not have to call useUser alongside useSigninCheck.

[aalises]

Amazing! How would you go about allowing a claim to be a set of values? I have this role claim that can be a fixed set of string values e.g. ADMIN, CUSTOMER.

I am using a custom implementation of ClaimsCheck where you can allow an array of values to be passed for a claim and validates if one of them passes.

For example, I would want a feature to only be available for admins and also some other special users, so I could do:

{role: ["ADMIN", "SPECIAL_USER"]}

so both users with admin and special_user role claim would be validated.

In the current implementation only one value can be checked by custom claim... how would you guys contemplate this case?

[jhuleatt]

How would you go about allowing a claim to be a set of values? I have this role claim that can be a fixed set of string values e.g. ADMIN, CUSTOMER.

In the current implementation only one value can be checked by custom claim... how would you guys contemplate this case?

Thanks for the thoughtful feedback @aalises! Maybe we should allow a validator function to be passed to useSigninCheck as an alternative to a requiredClaims object if more advanced validation logic is needed. In your case, it would let you do something like:

const { status, data: signInCheckResult } = useSigninCheck({
  checkClaims: (userClaims) => {
    const validClaimsSet = ["ADMIN", "SPECIAL_USER"];
    let hasAnyClaim = false;

    for (const claim of validClaimsSet) {
      if (userClaims[claim] === true) {
        hasAnyClaim = true;
        break;
      }
    }

    // To populate the `SigninCheckResult` object
    return {
      hasRequiredClaims: hasAnyClaim,
      missingClaims: hasAnyClaim ? undefined : validClaimsSet,
    };
  },
});

What do you think?

[aalises]

That'd be great @jhuleatt !

[MrHazimAli]

hye @jhuleatt , may I know if there is any eta on new version include this PR? 👍 looking forward to it :-)

[jhuleatt]

Hi @MrHazimAli, we're waiting on #372 before publishing a new release candidate, but in the meantime you can try out the new hooks with a canary version: npm install reactfire@canary