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

(exchanges) implement auth exchange #939

Merged
merged 20 commits into from
Sep 7, 2020
Merged

(exchanges) implement auth exchange #939

merged 20 commits into from
Sep 7, 2020

Conversation

kadikraman
Copy link

@kadikraman kadikraman commented Aug 14, 2020

Summary

Currently, every user will have to build their own auth exchange for authentication, and token refresh. The motivation for this exchange is to provide a general api that enables auth and token refresh for both React and React Native.

High level overview

The exchange will allow flexibility in:

  • being used in React (web) or React Native (primarily the implication is that, the initial auth state can be fetched asynchronously or synchronously)
  • what constitutes an auth error (e.g. a 401 or a specific error code)
  • how to add auth to a request (format of the header(s))
  • what auth state looks like (you can also store and use refresh token, token expiry or anything else)
  • how to refresh token (using a REST or graphQL endpoint)
  • how to react to errors (when to log the user out)

Once the exchange is created, we trigger getAuth for the first time. Here you should fetch your existing auth state from storage and return it. If the storage is asynchronous (in case of React Native), the exchange will pause the execution of requests until the auth is fetched and can be added to requests. Note that we don't call getAuth for every request, but store the authState in the exchange.

When the authState has been returned, addAuthToOperation will be called for each operation. You should use the auth state to add the proper auth to each request (e.g. the auth header).

Next, willAuthError is called. This is an opportunity to bail out early before doing a request. E.g if the JWT is expired we'll know there's no need to do the request, and it'll definitely fail.

Once the request has been done, if an error occurs, then didAuthError is called to check whether the error was an auth error. If this returns true, then the auth library will call getAuth - this is where you should use the existing auth state to create a new (hopefully valid) auth state, e.g. use the refresh token to refresh your JWT. The authExchange then calls addAuthToOperation with the new auth state and attempts to do the request again. If the second attempt also fails with an authentication error, we let the error fall through.

Finally, we use the errorExchange to handle critical auth failures and log out. We know that once the error reached the errorExchange, authentication refresh has already been attempted:

import { errorExchange } from 'urql';

errorExchange({
  onError: ({ error }) => {
    const isAuthError = error.graphQLErrors.some(
      e => e.extensions?.code === "FORBIDDEN",
    );

    if (isAuthError) {
      // log the user out
    }
  }
})

Note! The errorExchange must be placed before the authExchange in the exchanges array, because exchanges get executed in reverse order.

Configuration options

getAuth: ({ authState: T | null }) => Promise<T | null>;

Allows you to handle retrieving your auth state (usually a token and refresh token). Here is there the bulk of the auth is handled.

This function is called during initial load and whenever an auth error occurs. This way you can use is to fetch your token from storage on initial load,
or do a token refresh during auth failure.

addAuthToOperation: ({ authState: T | null; operation: Operation; }) => Operation;

Given the auth state you have defined, use that to add auth to each operation. Usuall this involves adding a JWT to the request header.

didAuthError: ({ error: CombinedError; authState: T | null; }) => boolean;

This gets called whenever a graphQL error occurs. Return true if the error should count as an auth error, false otherwise.

willAuthError: ({ operation: Operation; authState: T | null; }) => boolean;

This is checked before a network request is done and gives you the opportunity to bail out early.
For example if you already you have an expiry on your jwt, this could be stored in authState and checked in this function.
If the token is expired, we'll know the request will fail with an auth error without having to do the request.
Defaults to false

Set of changes

  • fetch token from storage
  • add token to operation
  • define unauthorised response
  • token refresh with REST
  • token refresh with graphQL
  • document input options

@changeset-bot
Copy link

changeset-bot bot commented Aug 14, 2020

💥 No Changeset

Latest commit: 6ee33b3

Merging this PR will not cause any packages to be released. If these changes should not cause updates to packages in this repo, this is fine 🙂

If these changes should be published to npm, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@kadikraman kadikraman changed the title [wip] Auth exchange (exchanges) [wip] implement auth exchange Aug 14, 2020
@kadikraman kadikraman marked this pull request as draft August 14, 2020 19:29
@kadikraman kadikraman changed the title (exchanges) [wip] implement auth exchange (exchanges) implement auth exchange Aug 14, 2020
@StevenLangbroek
Copy link
Contributor

Hey @kadikraman! Thanks for this 🥳 . We're working on authentication in our app and it seems to work. One small note; it doesn't seem to play well with URQL DevTools; it throws a promise which contains the introspection query operation. cc @kitten

@kitten
Copy link
Member

kitten commented Sep 7, 2020

I've just added the planned mutate method to the getAuth's function parameters which means that users can now send one-off mutations as part of their authentication flow, in case they for instance have a refreshToken mutation rather than REST endpoints. This mutation is not part of the usual exchange pipeline so that they don't get stuck on other preceding exchanges or never pass through due to the pendingPromise.

The last task is to check which events we'd like to add for the devtools.

@kadikraman kadikraman force-pushed the feature/auth-exchange branch from 67ecc1a to 8f51a67 Compare September 7, 2020 18:13
exchanges/auth/README.md Outdated Show resolved Hide resolved
exchanges/auth/README.md Outdated Show resolved Hide resolved
exchanges/auth/README.md Outdated Show resolved Hide resolved
exchanges/auth/README.md Outdated Show resolved Hide resolved
exchanges/auth/README.md Outdated Show resolved Hide resolved
exchanges/auth/README.md Outdated Show resolved Hide resolved
exchanges/auth/README.md Outdated Show resolved Hide resolved
@kitten kitten marked this pull request as ready for review September 7, 2020 19:02
@kadikraman kadikraman merged commit 4ea13f6 into main Sep 7, 2020
@kadikraman kadikraman deleted the feature/auth-exchange branch September 7, 2020 19:10
@kadikraman
Copy link
Author

Hey @StevenLangbroek - just an FYI that v0.1.0 of this exchange has just been released! 🚀

@MilosRasic
Copy link

Great feature. Thank you very much. I found a problem with our custom auth exchange and this came right in time to save us from grappling with wonka streams.

That said, there's a bit of inflexibility with authState. Nothing too serious, workarounds are possible, but they are not very elegant. In short, I'd like to be able to update or clear authState from outside of the functions provided to authExchange config. The two specific use cases I ran into are refreshing token on every app start and continuing app usage after logging out.

Token refresh on app start

This is done outside of urql with a RESTful API request. The problem is by the time the request is made urql will have called getAuth() and picked up the old tokens from the storage. When the RESTful request is resolved and tokens in the storage updated, urql will never call getAuth() again and keep using the old tokens.

The workaround is to have an initialTokenRefreshDone flag which starts with false and makes getAuth() refresh the token even when it's called with null authState. Not terrible but still not a great solution. initialTokenRefreshDone is a module global.

Clear token on logout

This should be a much more common use case. When the user logs out, we clear the tokens from the storage, but urql will blissfully keep adding the token it stored in authState to every subsequent request. This can be worked around by adding a logged out flag somewhere and not adding the token in addAuthToOperation() when the user is logged out or always returning true from willAuthError() so that getAuth() is called again. Neither are very nice since, again, a module global flag is needed with an exported setter function, or we can keep reading from storage in addAuthToOperation() or willAuthError() which is not a great solution either.

@kadikraman
Copy link
Author

Hi @MilosRasic - thanks for the feedback!

Token refresh on app start - could you do this?

const getAuth = async ({ authState }) => {
  if (!authState) {
    const initialAuthState = await doTokenRefreshOnAppStart();
    return initialAuthState;
  }
}

Clear token on logout - we've just published some documentation relating to the auth exchange today. After logout, it's best to reinitialise the client to ensure the cache is cleared. This will also reset any auth state - docs

@MilosRasic
Copy link

Thanks for the suggestions, @kadikraman .

For refresh on app start, I didn't find anything in the docs that implies that getAuth() will be called only once, so my implementation supports calling it again. At least I think it should 🙂

const getAuth = async ({authState}) => {
	// if called without the param, should fetch and return stored tokenss
	// but if initial refresh wasn't done, do it here
	if (!authState && initialTokenRefreshDone)
		return {
			token: localStorage.getItem('token'),
			refreshToken: localStorage.getItem('refreshToken'),
		};

	// if no refresh token in authState, try to get one from storage just in case
	const refreshToken = authState?.refreshToken || localStorage.getItem('refreshToken');

	if (!refreshToken)
		return {};

	initialTokenRefreshDone = true;

	const { err } = await doTokenRefresh();

	if (err)
		throw err;

	// now that tokens are certainly in local storage, recurse with null authState
	return getTokens({
		authState: null,
	});
};

For logout, makes sense, thanks. Much better than anything I could come up with and takes care of stale cache as well.

@kadikraman
Copy link
Author

getAuth is called once before the first query is run, and every time didRequestError or willRequestError return true. The structure of the authState is completely up to you. It will be null on the initial call and whatever you've returned from getAuth in subsequent calls.

Is you auth flow like so:

  1. fetch tokens from storage
  2. immediately do an API call to refresh the auth token
  3. save tokens to storage
  4. use tokens refetched in 2

If so, you could do something like this:

const getAuth = async ({ authState }) => {

  // authState is not defined, this means that it's the initial launch
  if (!authState) {
    const tokenFromStorage = localStorage.getItem('token');
    const refreshTokenFromStorage = localStorage.getItem('refreshToken');

    const { error, token, refreshToken } = await doTokenRefresh({
      token: tokenFromStorage,
      refreshToken: refreshTokenFromStorage
    });
    
    localStorage.setItem('token', token);
    localStorage.setItem('refreshToken', refreshToken);

    if (error) {
      throw error;
    }

    return { token, refreshToken };

  } else {
    // we will only get here if an auth error has occurred
    // you can either refresh the token or log the user out

    return {};

  }
};

@kitten
Copy link
Member

kitten commented Sep 11, 2020

@MilosRasic just as a side note ✌️ I hope we can resolve your questions, but please move them to a new topical GitHub issue or a Spectrum thread for discoverability and to keep the noise on this implementation-specific PR thread low. Cheers! 🙏

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

Successfully merging this pull request may close these issues.

4 participants