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

ServiceAccount interface definition errors #522

Closed
dgobaud opened this issue May 7, 2019 · 10 comments
Closed

ServiceAccount interface definition errors #522

dgobaud opened this issue May 7, 2019 · 10 comments

Comments

@dgobaud
Copy link

dgobaud commented May 7, 2019

the ServiceAccount interface specifies the member variables using camel case but they come in from Google Cloud JSON download as snake case and if changed in the JSON file to match the interface, when exporting the JSON file path in GOOGLE_APPLICATION_CREDENTIALS to work with other Google Cloud services locally in the emulator https://firebase.google.com/docs/functions/local-emulator#set_up_admin_credentials_optional it gives error {"result":"The incoming JSON object does not contain a client_email field"}

interface ServiceAccount {

  interface ServiceAccount {
    projectId?: string;
    clientEmail?: string;
    privateKey?: string;
  }
@google-oss-bot
Copy link

I found a few problems with this issue:

  • I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.
  • This issue does not seem to follow the issue template. Make sure you provide all the required information.

@hiranya911
Copy link
Contributor

ServiceAccount interface is used by the admin.credentials.cert() function. And that function is programmed to accept both camel case and snake case attributes. Therefore you don't have to modify downloaded service account JSON files to satisfy the interface. If you do it will most certainly break the OAuth flow.

One of the following should work for pretty much any use case:

// loading from file system
admin.credentials.cert('path/to/file.json');
// loading from the env (json contents in variable)
admin.credential.cert(process.env['GOOGLE_APPLICATION_CREDENTIALS']);
// loading from the env (file path in variable)
const obj = require(process.env['GOOGLE_APPLICATION_CREDENTIALS']);
admin.credential.cert(obj);

@MateusZitelli
Copy link

MateusZitelli commented May 7, 2019

@hiranya911 - I am facing this issue when trying something similar to the last code snipped you posted.

import serviceAccount from "./serviceAccountKey.json";
admin.credential.cert(serviceAccount)

Here is the linter message:

Argument of type '{ "type": string; "project_id": string; "private_key_id": string; "private_key": string; "client_email": string; "client_id": string; "auth_uri": string; "token_uri": string; "auth_provider_x509_cert_url": string; "client_x509_cert_url": string; }' is not assignable to parameter of type 'string | ServiceAccount'.
  Type '{ "type": string; "project_id": string; "private_key_id": string; "private_key": string; "client_email": string; "client_id": string; "auth_uri": string; "token_uri": string; "auth_provider_x509_cert_url": string; "client_x509_cert_url": string; }' has no properties in common with type 'ServiceAccount'.

Basically it expects camel case keys, while the json file come in snake case, as described by @dgobaud .

@hiranya911
Copy link
Contributor

hiranya911 commented May 8, 2019

Just require() the file instead of importing it (also see my examples above):

const serviceAccount = require('./serviceAccountKey.json');
admin.credential.cert(serviceAccount);

If you insist on importing, then you should do something like the following:

import serviceAccount from "./serviceAccountKey.json";
admin.credential.cert({
  privateKey: serviceAccount.private_key,
  clientEmail: serviceAccount.client_email,
  projectId: serviceAccount.project_id,
});

@dgobaud
Copy link
Author

dgobaud commented May 8, 2019

But this must be a bug right? We want the type checking shouldn't the type match the data?

@hiranya911
Copy link
Contributor

It seems a conscious effort has been made to keep snake-cased attributes out of the typings. I'm not sure why, but I guess it's a style-related choice made by the original developers of this library. This interface is quite old (last modified over 2 years ago), and it hasn't changed since I joined the project.

We can add the snake-cased properties to the interface as a solution. But given that there are simple workarounds, and this is the first time this issue has come up in over 2 years, I'm not sure if it's even necessary.

Does the problem occur only when the service account json is imported as a module? Any other ways to reproduce the issue?

@dgobaud
Copy link
Author

dgobaud commented May 8, 2019

I think think this is it and thinking about it more I think you have a good enough point it isn't necessary the JSON format match the interface so probably can leave it. Below I think makes sense as the way to do it.

import serviceAccount from "./serviceAccountKey.json";
admin.credential.cert({
  privateKey: serviceAccount.private_key,
  clientEmail: serviceAccount.client_email,
  projectId: serviceAccount.project_id,
});

@diogocapela
Copy link

This seems to work:

import admin from 'firebase-admin';
import serviceAccount from './serviceAccountKey.json';

admin.initializeApp({
  credential: admin.credential.cert(serviceAccount as admin.ServiceAccount),
  databaseURL: 'https://twisteringo-310c3.firebaseio.com',
});

@charles-allen
Copy link

charles-allen commented Dec 7, 2020

TL;DR: The types are wrong in the SDK, but instead of just fixing them, you're advising every single (TypeScript) client of your SDK to hack their code.

Given you say that the function actually does support snake_case, why don't you just fix the type?

May I PR this?

export interface ServiceAccountSnake {
  project_id?: string
  client_email?: string
  private_key?: string
}

...

function cert(
    serviceAccountPathOrObject: string | _admin.ServiceAccount | _admin.ServiceAccountSnake,
    httpAgent?: Agent): admin.credential.Credential;

@charles-allen
Copy link

This seems to work:

  credential: admin.credential.cert(serviceAccount as admin.ServiceAccount),

This is the cleanest hack today, but don't forget that it is a straight-up lie! You're asserting to TypeScript that your snake_case config is camelCase. You get away with it because the real JS implementation doesn't care what TS thinks & happily accepts snake_case. If the SDK were to drop snake_case support in the future, your code would runtime error (TypeScript would continue to trust your assertion).

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