firebase / firebase-js-sdk

Firebase Javascript SDK
https://firebase.google.com/docs/web/setup
Other
4.84k stars 889 forks source link

Link Saml provider to existing one - SAML providers are considered authoritative. #8016

Closed flea89 closed 6 months ago

flea89 commented 8 months ago

Operating System

14.2.1

Browser Version

Chrome 121.0.6167.85

Firebase SDK Version

10.8.0

Firebase SDK Product:

Auth

Describe your project's tooling

Worth mentioning we proxy request as described here.

For testing purposed we have 2 different SAML identity Providers Auth0 and JumpCloud.

Describe the problem

SAML providers are treated as authoritative, while they probably shouldn't. Specifically when a user account already exists (created either with user/password or Gooogle sign-in), and the same user tries to sign-in using the same email address through a SAMLProvider (single sign-on), if the sign-in is successful on the identity provider, the account is automatically linked with the existing account and auth/account-exists-with-different-credential is not raised.

This doesn't seem to reflect what's described in the documentation:

when a user tries to sign in to a provider (such as SAML) with an email that already exists for another provider (such as Google), the error auth/account-exists-with-different-credential is thrown (along with an AuthCredential object).

While I'm not an expert, this seems to be to be quite problematic from a security perspective.

Think about this scenario

Hypothesis

Steps

  1. UserA creates an account on MyApp with email userA@gmail.com and passwordA.
  2. Attacker creates an account on theSingleSignOnApp using userA@gmail.com and a passwordB.
  3. Attacker tries to log into MyApp using SSO
  4. Attacker can authenticate on SingleSignOnApp because they own the account
  5. On MyApp the accounts are automatically linked and now Attacker controls UserA account.

While this is true only if the IDP doesn't have email verification enabled, that can't be controlled and verified by the app. To me, it looks the same problem described for Social Providers, where they can't trusted so confirming the identity before linking is required.

Steps and code to reproduce issue

  1. Enable and set up a Saml provider authentication from the console.
  2. Create a user with createUserWithEmailAndPassword [email: user_a@gmail.com]
  3. Make sure user has verified their email
  4. user_a@gmail.com signs in using SSO with the same email address user_a@gmail.com
    
    const samlProvider = new SAMLAuthProvider(<ssoProviderId>);
    signInWithRedirect(auth, samlProvider);

...

const userCredential = await getRedirectResult(auth)


  6. `auth/account-exists-with-different-credential` is not raised and Providers  are linked automatically
NhienLam commented 8 months ago

Hi @flea89, thank you for reporting this!

This is an expected behavior. This documentation needs to be updated.

When account linking is enabled for the project, the behavior is:

  1. When user signs in with a trusted provider then signs in with a different trusted provider with the same email, both providers will be linked without errors. In your case, Email/Password with email verification (trusted) followed by SAML (trusted).

  2. When user signs in with a trusted provider then signs in with untrusted provider with the same email (for example, Google followed by Facebook), auth/account-exists-with-different-credential will be thrown.

IdPs that either own the domain or always require verification are considered trusted. SAML is considered as trusted. You can find the list of trusted providers at https://firebase.google.com/docs/auth/users#verified_email_addresses.

I will keep this bug open to track the doc update.

flea89 commented 8 months ago

Hi @NhienLam, thanks a lot for your response. The way account linking works does make sense to me.

What I'm wondering is if trusting a SAML provider is correct decision in the first place.

Specifically I don't think we can consider this statement true for any IDP.

IdPs that either own the domain or always require verification are considered trusted

As an example I've created an app on Auth0 and enabled the SSO integration. I've successfully set it up as SAML provider for my Firebase application.

But by default Auth0 doesn't enforce email verification and emails with any domain can be added to the Auth0 app user list.

So, from what I can tell it's possible to successfully authenticate without having verified the email in the first place. That opens up to the problems described in the issue description, and makes SAML providers liable to the same security flaws social providers have if linking is automatic, doesn't it?

I reproduced the same problem by setting up JumpCloud as IDP.

Let me know if you need a demo or any more details.

ps: I don't have a lot of experience setting up an IDP and SSO integrations.
My setup is pretty basic, and I know it's possible to enforce a stricter security requirements. At the same time I don't think firebase can "trust" that to be the case. That's especially true in the context of a multi-tennant application where different IDPs are enabled and they're not necessarily controlled by the app developers.

flea89 commented 8 months ago

Any updates on this one?

I'm worried this is quite a serious security hole 😇

NhienLam commented 8 months ago

Hi @flea89, I discussed with the team. Your concern is valid.

If you add a SAML provider which doesn't verify email address, can you try using the blocking function to mark the email as unverified - https://cloud.google.com/identity-platform/docs/blocking-functions#treating_certain_identity_provider_emails_as_verified ? Treating SAML provider emails as unverified should make SAML an untrusted provider. Hence, the behavior should now be:

When user signs in with a trusted provider then signs in with untrusted provider with the same email (for example, Google followed by Facebook), auth/account-exists-with-different-credential will be thrown.

flea89 commented 8 months ago

@NhienLam thanks for getting back.

I reckon not trusting SAML providers should be the default thought. As you pointed out, the behaviour could still be overridden by https://cloud.google.com/identity-platform/docs/blocking-functions#treating_certain_identity_provider_emails_as_verified.

Do you have plans to fix it anytime soon?


Regarding your suggested workaround, it doesn't seem to work, by the time the beforeUserSignedIn the SamlProvider is already linked to the user and auth/account-exists-with-different-credential is not thrown.

const logger = require("firebase-functions/logger");
const {beforeUserSignedIn} = require("firebase-functions/v2/identity")
const gcipCloudFunctions = require("gcip-cloud-functions");

const authClient = new gcipCloudFunctions.Auth();

/**
 * This function is exceuted before the user is signed in.
 * It checks if the user is trying to sign in with a SAML provider and if the user is not yet
 * linked to the SAML provider.
 * It sets their email to false, so that the user is required to confirm their email address.
 *
 *
 * @param {import("firebase-functions/v2/identity").AuthBlockingEvent} event
 * @returns {Promise<import("firebase-functions/lib/common/providers/identity").BeforeSignInResponse|void>}
 */
async function beforeSignInCb (event) {
  const providerId = event.additionalUserInfo?.providerId
  logger.info(`ProviderId: ${providerId}`)

  if(!providerId) {
    logger.warn("No providerId found")
    return;
  }

  const user = event.data;
  const linkedProviders = user.providerData.map((p) => p.providerId)
  logger.info(`Linked providers: ${linkedProviders.join(", ")}, set emailVerified to false`);

  if(providerId.startsWith("saml")) {
    logger.info(`Provider <${providerId}> is a SAML provider, setting emailVerified to false`)

    return {
      emailVerified: false,
    }
  }
}

exports.beforesignin = beforeUserSignedIn(beforeSignInCb)

Logs:

INFO ProviderId: saml.qy********
INFO Linked providers: google.com, saml.qy********
INFO Provider <saml.qy********> is a SAML provider, setting emailVerified to false

The exception is not thrown. user.emailVerified appears to be false, as expected.

Obviously the provided snapshot is naive since it sets the emailVerified to always be false when signing in with SAML. In a real world scenario, after confirming the email, emailVerified would be True.

Am I missing something? Can you advise?

flea89 commented 7 months ago

Good morning, any updates on this one?

NhienLam commented 6 months ago

Sorry for the late reply. I was out of office for the past weeks.

After discussing with our team, we currently don't have plans to change the current behavior and continue treating SAML providers as trusted. We recommend not using SAML providers that you don't trust.