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

Issues with the refresh endpoint endlessly redirecting after signin #190

Open
HudsonAkridge opened this issue Feb 23, 2022 · 33 comments
Open

Comments

@HudsonAkridge
Copy link

We're using v2.1.0 (vanilla, nothing fancy/special). Occasionally, a user will get stuck in this redirect loop. I suspect it's when the JWT expires, and a lazy evaluation happens. Not all the time, but sometimes, it will put the browser into this constant back and forth redirect ping thing that keeps showing the message/image displayed below. If you look at the XHR request stuff, you'll see that it's just endlessly redirecting from the /refresh endpoint, to the cognito /auth and /login endpoints. I can post the XHR's (once I sanitize them), if needed.

This might be due to a refactor done recently for this version that has the JWT refreshed lazily perhaps?

I've tried with the JWT expiration set to 1 hour, 12 hours, 24 hours, and 5 minutes and the same thing. It just changes how fast the cycles go before this issue manifests/gets recreated.

For the end user, there's no solution other than clearing cookies to correct this problem. Once they sign in again, the problem goes away for several refreshes, some unknown number (can't recreate it consistently, it appears to be a timing thing) before it hits again.

I'm going to try reverting back to a previous version of the app (v2.0.19) to see if this has any positive impact.

image

@ottokruse
Copy link
Collaborator

Thanks for reporting this.

What are your values for parameters enableSPAmode and cookieSettings?

To debug, probably best to set loglevel to DEBUG for a bit, if that's acceptable to you (logs all requests, include sensitive data). That will show the responses generated by the Lambda@Edge functions––would be good to see if the checkAuth actually did set the nonce, and whether that nonce was sent by the browser to the refresh endpoint. You can also check that in the network panel.

I can post the XHR's (once I sanitize them), if needed.

That would be great.

@HudsonAkridge
Copy link
Author

EnableSPAmode = true
CookieSettings = { "idToken": null, "accessToken": null, "refreshToken": null, "nonce": null }

I do have some debug logs that should have been captured. Just for the /refreshauth lambda though. Let me see if I can get those (this has been a problem for the last week, we've been trying to figure out the cause).

Also, it'll take a few for me to get those XHR's, but will do. I'll post here with additional data.

So far, for what it's worth, I rolled back to v2.0.19 and haven't had this issue show up in the last 24 hours. Except the initial time when I rolled it back and changed the JWT/ID token expiration from 24 hours back to 60 minutes. But, since then, no issues reported.

@ottokruse
Copy link
Collaborator

Those settings are vanilla and should work.
You mentioned "Occasionally" which is interesting -- any clue as to the precise scenarios where this does or does not occur?

@HudsonAkridge
Copy link
Author

@ottokruse
Re: Scenarios. Unfortunately there's not an easy way to recreate it. What I do know are the following based on pulling a few different levers:

  1. It happens no matter what the JWT/ID expiration is set to in the Cognito client
  2. If the JWT/ID token expiration is set to longer (e.g. 12 hours/720 minutes), it takes much longer to recreate the issue, but it always seems to happen
  3. It appears to happen whenever the JWT/ID Token expires, when the user hasn't hit the site in a while. e.g. they were signed in yesterday morning, the token lasted 12 hours, and now they try and refresh the site the next morning, that seems to trigger it more often than other patterns.
  4. If they're using the site continually, it doesn't seem to happen, it appears to mostly be after letting it sit for a bit past the JWT/ID token expiration.

@HudsonAkridge
Copy link
Author

@ottokruse
This took a lot longer than I expected to sanitize the .har file 🙂 but finally got it done. I've redacted anything that could identify us, but I don't think anything I've removed would make it more difficult for you to see the issue.

LMK if I'm wrong about that and you need some bit of data that was redacted. If it's not an identifying piece of data for the user or our app, I'd be happy to add it back.

I had to change the type to .txt for it to upload to here so you'll have to change it back to .har 🙂
redirects-issue-redacted.txt

@HudsonAkridge
Copy link
Author

The largest issues with the redirect loop are:

  1. A normal user is unlikely to notice this and it'll just keep redirecting in the background forever. Hundreds/thousands of times until they stop the cycle
  2. The Sign-In button is broken and won't actually work to let them sign back in.
  3. Clearing cookies is the only way to solve this right now we've found, which is obviously not ideal.

We've got a handful of power-users that are going to test our app with v2.0.19 over the weekend. I'll let you know if we see the same problem there. But so far, no issues. Only with v2.1.0 (which could make sense, as a lot changed with the refreshauth logic in there it looks like)

@ottokruse
Copy link
Collaborator

ottokruse commented Feb 25, 2022

Thanks that was very helpful.

I can see the redirect loop, going back and forth to /refreshauth, until chrome throws net::ERR_TOO_MANY_REDIRECTS. That then leads to another trip to /refreshauth but this time without cookies. That would then show the error page you pasted the screen shot of above.

The root error is the redirect loop. Interestingly, from the HAR file the refresh seems to work okay:

  • User is redirected to /refreshauth (apparently because JWT expired)
  • User is redirected back to /, with new JWTs in the response.

All good, but then checkAuth decides again to send the user to /refreshauth.

Need to look closer into the reason for that. Best next step is to enable info logging, that will show the exact reason why checkAuth thinks the JWT is invalid in the logs:

CONFIG.logger.info("Access denied:", err);

Have a look at what the error says exactly?

You didn't add the JWTs in the HAR (makes sense) but it would be good to have a look at them and maybe run them through jwt.io and verify with your own eyes that the JWT's in the cookies in the response from /refreshauth are good valid and not expired.

@ottokruse
Copy link
Collaborator

The Sign-In button is broken and won't actually work to let them sign back in.

Can you share more details on this too? Maybe there is a clue here. Just tested again on 2.1.0 and for me this button works fine: it signs you out under the hood and redirects to the Cognito Hosted UI to sign in again.
Related: what setting do you have for parameter SignOutUrl (the default value is /signout)

@HudsonAkridge
Copy link
Author

HudsonAkridge commented Feb 27, 2022

The Sign-In button is broken and won't actually work to let them sign back in.

Can you share more details on this too? Maybe there is a clue here. Just tested again on 2.1.0 and for me this button works fine: it signs you out under the hood and redirects to the Cognito Hosted UI to sign in again. Related: what setting do you have for parameter SignOutUrl (the default value is /signout)

The value is set to /signout, as the default. And that's what the Cognito HostedUI has. It shows: https://env.my-site.com/signout as the URL for the sign in button in the cognito UI, and this doesn't work at that point. However, the signout button/URL we have in the app that takes you to the same URL does work if you're not currently dealing with the refresh auth redirect issue.

We are using ReactJS and Amplify here, and I have the amplify config set to use cookies. Which does work, it bypasses the amplify sign in page.

I could turn off amplify from the UI if needed for testing.

After redeploying v2.0.19, we're still seeing the same thing eventually. Just had a lot of users experience this issue over the weekend and report it, it just seems to happen less frequently.

The scenario that caused it was one of our users had their computer update and reboot overnight, and when they opened up the page this morning, it triggered a "sea of redirect problems".

My logging was reverted after the 2.0.19 deploy, so I'll update it to at least the INFO level and see if I can't get you exactly what the errors are saying.

I'll also decode the JWTs from that redacted .har I uploaded and see if the timings for the refresh auth endpoint are not being expired prematurely.

@ottokruse
Copy link
Collaborator

My logging was reverted after the 2.0.19 deploy, so I'll update it to at least the INFO level and see if I can't get you exactly what the errors are saying.

Cool.

What cookie settings do you have in your Amplify config? This is a thing that changed in v2.1.0, recommend now to have config like this (to use host only cookies):

cookieStorage: {
      domain: " ", // Use a single space " " for host-only cookies
      expires: null, // null means session cookies
      path: "/",
      secure: true, // for developing on localhost over http: set to false
      sameSite: "lax"
    },

Also might be wise to have lines like this in your web app, to let Amplify do the refresh:

// Schedule check and refresh (when needed) of JWT's every 5 min:
const i = setInterval(() => Auth.currentSession(), 5 * 60 * 1000);
return () => clearInterval(i);

@bedaka
Copy link

bedaka commented May 3, 2022

@HudsonAkridge could you finally solve this issue?
As it seems like we're experiencing the same problem I would appreciate any hints how to approach this.

@ottokruse
Copy link
Collaborator

Hi @HudsonAkridge ! Did you ever get to the bottom of this? Did the logs reveal anything usefull?

@ottokruse
Copy link
Collaborator

Closing for now, can re-open if further info surfaces

@HudsonAkridge
Copy link
Author

HudsonAkridge commented Feb 2, 2023

Hi @ottokruse , long time no chat 🙂

We had some other stuff come up and just got around to giving this another go, thinking maybe something updated in the last 6+ or so months to solve the problem.

Did another deploy with the latest (2.1.5) and ran into the same issue (see attached XHR/Network logs)
image

So, still not sure what's happening/going on here.

What I'm planning to do tomorrow is:

  1. Do a full on vanilla deploy of this stack, without any parameters passed in other than what's strictly necessary
  2. Let that create the Cognito pool and all other resources needed
  3. Set up a few default/basic users
  4. Use the default URIs and everything for doing the auth
  5. Drop in our ReactJS app into the site's S3 folder instead of using the demo app
  6. Try and resolve whatever CORS/config issues come up as a result of that
  7. Test again

The only thing that seems non-vanilla about our setup, is our pre-existing custom configured Cognito pool and how we've got some custom domains set up with Amplify.

So the goal is to remove as many variables from our setup as possible to create a control and see if we can recreate the issue then. It just doesn't make any sense that we're the only ones running into this with (potentially) thousands of consumers of this system.

Thoughts?

@ottokruse ottokruse reopened this Feb 3, 2023
@ottokruse
Copy link
Collaborator

Hi @HudsonAkridge :) Great to chat again though too bad the issue is still there then.

So the goal is to remove as many variables from our setup as possible to create a control and see if we can recreate the issue then.

Great plan. And do set logLevel to DEBUG then when you deploy the stack.

Also, share the Amplify config? Eg the stuff you put in Amplify.configure(...) for Auth? (redact sensitive parts)

@HudsonAkridge
Copy link
Author

Hi @ottokruse
Just as an update, I think we figured the issue out that was causing us so much pain and misery :)

I hope to have a write-up here soon to help the next person that comes along with a similar issue.

Right now, I have sort of a question from a little out in left field, which is:
I'm thinking of getting rid of the AmplifyJS library used in the ReactJS app, as this repo's functionality gets me an idToken to grab and attach to our auth headers as well as restricts access to the site.

The only issue I'm having now is that once the idToken expires, it takes loading a route in the page/app again to hit the /refreshauth route which will refresh the idToken.

Now, I could do that/negotiate that with Cognito directly here, like the /refreshauth lambda does to get an updated version of the JWT, but that feels dirty.

So I was trying to figure out how to get the cookie updated in the background while the user is making API calls to a different sub-domained endpoint (e.g. if the app is www.example.com all of the API routes are api.example.com, and was wondering if you had any thoughts/ideas there?

So far, I've got:

  1. Try loading some route in the ReactJS app with a call in the background about 30 seconds before the expires is set to fire off
  2. Negotiate the tokens myself

Option 2 is the less attractive route as I would like to re-use the behavior your great library here is set up to use. We just need to force some page in the www.example.com space to load before the token expires, but in the background.

At least, that's my current thought, but very much open to input/suggestions here.

Thank you for all of your work/help with this, and I'm super excited it looks like we might finally get to use this for real (once we solve this problem that is).

@HudsonAkridge
Copy link
Author

HudsonAkridge commented Feb 16, 2023

Here's what I've done for the moment to try and alleviate the issue. Added a check to do a background no-cache refresh of a simple empty JSON file with a cooldown of 15 seconds before I get the idToken value from Cognito. Example code:

import Cookies from "universal-cookie";

const cookies = new Cookies();
const getIdTokenCookieValue = (obj, filter) => {
    let key = [];
    for (key in obj){
        if (Object.prototype.hasOwnProperty.call(obj, key) && filter.test(key)) {
            return key;
      }
    }
    return null;
  };

const backgroundRefreshCooldownInMs = 15 * 1000; //15 seconds
let lastBackgroundRefreshTimestamp = null;

export const getUserToken = async () => {
    if(!lastBackgroundRefreshTimestamp || (Date.now() - lastBackgroundRefreshTimestamp) >= backgroundRefreshCooldownInMs)
    {
        //We want to load a potential background token refresh so the next token we get is updated
        lastBackgroundRefreshTimestamp = Date.now();
        let backgroundLoadResponse = await fetch("backgroundLoad.json", {cache: "no-store"});
    }

    let allCookies = cookies.getAll();
    let cognitoIdTokenCookieName = getIdTokenCookieValue(allCookies, /idToken/);
    let idToken = allCookies[cognitoIdTokenCookieName];

    return idToken;
};

I've got a pretty long expiration set for the idToken, so I'll report back in the morning if this works the way we want. If it does, people are welcome to use my code above for their needs if they're also getting rid of Amplify in their SPA.

My goal here is to get off Amplify completely for our ReactJS app. Amplify was nice as a boostrap in the beginning, but it's not particularly flexible and has some other weird oddities with it. It's simpler to just use this repo deployment and the above code to get the idToken and refresh in the background without needing the AmplifyJS dependency/complexity/UI.

@ottokruse
Copy link
Collaborator

ottokruse commented Feb 16, 2023

Just as an update, I think we figured the issue out that was causing us so much pain and misery :)

🎉

I hope to have a write-up here soon to help the next person that comes along with a similar issue.

Awesome. I'm pretty curious.

Try loading some route in the ReactJS app with a call in the background about 30 seconds before the expires is set to fire off

I like that idea.

If backgroundLoad.json is a 0-byte file, then it may be okay enough to be very dumb and simple about it and just add:

// Do a dummy fetch every minute, to refresh tokens (which is a no-op if they don't need refresh)
setInterval(() => fetch("backgroundLoad.json", {cache: "no-store"}), 1 * 60 * 1000)

UPDATE: this leaves a chance of the tokens becoming expired within the first 1 minute, for returning visits where the user already has cookies that might almost expire (within 1 minute).

Set the cache-control header on backgroundLoad.json on S3 to e.g. public,max-age=31536000,immutable so CloudFront caches that file nice and long. Then the only price you're paying for the call every minute, is Lambda@Edge running––if that's bearable depends on the amount of users on your site.

But your approach of only refreshing if needed, is certainly more sophisticated.

@ottokruse
Copy link
Collaborator

One more thought.

If I understand your situation it is roughly this:

  1. You host a website (SPA) using this Auth@Edge solution
  2. Your website's JavaScript calls APIs for which it uses the JWTs from the cookies
  3. The APIs are not fronted by the Auth@Edge CloudFront and to call them you need valid JWTs, therefore you're looking for a method to trigger their refresh

If that's indeed true, then a good way forward would be to change 3 above: front the APIs also with the Auth@Edge CloudFront. Then refreshes will be seamless our of the box, as you've seen Auth@Edge redirects automatically to the refresh endpoint and back to the requested location. So then you don't need to trigger token refresh manually in your SPA. Side benefit is that this also eliminates CORS preflight requests, so better latency.

@jimboboliver
Copy link

jimboboliver commented Feb 22, 2023

@HudsonAkridge I would love to hear about how you solved the problem because we've also been experiencing something similar to this for a long time. We have a similar setup to what you described with the pre-existing custom configured Cognito user pool being fed into the CloudFormation deployment.

@HudsonAkridge
Copy link
Author

@ottokruse @james1050
I've verified that this works with everything, and I was able to remove Amplify from the configuration and it's all good!

I'll try and briefly type up the approach with a more detailed explanation later when I have more time. For now, this is some o fthe things done to make this work starting from a fresh deploy.

  1. Set the ARN for cognito, add the ClientId of your app pool, and set the Auth URL for Cognito to whatever the auth deployment endpoint is, in this case something like auth.yourdomain.com works for me.
  2. In the HttpHeaders section, REMOVE the Content-Security-Policy header section of the JSON completely. Leave the others in place.
  3. For the cookieSettings, the cookies need to change to become:
  "cookieSettings": {
        "idToken": "Path=/; Secure; Domain=yourdomain.com; SameSite=Lax",
        "accessToken": "Path=/; Secure; Domain=yourdomain.com; SameSite=Lax",
        "refreshToken": "Path=/; Secure; Domain=yourdomain.com; SameSite=Lax",
        "nonce": "Path=/; Secure; HttpOnly; Domain=yourdomain.com; SameSite=Lax"
    }

This should be the setting for all configuration.json files after deploy. You might need to manually update the Lambdas that have this and redeploy (RefreshAuthHandler, ParseAuthHandler, SignOutHandler, HttpHeadersHandler, CheckAuthHandler) if it doesn't show as correct like the above.

  1. Leave defaults for all other options.

Now, in the frontend client code. Remove Amplify. Like, all of it. You shouldn't need it because this repo @ottokruse and co have built pretty much does it all for you.

Create or modify the method used to get the JWT for your application's headers (in my case, I need the JWT to attach to a call to api.yourdomain.com with Authorization: "Bearer <jwt>"). Here's the final code that's worked well for me in testing/trialing so far:

import Cookies from "universal-cookie";
import jwt_decode from 'jwt-decode';

const cookies = new Cookies();
const getIdTokenCookieValue = (obj, filter) => {
    let key = [];
    for (key in obj){
        if (Object.prototype.hasOwnProperty.call(obj, key) && filter.test(key)) {
            return key;
      }
    }
    return null;
  };

const rateLimitInMs = 15 * 1000; //15 seconds
const refreshTokenWindowInTicks = 15; //15 seconds
let lastBackgroundRefreshTimestamp = null;

export const getUserToken = async () => {
    let allCookies = cookies.getAll();
    
    //Calculate the idToken expiration. If this fails for some reason, we should be reloading/re-signing in to the app completely
    // so an error at this point should halt execution
    let initialIdTokenCookieName = getIdTokenCookieValue(allCookies, /idToken/);
    let initialIdToken = allCookies[initialIdTokenCookieName];
    let decodedToken = jwt_decode(initialIdToken);
    let initialIdTokenExpires = decodedToken.exp;
    let currentTicks = Math.floor(Date.now() / 1000); //Need to convert JSdate to Unix Ticks for comparisons
    let remainingIdTokenExpirationTicks = initialIdTokenExpires - currentTicks;
    let isWithinTokenExpirationWindow = remainingIdTokenExpirationTicks <= refreshTokenWindowInTicks;

    
    //Rate limit calls to doing a background fetch so we don't hammer this in case someone is impatient and keeps clicking
    let isNotRateLimited = !lastBackgroundRefreshTimestamp || (Date.now() - lastBackgroundRefreshTimestamp) >= rateLimitInMs;

    if(isNotRateLimited && isWithinTokenExpirationWindow)
    {
        lastBackgroundRefreshTimestamp = Date.now();
        let backgroundLoadResponse = await fetch("backgroundLoad.json", {cache: "no-store"});
        console.log("Refreshed page in background.");
    }

    //This may be a different idToken cookie value based on the Cognito Hash, we can't be sure it will match the one we retrieved above
    let currentIdTokenCookieName = getIdTokenCookieValue(allCookies, /idToken/);
    let idToken = allCookies[currentIdTokenCookieName];

    return idToken;
};

I'm not saying that the code above is the most optimized way to do this, or the cleanest, just that it's fairly simple and it works.

Also, add the backgroundLoad.json file to your site, and include a .bundleignore file at the root of your project (if you're using Yarn that is) and just put a single entry in it for backgroundLoad.json. This should prevent it from bundling that JSON file in with other javascript/json stuff so your file size is kept at small as possible. You're only going to be refreshing that when it's close to time for the token to expire, so it's not going to be creating much traffic unless your token is set to expire insanely fast for some reason.

What should happen is that if a token is expired, it should do a fetch of that json file behind the scenes which will load the JSON file, hitting the route, firing off the /refreshauth lambda. Once that's complete, it'll fetch the updated idToken value and use that JWT value until it's set to expire or close to an expiration.

This prevents hammering anything on the server and only executes it once, when needed.

Occasionally an individual request will fail on the background fetch, and you just have to click a link somewhere else in your app again (assuming it's always calling this getUserToken method for every authentication related call), I haven't been able to totally solve for that yet, but I haven't had a lot of time to dig in to looking at it.

So far, so good.

I hope this helps someone else 🙂

@ottokruse
Copy link
Collaborator

Cheers and thanks for the explanation, happy it's sorted for you!

@james1050 if this doesn't help you, I can look along with you. Send me a har file of the situation at ottokrus at amazon dot nl

@JaysenWankhade
Copy link

I am too facing the same issue, I tried with the latest version of the code as well as version 2.0.7. My configuration is as follows.
EnableSpaMode = false, Version = 2.07 as well as 2.0.9. We are hosting multiple static websites & serving it over the cloud front. When we Lauch multiple URL's in quick succession we get this error. Or sometimes even when we launch a standalone URL this happens. It says we can't refresh your sign-in automatically because of a technical problem also, redirected too many times, please try deleting your cookies. Please help!
Picture1

@ottokruse
Copy link
Collaborator

When we Lauch multiple URL's in quick succession we get this error. Or sometimes even when we launch a standalone URL this happens.

Hi mate. Can you be a bit more exact? Can you explain the steps to reproduce?

And what is the technical problem? If you click on it, what does it say? Is it the same message reported at the start of this issue?

@JaysenWankhade
Copy link

JaysenWankhade commented Mar 5, 2024

When we Lauch multiple URL's in quick succession we get this error. Or sometimes even when we launch a standalone URL this happens.

Hi mate. Can you be a bit more exact? Can you explain the steps to reproduce?

And what is the technical problem? If you click on it, what does it say? Is it the same message reported at the start of this issue?

Thanks for quick reply, well here is what is happening.

2 years back we have deployed an instance of this solution in Static Website mode.
In S3 we store multiple sites (Simple Static Html's).
Each website can be then accessed in below manner.

myexampledomain.com/site1/index.html
myexampledomain.com/site2/index.html
.
.
.
myexampledomain.com/siteN/index.html

Currently, we have deployed another instance of this solution with latest codebase in another AWS account.
Current configuration is like as follows:

Version: 2.1.9
HttpHeaders: {
"Content-Security-Policy": /our own applicable csp/
"Strict-Transport-Security": "max-age=31536000; includeSubdomains; preload",
"Referrer-Policy": "same-origin",
"X-XSS-Protection": "1; mode=block",
"X-Frame-Options": "SAMEORIGIN",
"X-Content-Type-Options": "nosniff",
"Cache-Control": "max-age=300, must-revalidate"
}
EnableSPAMode: false
CookieSettings: {
"idToken": null,
"accessToken": null,
"refreshToken": null,
"nonce": null
}
My App Client has ID token expiration of 5 minutes.

So, after launching the any of the URL mentioned (this happens randomly) after 5 mins of inactivity randomly one of the url fails to
get the idToken refreshed it says "myexampledomain.com redirected you to many times, try deleting cookies" & eventually land on the error page saying
"Your browser didn't send the nonce cookie along, but it is required for security (prevent CSRF). [log region: ap-southeast-1]"
Sometimes this even causes a problem in an active session as well. Not able to understand what's going wrong here.

Also, I have one more question, in past versions of this codebase, Check Auth function used to have this piece of codebase.
If the token has (nearly) expired and there is a refreshToken: refresh tokens
const { exp } = decodeToken(idToken);
if ((Date.now() / 1000) - 60 > exp && refreshToken) {
Which is not there in latest code base, so not sure how refreshing token is being handled in latest code.
Hoping for your reply :)

@ottokruse
Copy link
Collaborator

ottokruse commented Mar 6, 2024

Can you share a HAR file perhaps so I can see in detail what goes on? Send it to me directly via email.

Which is not there in latest code base, so not sure how refreshing token is being handled in latest code.

JWT refreshing is now lazy, it's only done when the JWT actually expires. (reason was that with a small expiry window for JWTs, that some people configure, you could run into infinite refreshes with the previous code, that pre-emptively refreshed)

Anyways this is proving to be tricky problem and the solution will be to monitor the browser network tab (and record a HAR please) and looking at the CloudWatch logs at the same time, so you can see why they do what they do (set logging to DEBUG for this).

@ottokruse
Copy link
Collaborator

ottokruse commented Mar 7, 2024

Looking closely at the network tab screenshot from @HudsonAkridge I can see that it is actually favicon.ico that triggers the cycle of redirects. That is of course silly (and maybe we should special case favicon.ico to not trigger refreshes to begin with).

If I look at the screenshot, the 1st request (is that in response to a redirect?) to refreshauth (first line in the screenshot) succeeds and comes back with HTTP 200. But, that means it is actually the error page, because if the JWTs would have been refreshed successfully, you'd see a 307 instead (easy to understand from the code here: https://github.com/aws-samples/cloudfront-authorization-at-edge/blob/master/src/lambda-edge/refresh-auth/index.ts).

What confuses me is that upon loading the error page, favicon.ico is apparently fetched, and it triggers the redirect to refreshauth also, which now works because I see a 307 (!)

But even though it works, it's cookies+JWTs are apparently not persisted, or something else is wrong, which is why the subsequent request to favicon.ico triggers the redirect to refreshauth again, which returns 307 (so tokens must again be refreshed) but still doesn't work, and so on until the browser has had enough of it.

This is one way to rack up the number of calls to Cognito! We should do something about it.

Questions I have, which can probably only be answered by going through the HAR file AND looking at the LambdaEdge logs side by side:

  • Why does the initial request to refreshauth return the error page, meaning the refresh didn't work? (Because all subsequent refreshauth requests succeed –– I see 307s there)
  • Why does the user navigate to refreshauth directly (1st request in the whole sequence)? I would never expect the first request to a website to go to refreshauth. You would only go to refreshauth because you get redirected there because of JWT expiry. (--> maybe this is the real culprit that we should fix)
  • Why are the cookies from all subsequent 307s not persisted? It's like cookies set in responses to favicon.ico requests are ignored by the browser? (maybe they have also special cased favicon.ico haha)

@ottokruse
Copy link
Collaborator

Users should never go directly to /refreshauth, they go to wherever they want to go, get redirected to /refreshauth where the JWT refresh magic happens, and then get redirected back to where they came from.

Users should never see the /refreshauth URL, if all goes well the redirect to there and back to where they came from is so fast they do not even notice it. But if the error page is displayed, then they would see it.

Maybe we should detect if the user goes to /refreshauth directly (without being redirected) and then redirect them back without trying the refresh. There is no foolproof way for this that I know of, but checking the absence of the Referer header could be pragmatic enough.

Also, I kinda like not triggering refresh for favicon.ico.

But let's not jump the gun and first get to the bottom of why this happens.

@JaysenWankhade
Copy link

JaysenWankhade commented Mar 7, 2024

Looking closely at the network tab screenshot from @HudsonAkridge I can see that it is actually favicon.ico that triggers the cycle of redirects. That is of course silly (and maybe we should special case favicon.ico to not trigger refreshes to begin with).

If I look at the screenshot, the 1st request (is that in response to a redirect?) to refreshauth (first line in the screenshot) succeeds and comes back with HTTP 200. But, that means it is actually the error page, because if the JWTs would have been refreshed successfully, you'd see a 307 instead (easy to understand from the code here: https://github.com/aws-samples/cloudfront-authorization-at-edge/blob/master/src/lambda-edge/refresh-auth/index.ts).

What confuses me is that upon loading the error page, favicon.ico is apparently fetched, and it triggers the redirect to refreshauth also, which now works because I see a 307 (!)

But even though it works, it's cookies+JWTs are apparently not persisted, or something else is wrong, which is why the subsequent request to favicon.ico triggers the redirect to refreshauth again, which returns 307 (so tokens must again be refreshed) but still doesn't work, and so on until the browser has had enough of it.

This is one way to rack up the number of calls to Cognito! We should do something about it.

Questions I have, which can probably only be answered by going through the HAR file AND looking at the LambdaEdge logs side by side:

  • Why does the initial request to refreshauth return the error page, meaning the refresh didn't work? (Because all subsequent refreshauth requests succeed –– I see 307s there)
  • Why does the user navigate to refreshauth directly (1st request in the whole sequence)? I would never expect the first request to a website to go to refreshauth. You would only go to refreshauth because you get redirected there because of JWT expiry. (--> maybe this is the real culprit that we should fix)
  • Why are the cookies from all subsequent 307s not persisted? It's like cookies set in responses to favicon.ico requests are ignored by the browser? (maybe they have also special cased favicon.ico haha)

Hi Mate,

I tried something, I took a pull of code where lazy refresh doesn't happen & just tried to update ChekAuthHandler with that piece of code & things seem to be working now. I suspect the issue has something to with the lazy refresh (although can't say for sure, but the behavior of things does align).

Why does the user navigate to refreshauth directly - I don't think user does that, In my case, after 5mins when the token expires & post that when I visit to the site, the issue occurs.

Hope this helps!

@ottokruse
Copy link
Collaborator

Yeah it is likely, the lazy refresh was introduced in v2.1.0, which is the 1st release in which this issue was reported.

But I still want to understand why this happens now with the lazy refresh before making any fixes.

@ottokruse
Copy link
Collaborator

Just tried to reproduce it myself (against a fresh and vanilla deployment out of the Serverless Repo) but the refresh works flawless for me:

image

Will be hard to fix this without a reliable way to reproduce it :|

@JaysenWankhade can you check if in your case also the 1st refresh goes wrong (shows error page) after which favicon.ico triggers the loop?

@ottokruse
Copy link
Collaborator

Yeah it is likely, the lazy refresh was introduced in v2.1.0, which is the 1st release in which this issue was reported.

Wait you mention also seeing this in 2.07 and 2.09, if so, it can't be the lazy refresh then

@ottokruse
Copy link
Collaborator

FYI tweaked the way refreshing works in #271 which is released as part of v2.3.0

There's a chance that fixes this issue as well.

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

5 participants