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

refreshAuthLogic is not recalled on a second failure #123

Open
antoniom opened this issue Oct 15, 2020 · 19 comments
Open

refreshAuthLogic is not recalled on a second failure #123

antoniom opened this issue Oct 15, 2020 · 19 comments

Comments

@antoniom
Copy link

antoniom commented Oct 15, 2020

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

@Flyrell
Copy link
Owner

Flyrell commented Oct 20, 2020

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

@aricma
Copy link

aricma commented Oct 22, 2020

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
Copy link

aricma commented Oct 22, 2020

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

@israelKusayev
Copy link

israelKusayev commented Oct 31, 2020

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
Copy link

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
Copy link

israelKusayev commented Oct 31, 2020

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
Copy link

@Flyrell

@vchar00
Copy link

vchar00 commented Nov 8, 2020

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

@Flyrell
Copy link
Owner

Flyrell commented Jan 12, 2021

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.

@CteixeiraPW
Copy link

CteixeiraPW commented Apr 22, 2021

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.

@fullergalway
Copy link

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;
    }
  );

@Flyrell
Copy link
Owner

Flyrell commented Sep 16, 2021

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

@CteixeiraPW
Copy link

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.

@ma-v
Copy link

ma-v commented Nov 15, 2021

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

@theartofdevel
Copy link

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

@DovahBrownies
Copy link

Just bumping this. :)

@SkiperX
Copy link

SkiperX commented Jul 28, 2022

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()
		}
	})
}

@hgbao
Copy link

hgbao commented Mar 8, 2023

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

@vietvh-3042
Copy link

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

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

No branches or pull requests