Support for creating custom tokens without service account credentials by hiranya911 · Pull Request #285 · firebase/firebase-admin-node

hiranya911 · 2018-05-31T20:35:15Z

Currently the SDK must be initialized with service account credentials in order to be able to call FirebaseAuth.createCustomToken(). With this PR, the SDK will attempt to sign custom tokens by calling the IAM service in the cloud when the service account credentials are not provided.

// Following will work in GCP managed runtimes where the Metadata service is present
admin.initializeApp();
admin.auth().createCustomToken(uid).then(...);

// Following will work anywhere
FirebaseApp.initializeApp({
  // need to only specify the service account email here...
  serviceAccountId: 'test-service-account@project-id.iam.gserviceaccount.com',
});
admin.auth().createCustomToken(uid).then(...);

As a part of this implementation I'm also refactoring the existing FirebaseTokenGenerator abstraction by removing all the token verification logic out of it.

go/firebase-admin-sign
go/firebase-admin-iam-sign

hiranya911 · 2018-05-31T20:59:19Z

Resolves #224

bojeil-google · 2018-06-27T22:01:06Z

src/auth/auth.ts


 import * as utils from '../utils/index';
 import * as validator from '../utils/validator';
+import { FirebaseTokenVerifier, newSessionCookieVerifier, newIdTokenVerifier } from './token-verifier';


Rename newSessionCookieVerifier and newIdTokenVerifier to createSessionCookieVerifier and createIdTokenVerifier.

bojeil-google · 2018-06-27T22:08:02Z

src/auth/token-generator.ts

-    );
+  }
+
+  public sign(buffer: Buffer): Promise<Buffer> {


Add javadoc descriptions to all function declarations.

bojeil-google · 2018-06-27T22:09:53Z

src/auth/token-generator.ts

+   * Cryptographically signs a buffer of data.
+   *
+   * @param {Buffer} buffer The data to be signed.
+   * @returns {Promise<object>} A promise that resolves with a base64-encoded signature.


Promise<Buffer>

bojeil-google · 2018-06-27T22:11:33Z

src/auth/token-generator.ts

+   *
+   * @returns {Promise<string>} A promise that resolves with a service account ID.
+   */
+  getAccount(): Promise<string>;


getAccountId()

bojeil-google · 2018-06-27T22:14:57Z

src/auth/token-verifier.ts

+/**
+ * Creates a new FirebaseTokenVerifier to verify Firebase ID tokens.
+ *
+ * @param projectId Project ID string.


* @param {string} projectId Project ID string. * @return {FirebaseTokenVerifier}

bojeil-google · 2018-06-28T02:48:47Z

test/unit/auth/token-verifier.spec.ts

+
+  it('resolves when underlying idTokenVerifier.verifyJWT() resolves with expected result', () =>  {
+    idTokenVerifier.verifyJWT.withArgs(idToken).returns(Promise.resolve(decodedIdToken));
+    const auth = new Auth(mocks.app());


same here. these tests should not depend on Auth.

These tests basically check the verifyIdToken() and verifySessionCookie() methods on Auth. They are better off in auth.spec.

bojeil-google · 2018-06-28T03:10:36Z

test/unit/auth/token-generator.spec.ts

+      }).to.throw('Must provide a HTTP client to initialize IAMSigner');
+    });
+
+    describe('explicit service account', () => {


'explicit service account id'

bojeil-google · 2018-06-28T03:13:27Z

test/unit/auth/token-generator.spec.ts

+
+    describe('explicit service account', () => {
+      const response = {signature: Buffer.from('testsignature').toString('base64')};
+      const input = Buffer.from('base64');


first encoding argument of toString() and the second argument of Buffer.from is `base64' making this confusing. Change to 'base64string' or something different.

Changed to input

bojeil-google · 2018-06-28T03:20:09Z

test/unit/auth/token-generator.spec.ts

+    });
+
+    describe('auto discovered service account', () => {
+      const input = Buffer.from('base64');


Same here on 'base64' first argument.

bojeil-google · 2018-06-28T03:24:37Z

test/unit/auth/token-generator.spec.ts

+        });
+      });
+
+      it('should return the discovered service account', () => {


Add a test when getAccount() rejects with an error and confirm expected error.

hiranya911 · 2018-06-28T20:17:01Z

I'm posting a decoded custom token generated against the master branch and this branch for comparison. Notice that other than the order of some claims (which has no significance in JSON and JWT), nothing has changed. During the last code review round I noticed that the typ header was missing which I have now added (interestingly, Firebase Auth did not fail on that).

Tokens decoded via https://jwt.io/

JWT Header

Before:

{
  "alg": "RS256",
  "typ": "JWT"
}

After:

{
  "alg": "RS256",
  "typ": "JWT"
}

JWT Body

Before:

{
  "claims": {
    "isAdmin": true
  },
  "uid": "testuser",
  "iat": 1530216676,
  "exp": 1530220276,
  "aud": "https://identitytoolkit.googleapis.com/google.identity.identitytoolkit.v1.IdentityToolkit",
  "iss": "*******@*******.iam.gserviceaccount.com",
  "sub": "*******@*******.iam.gserviceaccount.com"
}

After:

{
  "aud": "https://identitytoolkit.googleapis.com/google.identity.identitytoolkit.v1.IdentityToolkit",
  "iat": 1530216493,
  "exp": 1530220093,
  "iss": "*******@*******.iam.gserviceaccount.com",
  "sub": "*******@*******.iam.gserviceaccount.com",
  "uid": "testuser",
  "claims": {
    "isAdmin": true
  }
}

hiranya911 · 2018-07-02T18:17:25Z

Resolves #224

bojeil-google

Thanks for making the changes. I would have preferred keeping the dependency on jsonwebtoken for minting custom tokens with service accounts but since this is undergoing a launchcal review, I leave the final say to reviewers.

hiranya911 added 11 commits May 16, 2018 16:21

Initial framework for signing tokens remotely

eb5aae1

Separated token generator and verifier

877fc17

Unit tests for service acct signer

2974090

Added integration test

6ac609b

Changed getAccount() to return a Promise

64bfedd

Returning a buffer from sign API

e4d1b11

More unit tests

9ba16e2

Removing unused config

b0a4309

Merge branch 'master' into hkj-jwt-sign

f8bb726

Updated test description

d2c2b89

Cleaning up imports

20a91fa

hiranya911 mentioned this pull request May 31, 2018

admin.auth().createCustomToken() is not correctly returning a rejected promise #284

Closed

hiranya911 added 6 commits May 31, 2018 16:22

Updated test to verify metadata server headers

d31d971

Merge branch 'master' into hkj-jwt-sign

c45ccdf

Renamed serviceAccount to serviceAccountId

7e38847

Merge branch 'master' into hkj-jwt-sign

1063d4a

Using new http API for token generation

c6dac08

Added comments and documentation

55ce947

hiranya911 requested a review from bojeil-google June 20, 2018 23:21

hiranya911 assigned bojeil-google Jun 20, 2018

hiranya911 added 2 commits June 26, 2018 10:39

Merged with master

9d6e50c

Updated changelog

41a313a

bojeil-google suggested changes Jun 28, 2018

View reviewed changes

bojeil-google assigned hiranya911 and unassigned bojeil-google Jun 28, 2018

Responding to code review feedback

c89b96a

hiranya911 requested a review from bojeil-google June 28, 2018 20:19

hiranya911 assigned bojeil-google Jun 28, 2018

hiranya911 removed their assignment Jun 28, 2018

piuccio mentioned this pull request Jun 29, 2018

Error: Can't determine Firebase Database URL. Upgrading functions to v1 #299

Closed

hiranya911 added 2 commits July 2, 2018 15:02

Updated IAM errorm essages

b51a8e3

Updated error message

d396206

ghost approved these changes Jul 4, 2018

View reviewed changes

Merged with master

7602ed8

bojeil-google approved these changes Jul 10, 2018

View reviewed changes

bojeil-google assigned hiranya911 and unassigned bojeil-google Jul 10, 2018

hiranya911 merged commit 0e3311a into master Jul 12, 2018

hiranya911 deleted the hkj-jwt-sign branch July 12, 2018 17:14

rsgowman mentioned this pull request Dec 10, 2019

Allow createCustomToken() to work with tenant-aware auth #708

Merged

Jeremip11 mentioned this pull request Dec 14, 2023

[Snyk] Fix for 1 vulnerabilities Jeremip11/firebase-admin-node#20

Open

takeratta mentioned this pull request Dec 14, 2023

[Snyk] Fix for 1 vulnerabilities takeratta/firebase-admin-node#33

Open

Conversation

hiranya911 commented May 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hiranya911 commented May 31, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hiranya911 commented Jun 28, 2018

JWT Header

JWT Body

Uh oh!

hiranya911 commented Jul 2, 2018

Uh oh!

bojeil-google left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hiranya911 commented May 31, 2018 •

edited

Loading