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 HTTP 401 basic auth revalidation #2515

Closed
nitzanashi opened this issue May 18, 2020 · 11 comments
Closed

Issues with HTTP 401 basic auth revalidation #2515

nitzanashi opened this issue May 18, 2020 · 11 comments
Assignees
Labels
Bug An issue with our existing, production codebase. workbox-core workbox-navigation-preload

Comments

@nitzanashi
Copy link

nitzanashi commented May 18, 2020

Browser & Platform:
all browsers, mostly mobile

Issue or Feature Request Description:
After implementing offline page in our web-app we noticed that our non-production environment which are covered with basic auth are getting 401 unless visiting root, when disabling the service worker everything is working fine for us :/, this is not happening all the time but I would say 90% of time

here is the code:

import {registerRoute, NavigationRoute} from 'workbox-routing';
import {NetworkOnly} from 'workbox-strategies';
import {precacheAndRoute} from 'workbox-precaching';
import {skipWaiting, clientsClaim} from 'workbox-core';
import * as navigationPreload from 'workbox-navigation-preload';

const CACHE_NAME = 'offline-app';
const FALLBACK_HTML_URL = '/index.html';

skipWaiting();
clientsClaim();
navigationPreload.enable();

precacheAndRoute(self.__WB_MANIFEST, {
    ignoreURLParametersMatching: [
        /token-[^-]*\.json/,
        /\.(?:map)$/,
    ],
});

self.addEventListener('install', async (event) => {
    await /** @type {ExtendableEvent} */(event).waitUntil(
        caches.open(CACHE_NAME)
            .then((cache) => cache.add(
                new Request(FALLBACK_HTML_URL, {credentials: 'same-origin'}),
            )),
    );
});

const navigationHandler = async (params) => {
    try {
        return await new NetworkOnly({
            fetchOptions: {
                credentials: 'same-origin',
            },
        }).handle(params);
    } catch {
        return caches.match(FALLBACK_HTML_URL, {
            cacheName: CACHE_NAME,
        });
    }
};

registerRoute(
    new NavigationRoute(navigationHandler, {
        denylist: [
            /token-[^-]*\.json/,
            /\.(?:map)$/,
        ],
    }),
);

and here is a video

@jeffposnick
Copy link
Contributor

A few questions/observations:

  • I'm assuming your / is included in self.__WB_MANIFEST, and that's why you're seeing different behavior for navigations to the top-level / vs. any other page. In your setup, precaching's routes will take precedence over the navigation route.

  • Can change your navigationHandler to include catch (error) and then log error somewhere? I'm assuming that it might have something meaningful in it. It would also make sense to try breaking up your NetworkOnly construction and handling. Something like:

const networkOnly = new NetworkOnly({
   fetchOptions: {
     credentials: 'same-origin',
   },
});
const navigationHandler = async (params) => {
    try {
        return await networkOnly.handle(params);
    } catch (error) {
        console.log('Error in navigationHandler:', error);
        return caches.match(FALLBACK_HTML_URL, {
            cacheName: CACHE_NAME,
        });
    }
};
  • I initially thought that in your install handler, the call to cache FALLBACK_HTML_URL ends up caching a 401 Unauthorized response, but that would actually cause the cache.add() to reject, which would in turn cause installation to fail. That being said, what happens if you take a look at the contents of the FALLBACK_HTML_URL entry in either Chrome's DevTools cache viewer, or by running in the console:
r = await caches.match(FALLBACK_HTML_URL, {cacheName: CACHE_NAME});
t = await r.text();
console.log(t);

@nitzanashi
Copy link
Author

Thanks for responding Jeff,

  • Yes we're caching / under self. __WB_MANIFEST, your explanation to that point makes sense, thanks for that.
  • I included the error but as I expected it's catching only the error for Offline network.
  • t will return the cached file data as raw text as expected.

Before introducing offline-app we used the GenerateSW and we had no problem with that.
Also I'm just wondering if there is something off in my settings here :|

@jeffposnick
Copy link
Contributor

Any chance you could host this setup on a publicly accessible test server somewhere? It's hard to know how to debug further without trying it out myself at this point.

@nitzanashi
Copy link
Author

I'm working on supplying a public test.

For now, I noticed that in the cases which are failing the Authorization field is not being passed on requests, is that maybe helping by figuring out the issue?

@nitzanashi
Copy link
Author

@jeffposnick I got a public test but for that I will have to send you the credentials privately, how can I contact you?

@jeffposnick
Copy link
Contributor

You can DM me on Twitter at @jeffposnick.

@jeffposnick jeffposnick changed the title 401 Authorization Required because of SW HTTP 401, basic auth, & navigation preload Jun 3, 2020
@jeffposnick jeffposnick self-assigned this Jun 3, 2020
@jeffposnick jeffposnick added Bug An issue with our existing, production codebase. workbox-core workbox-navigation-preload labels Jun 3, 2020
@jeffposnick
Copy link
Contributor

After debugging this a bit more with the live reproduction, I think we can narrow things down to a combination of using workbox-navigation-preload with HTTP basic auth. Once the existing auth token expires, a preloaded HTTP 401 response is being used to satisfy the navigation request, and you're never presented with another basic auth login screen. (This seems to happen more often on Android for some reason.)

I'm going to think a bit about the next steps here—it might be that we need to check the navigation preload response's HTTP status and not use it when it's not 200. I'm currently not clear on the heuristic used to determine whether Chrome will display the basic auth login screen or not.

Disabling navigation preload for your basic auth-protected staging environment may be sufficient as a workaround in the meantime.

@jeffposnick
Copy link
Contributor

So actually, I think the issue is more about https://chromestatus.com/feature/5682567464353792 not working as expected, as I'm able to reproduce this issue by following thee steps in https://glitch.com/edit/#!/sw-basic-auth?path=README.md%3A9%3A0 without any navigation preload.

I'm going to follow up with some folks from the Chrome engineering team to figure out what's up, and see if it's a bug with Chrome or something else.

In the short-term, though, I wouldn't rely on using HTTP basic auth that expires after a period of time if you're using a service worker.

@nitzanashi
Copy link
Author

I did try to disable navigation preload but the issue still occurred, so it fits to what you described.

@jeffposnick
Copy link
Contributor

@jeffposnick jeffposnick changed the title HTTP 401, basic auth, & navigation preload Issues with HTTP 401 basic auth revalidation Jun 4, 2020
@jeffposnick
Copy link
Contributor

I'm going to close this issue as it don't appear to be something Workbox has any control over.

https://bugs.chromium.org/p/chromium/issues/detail?id=1055253 tracks the issue with Chrome, which ostensibly should show your a Basic Auth dialog box when this happens.

Firefox and Safari haven't implemented that behavior at all, though, so in those browsers you won't see a Basic Auth dialog box either.

I'd basically (hah!) recommend that you don't use Basic Auth to protect your HTML when you're also relying on a service worker to handle navigation requests. If you do need to, then wait until https://bugs.chromium.org/p/chromium/issues/detail?id=1055253 is resolved and know that it won't work in Firefox and Safari.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug An issue with our existing, production codebase. workbox-core workbox-navigation-preload
Projects
None yet
Development

No branches or pull requests

2 participants