Skip to content
This repository has been archived by the owner on Mar 29, 2022. It is now read-only.

implement customToken auth #15

Merged
merged 1 commit into from
Oct 26, 2015
Merged

Conversation

jamestalmage
Copy link
Contributor

closes #8

Still Missing:

Mirror Server Error Responses for authWithCustomToken in the following Scenarios

  • Invalid JWT - it is not a valid JWT token
  • Invalid Signature - wrong secret
  • Too Early - the server time is before notBefore
  • Too Late - the server time is after expires
  • No uid property - firebase-token-generator the docs say you are required to provide a uid as part of authData. That is a firebase specific requirement (it will not be validated by a generic JWT library like jwt-simple).
  • No iat property - I do not actually know if this is required, but firebase-token-generator automatically adds it (it is the time the token was created).

Mirror Server Action as Token Reaches Expiration

We need to figure out what happens when a token expires.

  • Are you just switched to "unauthenticated" mode?
  • Is a message sent to idle clients?
  • How is ref.onAuth() triggered as your token expires?

Additional Items / Housekeeping

  • Update documentation
  • more descriptive methods: setSecret => setAuthSecret
  • option to disable expires / notBefore checks
  • option to disable token signature check (setSecret(null))

Object.setPrototypeOf(getTime, TestableClock.prototype);
} else {
getTime.__proto__ = TestableClock.prototype; // eslint-disable-line no-proto
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the deal with all the prototype trickery? Isn't there a simpler way to achieve this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It gives you a passing instanceOf TestableClock check.
We can probably just drop all that, since typeof new TestableClock() === 'function'.

@urish
Copy link
Owner

urish commented Oct 26, 2015

Overall, great work, thank you so much :-)

Two general notes:

  1. Can you please also update the README.md file to mention the new methods (setSecret, setTime)?
  2. Let's try to keep test description more TDD like (see my example above)

Many thanks, I am really excited to see all these changes coming in 👍

@jamestalmage
Copy link
Contributor Author

Works for me.

As for the complexity notes, I say merge this and create a new issue to reduce complexity.
We can bounce around ideas there.

UPDATE: #22 created.

Lets discuss strategy for reducing complexity there.

urish added a commit that referenced this pull request Oct 26, 2015
@urish urish merged commit 87212e1 into urish:master Oct 26, 2015
@urish
Copy link
Owner

urish commented Oct 26, 2015

Sounds good, merged :)

@jamestalmage
Copy link
Contributor Author

Oops, I meant to address some of your line notes before the merge. Filed as separate PR's now.

@jamestalmage jamestalmage mentioned this pull request Oct 27, 2015
13 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support auth with custom tokens
2 participants