Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(app-check): adds all web providers as an option #812

Merged
merged 12 commits into from
Feb 6, 2025

Conversation

eljass
Copy link
Contributor

@eljass eljass commented Jan 27, 2025

Pull request checklist

Please check if your PR fulfills the following requirements:

  • The changes have been tested successfully.
  • A changeset has been created (npm run changeset).
  • I have read and followed the pull request guidelines.

Adds option to use different attestation providers in web. This adds support for reCaptcha Enterprise and Custom provider in addition to existing reCaptcha v3.

Close #811

@robingenz
Copy link
Member

Thank you. I'm currently working on the update to Capacitor 7. I will soon take a look at your PR.

@eljass eljass requested a review from robingenz February 3, 2025 15:22
.changeset/chilly-squids-hammer.md Outdated Show resolved Hide resolved
packages/app-check/src/definitions.ts Outdated Show resolved Hide resolved
packages/app-check/src/definitions.ts Outdated Show resolved Hide resolved
packages/app-check/src/definitions.ts Outdated Show resolved Hide resolved
packages/app-check/src/web.ts Outdated Show resolved Hide resolved
@robingenz
Copy link
Member

@eljass Wouldn't it be easier to let the user pass the provider directly?

import { AppCheckProvider } from 'firebase/app-check';

export interface InitializeOptions {
  ...
  provider: AppCheckProvider?
}

@eljass
Copy link
Contributor Author

eljass commented Feb 4, 2025

@eljass Wouldn't it be easier to let the user pass the provider directly?

import { AppCheckProvider } from 'firebase/app-check';

export interface InitializeOptions {
  ...
  provider: AppCheckProvider?
}

I was thinking this, but in case there would be new provider presented by Firebase before it is implemented in this plugin I think it was clearer to limit to ones currently tested and supported.

Also, with this typing it requires user to initialize the provider like:

import { ReCaptchaEnterpriseProvider } from 'firebase/app-check'
initializeAppCheck({
  provider: ReCaptchaEnterpriseProvider,
  siteKey: "abcd"
})

This would work and make the plugin's provider selection cleaner, but at the same time it makes it harder to check if required options are present (such as customProviderOptions). But let me know which way you see preferred.

@robingenz
Copy link
Member

robingenz commented Feb 4, 2025

@eljass I thought we could do it like that:

import { FirebaseAppCheck } from '@capacitor-firebase/app-check';
import { AppCheckProvider } from 'firebase/app-check';

const initialize = async () => {
  const provider = new ReCaptchaV3Provider("..."); // User creates the provider with all options
  await FirebaseAppCheck.initialize({
    provider: provider // No `customProviderOptions` are needed
  });
};

Or are there any limitations?

@eljass eljass requested a review from robingenz February 4, 2025 09:40
@eljass
Copy link
Contributor Author

eljass commented Feb 4, 2025

@robingenz let me refresh my memory and test it out that way.

@eljass
Copy link
Contributor Author

eljass commented Feb 4, 2025

@eljass I thought we could do it like that:

import { FirebaseAppCheck } from '@capacitor-firebase/app-check';
import { AppCheckProvider } from 'firebase/app-check';

const initialize = async () => {
  const provider = new ReCaptchaV3Provider("..."); // User creates the provider with all options
  await FirebaseAppCheck.initialize({
    provider: provider // No `customProviderOptions` are needed
  });
};

Or are there any limitations?

@robingenz
The issue I encounter here is that the types do not match for some reason. I assume the Firebase packages need to match perfectly in order for types to work.

CleanShot 2025-02-04 at 12 33 10

CleanShot 2025-02-04 at 12 33 37

For some reason I cannot make it work either with direct type exports:

/**
   * Set used app check provider for Web.
   * 
   * Only available for Web.
   *
   * Read more: https://firebase.google.com/docs/app-check/web/custom-provider
   * 
   * @since 7.1.0
   * @default ReCaptchaV3Provider
   */
  provider: ReCaptchaEnterpriseProvider | ReCaptchaV3Provider | CustomProvider;

Don't know if its my environment or what.

@robingenz
Copy link
Member

I just realized we are not allowed to import types from the Firebase JS SDK anyway, since the Firebase JS SDK is only an optional dependency of this plugin. You can just set the type from provider to any if you want:

export interface InitializeOptions {
  /**
     * The provider to use for App Check. Must be an instance of 
     * `ReCaptchaV3Provider`, `ReCaptchaEnterpriseProvider`, or `CustomProvider`.
     * 
     * Only available for Web.
     * 
     * @since 7.1.0
     * @see https://firebase.google.com/docs/app-check/web/custom-provider
     */
    provider?: any;
}

That's not best practice, but in this case it would be fine with me.

@eljass
Copy link
Contributor Author

eljass commented Feb 5, 2025

@robingenz How do you feel runtime validations then? Should we check something like this for example? Or do we just trust that the errors from Firebase SDK is enough if invalid provider is set?

const isValidProvider = (provider: unknown): provider is AppCheckOptions['provider'] => {
  return (
    provider instanceof ReCaptchaV3Provider ||
    provider instanceof ReCaptchaEnterpriseProvider ||
    provider instanceof CustomProvider
  )
}

export class FirebaseAppCheckWeb
  extends WebPlugin
  implements FirebaseAppCheckPlugin
{
  // ...other things

  public async initialize(options?: InitializeOptions): Promise<void> {
    // ...other things
    let provider = options?.provider
    if (!!provider) {
      if (!!provider && !isValidProvider(provider)) {
        throw new Error('Invalid provider')
      }
    } else {
      if (!options?.siteKey) throw new Error(FirebaseAppCheckWeb.errorSiteKeyMissing)
      provider = new ReCaptchaV3Provider(options?.siteKey)
    }
    this.appCheckInstance = initializeAppCheck(app, {
      provider,
      isTokenAutoRefreshEnabled: options.isTokenAutoRefreshEnabled,
    });
  }
  
  // ...rest of other things 
}

@robingenz
Copy link
Member

I want to keep it simple at the moment. Let the Firebase JS SDK do the validation.

@eljass
Copy link
Contributor Author

eljass commented Feb 5, 2025

@robingenz suggested way implemented now

Copy link

pkg-pr-new bot commented Feb 5, 2025

Open in Stackblitz

@capacitor-firebase/analytics

npm i https://pkg.pr.new/capawesome-team/capacitor-firebase/@capacitor-firebase/analytics@812

@capacitor-firebase/app

npm i https://pkg.pr.new/capawesome-team/capacitor-firebase/@capacitor-firebase/app@812

@capacitor-firebase/app-check

npm i https://pkg.pr.new/capawesome-team/capacitor-firebase/@capacitor-firebase/app-check@812

@capacitor-firebase/authentication

npm i https://pkg.pr.new/capawesome-team/capacitor-firebase/@capacitor-firebase/authentication@812

@capacitor-firebase/crashlytics

npm i https://pkg.pr.new/capawesome-team/capacitor-firebase/@capacitor-firebase/crashlytics@812

@capacitor-firebase/firestore

npm i https://pkg.pr.new/capawesome-team/capacitor-firebase/@capacitor-firebase/firestore@812

@capacitor-firebase/functions

npm i https://pkg.pr.new/capawesome-team/capacitor-firebase/@capacitor-firebase/functions@812

@capacitor-firebase/messaging

npm i https://pkg.pr.new/capawesome-team/capacitor-firebase/@capacitor-firebase/messaging@812

@capacitor-firebase/performance

npm i https://pkg.pr.new/capawesome-team/capacitor-firebase/@capacitor-firebase/performance@812

@capacitor-firebase/remote-config

npm i https://pkg.pr.new/capawesome-team/capacitor-firebase/@capacitor-firebase/remote-config@812

@capacitor-firebase/storage

npm i https://pkg.pr.new/capawesome-team/capacitor-firebase/@capacitor-firebase/storage@812

commit: dec4dd0

packages/app-check/src/web.ts Outdated Show resolved Hide resolved
packages/app-check/src/web.ts Outdated Show resolved Hide resolved
packages/app-check/src/definitions.ts Outdated Show resolved Hide resolved
packages/app-check/src/definitions.ts Outdated Show resolved Hide resolved
packages/app-check/README.md Outdated Show resolved Hide resolved
@eljass eljass requested a review from robingenz February 6, 2025 08:03
@robingenz robingenz merged commit 7f6117e into capawesome-team:main Feb 6, 2025
4 checks passed
@robingenz
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: support all the providers available for web
2 participants