Skip to content

design: persistent oauth credentials #230

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

Closed
jeffbcross opened this issue Jun 16, 2016 · 9 comments
Closed

design: persistent oauth credentials #230

jeffbcross opened this issue Jun 16, 2016 · 9 comments

Comments

@jeffbcross
Copy link
Contributor

Problem

The new Firebase SDK only makes credentials, such as accessToken, from third-party oauth providers available after successful login (included in the object emitted from the signInWithPopup() promise, or from getRedirectResult() after a successful redirect login). This differs from the previous version of the SDK which would persist the credentials and always attach the credentials to the auth object returned from getAuth(). AFAIK this is intentional design to prevent accidental usage of expired tokens. In AngularFire2 as of beta.1, we cache the credentials in memory, but don't persist them to disk to survive a page refresh.

So previously, where applications could safely assume that if a user is authenticated, their credentials are available, applications must now check that a user is authenticated AND has credentials available before trying to access credentials. While on the surface this seems like a degraded user experience, it at least helps the application know sooner if credentials are no longer valid (i.e. the application doesn't have to make a request to third party API with the credentials and check the response error to find out the credentials have expired).

Proposal

We could provide an opt-in mechanism to automatically store credentials on disk, and automatically expire the credentials if the user is unauthenticated.

We'll create a token, CredentialStore, and by default bind it to an in-memory store that we are currently using.

Within the AngularFire2 npm package we'll provide a separate module, persistent-credential-store which will export a Provider that will override the CredentialStore with an implementation that will persist to disk. A user could just add this provider to their providers in order to automatically persist the credentials to disk (leaving it up to the application to determine when the credentials have expired).

import { bootstrap } from '@angular/core';
import { FIREBASE_PROVIDERS, defaultFirebase } from 'angularfire2';
import { PersistentCredentialStore } from 'angularfire2/persistent-credential-store';

import { MyApp } from './my-app';

bootstrap(MyApp, [
  FIREBASE_PROVIDERS,
  defaultFirebase({
    apiKey: "<your-key>",
    authDomain: "<your-project-authdomain>",
    databaseURL: "<your-database-URL>",
    storageBucket: "<your-storage-bucket>",
  }),
  PersistentCredentialStore
]);
@jeffbcross
Copy link
Contributor Author

CC @alfongj @davideast

@jeffbcross
Copy link
Contributor Author

Probably should shorten the module name to be less obnoxious, persistent-credential.

@alfongj
Copy link

alfongj commented Jun 17, 2016

nit: For naming, I would actually prefer "cached-credential" or similar, rather than "persistent" (which has, for me, more of a connotation that it could be expired).

I am not sure however that I do understand fully the proposal.

Are you proposing that, if the developer includes the "persistent-credential" module, then the auth listener will always return the cached credential (even after page refreshes), as opposed to the case where that module is not included, in which case the cached credential will be returned only if the page hasn't been reloaded?

@jeffbcross
Copy link
Contributor Author

nit: For naming, I would actually prefer "cached-credential" or similar, rather than "persistent" (which has, for me, more of a connotation that it could be expired).

Fair point.

Are you proposing that, if the developer includes the "persistent-credential" module, then the auth listener will always return the cached credential (even after page refreshes), as opposed to the case where that module is not included, in which case the cached credential will be returned only if the page hasn't been reloaded?

Yep. If a null user is ever emitted, then the cache would be cleared.

@alfongj is it possible for a user to be authenticated with multiple providers at the same time? Or would it be safe to say that if a user is authenticated with one provider, all other providers' credentials could be cleared from cache?

@ciekawy
Copy link

ciekawy commented Jun 18, 2016

I found this issue just now while trying to understand why on auth event (I now switched to FirebaseAuth.subscribe) there is no longer auth token available.

I wonder then about possibility to store the credentials in browser storage - which is what I'm doing now on the app side using Ionic.

@jeffbcross
Copy link
Contributor Author

I wonder then about possibility to store the credentials in browser storage - which is what I'm doing now on the app side using Ionic.

That's certainly a reasonable solution. Once this design is implemented, it'd be just as easy to create an Ionic implementation to persist credentials.

@alfongj
Copy link

alfongj commented Jun 20, 2016

@jeffbcross Good question. Indeed a given user account can have more than one credential linked, and the API could reflect that in this case. The only way this would work without special backend support is to cache credentials as they are linked to an account (i.e. on signIn you can cache the first credential, and then on link..() api calls you could also persist such credentials).

Having said that, this approach may have inconsistentcies between what the status of the user is on the backend, and what we can deliver the user in the client. An account can have more than one provider attached to it in the backend but on the client SDK we may only see the one with which they sign in.

@ciekawy
Copy link

ciekawy commented Jun 21, 2016

I'm also interested in handling multiple AuthProvider credentials for single user but think it is probably broader scope than just persisting the credentials. Probably there should be dedicated issue (here I've created one #257) on this topic as there seem to be a number of factors to be considered regarding linking the credentials.

Actually it would be nice to have some concept unrelated to the firebase or possibly there is already something to adopt?

@katowulf
Copy link
Contributor

I feel like this may be the wrong approach. Since credentials expire within hours now, they will generally be stale if stored, and this approach will likely encourage devs to incorrectly try and use those creds before authentication is initialized or as a static, one-time fetch (en lieu of calling onAuth() to monitor for auth state changes and handle these gracefully).

Handling auth at the routing level still seems like the most elegant solution.

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

No branches or pull requests

5 participants