axios-auth-refresh icon indicating copy to clipboard operation
axios-auth-refresh copied to clipboard

refreshAuthLogic is not recalled on a second failure

Open antoniom opened this issue 4 years ago • 19 comments

Assume that you are making a request which returns a 401.

refreshAuthLogic will be called as it should and this second request is returning again a 401. This second failure is not triggering refreshAuthLogic again, is this intentional?

Is there any way to re-trigger the callback?

My scenario goes as follows: A web application is utilizing the UMA flow which uses RPT Tokens to access resources. RPT tokens are cached and they may expire. On expiration a 401 is issued and I retry using a normal access token which will respond (again) with a 401 and a permission ticket which in turn will be exchanged for a new RPT token. So the scenario is request -> 401 -> request again -> 401 -> request again -> possibly 200

antoniom avatar Oct 15 '20 11:10 antoniom

Sorry, I had no time to reply last few weeks. What version are you using, please? can you share your implementation? Thanks.

Flyrell avatar Oct 20 '20 06:10 Flyrell

hi, I have a similar problem. we use auth0 as authClient and fetch all req via axios from a REST api.

my issue goes: req1 with expired token -> BE res 401 -> axios-auth-refresh -> req1 with valid token -> BE res 200 req2 with expired token -> BE res 401 -> FE handles 401 error

both requests are called within a Promise.all

we use axios-auth-refresh ^2.2.3

aricma avatar Oct 22 '20 09:10 aricma

ok, I installed axios-auth-refresh 3.0.0 re-build everything and the issue is still there

aricma avatar Oct 22 '20 09:10 aricma

image

I also see only one request after refreshToken success I'm using axios-auth-refresh 3.0.0 and axios version 0.21.0

israelKusayev avatar Oct 31 '20 17:10 israelKusayev

I think it was because I set pauseInstanceWhileRefreshing to true but if I remove it, the second request is sent with the old token

image

israelKusayev avatar Oct 31 '20 17:10 israelKusayev

here is my implemention

axios.interceptors.request.use(config => {
  if (config.method === 'POST' || config.method === 'PATCH' || config.method === 'PUT')
    config.headers['Content-Type'] = 'application/json;charset=utf-8';

  const accessToken = AuthService.getAccessToken();
  if (accessToken && !config.headers.Authorization) config.headers.Authorization = 'Bearer ' + accessToken;

  return config;
});

// Function that will be called to refresh authorization
const refreshAuthLogic = (failedRequest: any) =>
  AuthService.refreshToken(AuthService.getRefreshToken()!)
    .then(data => {
      AuthService.storeTokens(data);
      failedRequest.response.config.headers['Authorization'] = 'Bearer ' + data.accessToken;
      return Promise.resolve();
    })
    .catch(() => {
      AuthService.removeTokens();
      history.push(routes.login);
    });

createAuthRefreshInterceptor(axios, refreshAuthLogic);



my refresh token is simple POST request with no special arguments

israelKusayev avatar Oct 31 '20 17:10 israelKusayev

@Flyrell

israelKusayev avatar Nov 02 '20 05:11 israelKusayev

I think it was because I set pauseInstanceWhileRefreshing to true but if I remove it, the second request is sent with the old token

same problem

vchar00 avatar Nov 08 '20 04:11 vchar00

Sorry guys, will look at it as I plan to release v3.1.0. Had a hard time at work at the end of last year and no time to do anything useful elsewhere. Are you 100% sure that this is not something which is caused by your token-binding logic?

For example (in the code above) it might not work because of the following if statement.

if (accessToken && !config.headers.Authorization) // Authorization header is there, but with old token

I would actually bind the token every time, it doesn't really cost that much and there's no need for that check. In case of multiple tokens and/or requests that are not triggered with token I'm using another instance fo axios, as using just one introduces more edge-cases in my opinion.

There are some tests for this logic explicitly, so it seems a bit odd to me, that there's would be an error like that and only a few people would notice it, while there's 21K downloads per week. Of course, if there's an error it needs to be fixed asap. I'm just making sure your implementations are working correctly. Thanks.

Flyrell avatar Jan 12 '21 08:01 Flyrell

I was having the exact same problem. After checking the source code and understanding how it works, I did a few tests. On the first test, I have a function with 3 calls to my API using an expired token and same Axios instance. This first test showed that the execution for these request happened sequentially. This first test did not produce any error.

const getInitialData = useCallback(async () => {
    
      await dispatch(getRequest("5dff213d-f7e3-4f34-9abe-0518b0e25786"));
      await dispatch(getRequest("71412802-6a6c-4696-831f-c902560204a7"));
      await dispatch(getRequest("18279f99-1f07-418f-9c6b-d1c304eb89b3"));
    
  }, [dispatch]);

  useEffect(() => {
    getInitialData();
    //getInitialData();
    //getInitialData();
  }, [getInitialData]);

Then, I did a second test where I called the function three times. The first call to the function was executed in parallel with the execution of the second call and the third call.

const getInitialData = useCallback(async () => {
    
      await dispatch(getRequest("5dff213d-f7e3-4f34-9abe-0518b0e25786"));
      await dispatch(getRequest("71412802-6a6c-4696-831f-c902560204a7"));
      await dispatch(getRequest("18279f99-1f07-418f-9c6b-d1c304eb89b3"));
    
  }, [dispatch]);

  useEffect(() => {
    getInitialData();
    getInitialData();
    getInitialData();
  }, [getInitialData]);

Something like this. image

This second test gave me the same errors like @israelKusayev (with pauseInstanceWhileRefreshing in true or false)

I am not an expert in JavaScript or Typescript. I think the responses are coming almost at the same time using the same Axios instance. Like using a different thread. The first response gets into the "refreshAuthLogic" and it creates the request interceptor that stalls any other request (when we have pauseInstanceWhileRefreshing in true). By the time when this happens, there are two more responses coming, and they will not execute the "refreshAuthLogic".

I solved this situation adding an extra interceptor to resend those requests that where not handle by the "refreshAuthLogic" and that are not stalled by the request interceptor, to add them to the stalled requests.

//this is to retry calls that are being executed using different threads
instance.interceptors.response.use(
  (response) => response,
  async (error) => {
    const request = error.config;

    if ((request as AxiosAuthRefreshRequestConfig) && (request as AxiosAuthRefreshRequestConfig).skipAuthRefresh)
      return error;
    
    if (error && error.response && error.response.status === 401 && error.response.headers["token-expired"]) {
      //let's retry
      return await instance(request);
    }

    throw error;
  }
);

Please let me know if you get same errors and if you have a better/cleaner solution.

CteixeiraPW avatar Apr 22 '21 00:04 CteixeiraPW

I had the same problem, and solution provided by @CteixeiraPW seems to work; In my code I made this change.

Before

 createAuthRefreshInterceptor(api.axiosInstance, refreshAuthLogic(api));

After

  createAuthRefreshInterceptor(api.axiosInstance, refreshAuthLogic(api), {
    pauseInstanceWhileRefreshing: true,
  });

  api.axiosInstance.interceptors.response.use(
    (response) => response,
    async (error) => {
      const request = error.config;
      if (request.skipAuthRefresh) return error;

      if (
        error &&
        error.response &&
        error.response.status === 401 &&
        error.response.headers['www-authenticate'] === 'Bearer'
      ) {
        // let's retry
        return api.axiosInstance(request);
      }

      throw error;
    }
  );

fullergalway avatar Apr 30 '21 11:04 fullergalway

Now when I look at it... I think the only problem is that you're not binding the token with an interceptor. The refreshLogic runs only once for the first request, so the other requests won't have their token refreshed when they're triggered again. That means the request interceptor for binding the token is pretty much a must have. Can you please confirm?

Part of the docs explaining the reasoning behind this: https://github.com/Flyrell/axios-auth-refresh#request-interceptor

Flyrell avatar Sep 16 '21 20:09 Flyrell

Hello @Flyrell,

I have in my app an interceptor to add the token.

const requestNewToken = async () => {
  const action = "/token/refresh";
  const data = JSON.stringify(getToken());

  try {
    const response = await instance.post(action, data, { skipAuthRefresh: true } as AxiosAuthRefreshRequestConfig);
    return response.data;
  } catch (error: any) {
    XXXX
  }
};

//  Function that will be called to refresh authorization
const refreshAuthLogic = async (failedRequest: any) => {
  //token-expired - inactivity - refresh
  if (failedRequest && failedRequest.response && failedRequest.response.headers["token-has-expired"]) {
    const token = await requestNewToken();

    if (token) {
      setToken(token);

      failedRequest.response.config.headers["Authorization"] = `Bearer ${token}`;
    } else {
      setUserAuth(false);
    }
  } else {
    setUserAuth(false);
  }
};

createAuthRefreshInterceptor(instance, refreshAuthLogic, {
  pauseInstanceWhileRefreshing: true,
});

//this is to retry calls that were not included into the retry stack of the createAuthRefreshInterceptor
instance.interceptors.response.use(
  (response) => response,
  async (error) => {

    const request = error.config;

    if ((request as AxiosAuthRefreshRequestConfig) && (request as AxiosAuthRefreshRequestConfig).skipAuthRefresh)
      throw error;

    if (error && error.response && error.response.status === 401 && error.response.headers["token-expired"]) {
      //let's retry
      return await instance(request);
    }

    throw error;
  }
);

// Use interceptor to inject the token to requests
instance.interceptors.request.use((request) => {
  if ((request as AxiosAuthRefreshRequestConfig) && (request as AxiosAuthRefreshRequestConfig).skipAuthRefresh)
    return request;
  request.headers.Authorization = `Bearer ${getToken()}`;
  return request;
});

If I remember well, I think in your code you are intercepting every response with a 401 and the first one triggers the refreshing of the token and create an interceptor to bind all the following requests, but the problem is that by the time that you create this "createRequestQueueInterceptor" more that one request have already been sent.

 // Create interceptor that will bind all the others requests until refreshAuthCall is resolved
        createRequestQueueInterceptor(instance, cache, options);

This means you may have two or more responses with 401, the first one gets a retry with the new token but not the second one.

The first response will add the instance to the "skipInstances" list

 if (options.pauseInstanceWhileRefreshing) {
            cache.skipInstances.push(instance);
        }

But the "shouldInterceptError" will reject the second response with 401 because the instance is already in the "skipInstances" list.

if (!shouldInterceptError(error, options, instance, cache)) {
            return Promise.reject(error);
        }

This is why I think this error is happening. Please, let me know if any of my assumptions is wrong. I am not an expert in JS or React.

CteixeiraPW avatar Sep 22 '21 20:09 CteixeiraPW

Hi there, Thanks @CteixeiraPW for your input, I have the exact same problem and I hope your solution might solve this. @Flyrell can't wait for your answer to Cteixeira on her last assumptions.

Cheers, Mav

ma-v avatar Nov 15 '21 15:11 ma-v

same problem.

  1. there is failed request.
  2. and another one
  3. auth request (refresh access token)
  4. repeated first one failed request, but not second one.

that's the problem is.

image

theartofdevel avatar Mar 27 '22 23:03 theartofdevel

Just bumping this. :)

DovahBrownies avatar Jul 20 '22 00:07 DovahBrownies

Can't get event when token request returns 401. $axios.onError doesn't work like all methods I've tried

import auth from "@/serviсes/auth";
import createAuthRefreshInterceptor from 'axios-auth-refresh';

export default function ({$axios, redirect, store, router, route}) {
	$axios.interceptors.request.use(
		async req => {
			let token = store.getters['auth/token']
			const refresh_token = store.getters['auth/refresh']
			if (token) {
				if (auth.isJwtValid(token)) {
					req.headers.common.Authorization = `Bearer ${store.getters['auth/token']}`
				}
			}
			return req
		}
	)

	const refreshAuthLogic = (failedRequest) => $axios.post('auth/refresh', {}, {
			headers: {
				'Authorization': `Bearer ${store.getters['auth/refresh']}`
			}
		}).then((tokenRefreshResponse) => {
			const {access_token, refresh_token} = tokenRefreshResponse.data
			auth.login({$store:store, $axios, $router:router, $route:route}, {token:access_token, refresh_token})
			failedRequest.response.config.headers['Authorization'] = `Bearer ${access_token}`;
			return Promise.resolve();
		}).catch(e => console.log(4444)); //doesn't work

	createAuthRefreshInterceptor($axios, refreshAuthLogic);

	$axios.onError(error => {
		console.log('onError') //doesn't work

		const code = parseInt(error.response && error.response.status)		
		if (code === 401) {
			auth.logout()
		}
	})
}

SkiperX avatar Jul 28 '22 15:07 SkiperX

This one has been around since 2020 :( @Flyrell do we have any plan to resolve this within the next release?

hgbao avatar Mar 08 '23 13:03 hgbao

This issue has existed for 4 years, the project I am working on is having the same problem and cannot solve it :)

vietvh-3042 avatar Mar 15 '24 02:03 vietvh-3042