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

Back Button Security Concerns #247

Closed
drfraker opened this issue Oct 6, 2020 · 20 comments
Closed

Back Button Security Concerns #247

drfraker opened this issue Oct 6, 2020 · 20 comments
Labels
core Related to the core Inertia library enhancement New feature or request investigate The issue needs further investigating

Comments

@drfraker
Copy link

drfraker commented Oct 6, 2020

I believe there may be some issues around the way Inertia handles it's ajax calls in the presence of Cache-Control headers that are put in place for security. See the link below for the closed issue where this is discussed.

The basic premise of cache control headers is that if you add 'no-cache, no-store' to cache control the app will require a full page reload even when a user clicks on the back button in the browser. This is a great way to prevent sensitive data from being displayed after a user logs out of an application and the back button is clicked. Since the app now has to make a full server request due to no cache history, the information will not be displayed and the user will be redirected back to the login page by the server.

When using Inertia this behavior is not present. If you log into an app with Inertia where the Cache-control headers are present to protect back button clicks, logout and press the back button you still see the sensitive pages.

Is there a way to check for this header and redirect to login if the session is no longer valid?

Originally posted by @drfraker in #102 (comment)

@drfraker
Copy link
Author

drfraker commented Oct 6, 2020

For reference: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control

Specifically this directive:
no-store
The response may not be stored in any cache. Although other directives may be set, this alone is the only directive you need in preventing cached responses on modern browsers. max-age=0 is already implied. Setting must-revalidate does not make sense because in order to go through revalidation you need the response to be stored in a cache, which no-store prevents.

@claudiodekker
Copy link
Member

claudiodekker commented Oct 6, 2020

Hi,

Unless I'm mistaken, this really is not a bug, and no-store is not consider to be a security feature. The HTTP 1.1 specification even describes that browsers should not be expected to respect http caching rules when clicking the back button:

User agents often have history mechanisms, such as “Back” buttons and history lists, which can be used to redisplay an entity retrieved earlier in a session.

History mechanisms and caches are different. In particular history mechanisms SHOULD NOT try to show a semantically transparent view of the current state of a resource. Rather, a history mechanism is meant to show exactly what the user saw at the time when the resource was retrieved.

By default, an expiration time does not apply to history mechanisms. If the entity is still in storage, a history mechanism SHOULD display it even if the entity has expired, unless the user has specifically configured the agent to refresh expired history documents.

This is not to be construed to prohibit the history mechanism from telling the user that a view might be stale.

However, this specification is outdated, and has been replaced by https://tools.ietf.org/html/rfc7234#section-6, which states:

User agents often have history mechanisms, such as "Back" buttons and history lists, that can be used to redisplay a representation retrieved earlier in a session.

The freshness model (Section 4.2) does not necessarily apply to history mechanisms. That is, a history mechanism can display a previous representation even if it has expired.

This does not prohibit the history mechanism from telling the user that a view might be stale or from honoring cache directives (e.g., Cache-Control: no-store).

While this does mention that the freshness model does not necessarily apply, it doesn't explicitly tell that the history mechanism should be expected to honor cache directives either, so, instead, we'll take a closer look at Section 4.2:

Note that freshness applies only to cache operation; it cannot be used to force a user agent to refresh its display or reload a resource. See Section 6 for an explanation of the difference between caches and history mechanisms.

Let's see if the no-store directive has anything more to offer us:

The "no-store" request directive indicates that a cache MUST NOT store any part of either this request or any response to it. This directive applies to both private and shared caches. "MUST NOT store" in this context means that the cache MUST NOT intentionally store the information in non-volatile storage, and MUST make a best-effort attempt to remove the information from volatile storage as promptly as possible after forwarding it.

This directive is NOT a reliable or sufficient mechanism for ensuring privacy. In particular, malicious or compromised caches might not recognize or obey this directive, and communications networks might be vulnerable to eavesdropping.

Note that if a request containing this directive is satisfied from a cache, the no-store request directive does not apply to the already stored response.

So, in conclusion, it's not really a bug, because history mechanisms are not http caches.

THAT SAID, since people keep complaining about it, and since we (Inertia) make HTTP requests and use History state as a cache of sorts, we might actually want to consider respecting the no-store directive. The only concerns that I have is that restoring a 'previous' state that is broken might cause more trouble than it's worth, because:

  1. How would we know what 'page' to display?
  2. If we display that page with no data, would it not cause javascript crashes due to missing values?

@reinink
Copy link
Member

reinink commented Oct 7, 2020

While I don't consider the current behaviour of Inertia.js to be "broken", I do understand why people want this behaviour.

We could update Inertia to only save the page object in the event that the Cache-Control header does not include no-cache or no-store, and then while navigating history, we could reload the data from the server. Then, in the event that the session expired, the user would be redirected to the login page. I haven't actually tested this, but I think in theory it should be possible. I do worry how well scroll restoration would work if you're waiting for a full page visit to occur when clicking back. I also worry what would happen if you click back/forward in the history quickly. Presumably we'd cancel the in-progress history restoration requests, but this could get ugly.

I wonder if there is a simpler way to handle this problem. Something like this:

// Track if the user is logged in (in localStorage) after every Inertia visit.
// This can be done using the Inertia "success" event, and by inspecting the page props.
Inertia.on('success', event => {
  window.localStorage.setItem('loggedIn', event.detail.page.props.auth.user !== null)
})

// When the user navigates, check if they are logged in.
// If they are not, stop the propagation to prevent Inertia from restoring the page.
// Then, instead, redirect them to the login page.
window.addEventListener('popstate', event => {
  if (window.localStorage.getItem('loggedIn') === 'false') {
    event.stopImmediatePropagation()
    window.location.href = '/login'
  }
})

The only hiccup with this approach is that this will also prevent navigating to other public pages. You could manually check for that though:

window.addEventListener('popstate', event => {
  // Only prevent history navigation for private pages.
  const publicUrls = ['/login', '/reset-password']
  if (!publicUrls.includes(event.state.url) && window.localStorage.getItem('loggedIn') === 'false') {
    event.stopImmediatePropagation()
    window.location.href = '/login'
  }
})

And of course, you could get fancy with this, and automatically mark pages as "private" server-side in your page props:

window.addEventListener('popstate', event => {
  // Only prevent history navigation for private pages.
  if (event.state.props.private && window.localStorage.getItem('loggedIn') === 'false') {
    event.stopImmediatePropagation()
    window.location.href = '/login'
  }
})

This actually achieves the desired behaviour very well. The biggest "issue" I see here is that for every click back in history (after logging out), this "eats" one of the history entries. This is because it's NOT possible to prevent (event.preventDefault()) a popstate event.

@drfraker
Copy link
Author

drfraker commented Oct 7, 2020

I really appreciate your time spent looking into this

You both have a much better idea that I do as to what would work best in the context of inertia. So I can't really add anything to that discussion.

I don't want to speak for anyone else, but I think anyone who is looking for this behavior would be ok with a few trade offs. If an app doesn't have "cache-content: no-store" headers, I imagine it would have no impact at all.

Let me know if I can help test or anything else.

@claudiodekker claudiodekker added core Related to the core Inertia library investigate The issue needs further investigating labels Oct 16, 2020
@claudiodekker claudiodekker added the enhancement New feature or request label Feb 17, 2021
@RedFoxxxxx
Copy link

RedFoxxxxx commented May 27, 2021

Hello everyone, I seem to also have a problem with this one. It goes back to dashboard even after log out. I tried the solution of @reinink but it didn't work for me. Any ideas on how to work around this?

@RedFoxxxxx
Copy link

I think I have found a work around. I just modified the code of @reinink to my own preference. Thanks.

@christoomey
Copy link

I've continued to explore this from the server side which would really be the ideal place to solve it, but unfortunately no combination of headers seems to get the browser (Chrome in my current testing) to behave as expected.

I decided to give it a quick poke based on the snippets @reinink shared above and was able to get something that roughly matches my goals, specifically:

  1. Allow for normal back-forward browser cache handling following GET requests
  2. Force full page reload when hitting back after any non-get request:
let lastRequestMethod = null

Inertia.on('start', (event) => {
  lastRequestMethod = event.detail.visit.method
})

window.addEventListener('popstate', (event) => {
  if (lastRequestMethod !== 'get') {
    event.stopImmediatePropagation()
    window.location.href = event.state.url
  }
})

It only adds this behavior for the single preceding request, but I believe it would degrade to the normal browser behavior, so I may be able to convince myself that this is enough and is a tolerable amount of mucking about in core browser logic.

@progdog-ru
Copy link

While I don't consider the current behaviour of Inertia.js to be "broken", I do understand why people want this behaviour.

We could update Inertia to only save the page object in the event that the Cache-Control header does not include no-cache or no-store, and then while navigating history, we could reload the data from the server. Then, in the event that the session expired, the user would be redirected to the login page. I haven't actually tested this, but I think in theory it should be possible. I do worry how well scroll restoration would work if you're waiting for a full page visit to occur when clicking back. I also worry what would happen if you click back/forward in the history quickly. Presumably we'd cancel the in-progress history restoration requests, but this could get ugly.

I wonder if there is a simpler way to handle this problem. Something like this:

// Track if the user is logged in (in localStorage) after every Inertia visit.
// This can be done using the Inertia "success" event, and by inspecting the page props.
Inertia.on('success', event => {
  window.localStorage.setItem('loggedIn', event.detail.page.props.auth.user !== null)
})

// When the user navigates, check if they are logged in.
// If they are not, stop the propagation to prevent Inertia from restoring the page.
// Then, instead, redirect them to the login page.
window.addEventListener('popstate', event => {
  if (window.localStorage.getItem('loggedIn') === 'false') {
    event.stopImmediatePropagation()
    window.location.href = '/login'
  }
})

The only hiccup with this approach is that this will also prevent navigating to other public pages. You could manually check for that though:

window.addEventListener('popstate', event => {
  // Only prevent history navigation for private pages.
  const publicUrls = ['/login', '/reset-password']
  if (!publicUrls.includes(event.state.url) && window.localStorage.getItem('loggedIn') === 'false') {
    event.stopImmediatePropagation()
    window.location.href = '/login'
  }
})

And of course, you could get fancy with this, and automatically mark pages as "private" server-side in your page props:

window.addEventListener('popstate', event => {
  // Only prevent history navigation for private pages.
  if (event.state.props.private && window.localStorage.getItem('loggedIn') === 'false') {
    event.stopImmediatePropagation()
    window.location.href = '/login'
  }
})

This actually achieves the desired behaviour very well. The biggest "issue" I see here is that for every click back in history (after logging out), this "eats" one of the history entries. This is because it's NOT possible to prevent (event.preventDefault()) a popstate event.

While I don't consider the current behaviour of Inertia.js to be "broken", I do understand why people want this behaviour.

We could update Inertia to only save the page object in the event that the Cache-Control header does not include no-cache or no-store, and then while navigating history, we could reload the data from the server. Then, in the event that the session expired, the user would be redirected to the login page. I haven't actually tested this, but I think in theory it should be possible. I do worry how well scroll restoration would work if you're waiting for a full page visit to occur when clicking back. I also worry what would happen if you click back/forward in the history quickly. Presumably we'd cancel the in-progress history restoration requests, but this could get ugly.

I wonder if there is a simpler way to handle this problem. Something like this:

// Track if the user is logged in (in localStorage) after every Inertia visit.
// This can be done using the Inertia "success" event, and by inspecting the page props.
Inertia.on('success', event => {
  window.localStorage.setItem('loggedIn', event.detail.page.props.auth.user !== null)
})

// When the user navigates, check if they are logged in.
// If they are not, stop the propagation to prevent Inertia from restoring the page.
// Then, instead, redirect them to the login page.
window.addEventListener('popstate', event => {
  if (window.localStorage.getItem('loggedIn') === 'false') {
    event.stopImmediatePropagation()
    window.location.href = '/login'
  }
})

The only hiccup with this approach is that this will also prevent navigating to other public pages. You could manually check for that though:

window.addEventListener('popstate', event => {
  // Only prevent history navigation for private pages.
  const publicUrls = ['/login', '/reset-password']
  if (!publicUrls.includes(event.state.url) && window.localStorage.getItem('loggedIn') === 'false') {
    event.stopImmediatePropagation()
    window.location.href = '/login'
  }
})

And of course, you could get fancy with this, and automatically mark pages as "private" server-side in your page props:

window.addEventListener('popstate', event => {
  // Only prevent history navigation for private pages.
  if (event.state.props.private && window.localStorage.getItem('loggedIn') === 'false') {
    event.stopImmediatePropagation()
    window.location.href = '/login'
  }
})

This actually achieves the desired behaviour very well. The biggest "issue" I see here is that for every click back in history (after logging out), this "eats" one of the history entries. This is because it's NOT possible to prevent (event.preventDefault()) a popstate event.

Please place this information in the documentation on your site.

@zarpio
Copy link

zarpio commented Feb 16, 2022

I think I have found a work around. I just modified the code of @reinink to my own preference. Thanks.

Can you please share your code what changes did you make, and where should I place the code in the layout file or app.js?

@atymic
Copy link

atymic commented Mar 1, 2022

I've run across this issue, and was also able to replicate it by logging out (post request) and then hitting back to be logged in again.

I actually have it in a different context, where we have a processing screen that shows, and customers hitting back get confused and think things are being double processed.

Is there any way to disable the caching completely, or on a per request basis?

@jeroen-fonky
Copy link

jeroen-fonky commented Jun 7, 2022

While I don't consider the current behaviour of Inertia.js to be "broken", I do understand why people want this behaviour.

We could update Inertia to only save the page object in the event that the Cache-Control header does not include no-cache or no-store, and then while navigating history, we could reload the data from the server. Then, in the event that the session expired, the user would be redirected to the login page. I haven't actually tested this, but I think in theory it should be possible. I do worry how well scroll restoration would work if you're waiting for a full page visit to occur when clicking back. I also worry what would happen if you click back/forward in the history quickly. Presumably we'd cancel the in-progress history restoration requests, but this could get ugly.

I wonder if there is a simpler way to handle this problem. Something like this:

// Track if the user is logged in (in localStorage) after every Inertia visit.
// This can be done using the Inertia "success" event, and by inspecting the page props.
Inertia.on('success', event => {
  window.localStorage.setItem('loggedIn', event.detail.page.props.auth.user !== null)
})

// When the user navigates, check if they are logged in.
// If they are not, stop the propagation to prevent Inertia from restoring the page.
// Then, instead, redirect them to the login page.
window.addEventListener('popstate', event => {
  if (window.localStorage.getItem('loggedIn') === 'false') {
    event.stopImmediatePropagation()
    window.location.href = '/login'
  }
})

The only hiccup with this approach is that this will also prevent navigating to other public pages. You could manually check for that though:

window.addEventListener('popstate', event => {
  // Only prevent history navigation for private pages.
  const publicUrls = ['/login', '/reset-password']
  if (!publicUrls.includes(event.state.url) && window.localStorage.getItem('loggedIn') === 'false') {
    event.stopImmediatePropagation()
    window.location.href = '/login'
  }
})

And of course, you could get fancy with this, and automatically mark pages as "private" server-side in your page props:

window.addEventListener('popstate', event => {
  // Only prevent history navigation for private pages.
  if (event.state.props.private && window.localStorage.getItem('loggedIn') === 'false') {
    event.stopImmediatePropagation()
    window.location.href = '/login'
  }
})

This actually achieves the desired behaviour very well. The biggest "issue" I see here is that for every click back in history (after logging out), this "eats" one of the history entries. This is because it's NOT possible to prevent (event.preventDefault()) a popstate event.

@reinink
Thanks for your explanation!

I've tried out this method and it kinda works for the first time i hit the back button. I get to see the page for like a second, and then go to the login page.
If i click more than once i still can see the data that i want to protect.

Anything more i can try?
Thanks in advance

@jameswong3388
Copy link

Any new approach till now?

@Erutan409
Copy link

Erutan409 commented Oct 8, 2022

@reinink That worked perfectly for me. I'm using Ziggy with Laravel and I was able to use this solution to verify where a user should be based on a globally accessible variable in Vue and by looking at named routes prefixed with a common name:

// safety for checking for valid login session
window.addEventListener('popstate', e => {

  const route = this.route()._unresolve(e.state.url);
  const authRoute = /^auth\./i;

  if (!route || !route.name) {
    return;
  }

  if (this.$app.loggedIn && authRoute.test(route.name)) {

    e.stopImmediatePropagation();
    this.$inertia.visit(this.route('home'));

  } else if (!this.$app.loggedIn && !authRoute.test(route.name)) {

    e.stopImmediatePropagation();
    this.$inertia.visit(this.route('auth.login'));

  }

});

@Abdulhakimsg
Copy link

Hi all

My team and I managed to solve it.For context our app only has one public route /login , the rest of the routes are protected routes. In our snippet we managed to achieve making sure no protected pages are loaded despite clicking on the back button numerous times.

//app.js

window.addEventListener('popstate', async (e) => {
    e.stopImmediatePropagation();
    try{
        // to check if application session is still valid
        const res = await fetch('/dashboard/state')
        if(res.ok){
            window.location.replace(e.state.url);
        }
    }catch(e){
        window.location.replace("/dashboard/user/login");
    }
},)

Hope the information above is helpful!

@rogervila
Copy link

Reloading the browser after clicking the back button ensures that pages are still accessible. This is my trick:

window.addEventListener('popstate', function (event: PopStateEvent): void {
    location.reload()
});

@reinink
Copy link
Member

reinink commented Jul 28, 2023

Hey folks, I don't anticipate us making any changes to how Inertia handles this, but I think my original workaround is still valid, so if this is something you're looking for I'd recommend trying that approach 👍

#247 (comment)

@reinink reinink closed this as completed Jul 28, 2023
@svengt
Copy link

svengt commented Mar 20, 2024

Hi @reinink, thank you for sharing your workaround. I appreciate the effort put into finding solutions. However, after examining it, I believe there are still significant flaws that need addressing.

While the suggested workaround attempts to mitigate the issue, it's important to highlight a potential vulnerability that remains. With JavaScript disabled after logging out, sensitive information can still be extracted from the browser's history. This poses a serious concern, especially in scenarios where users are accessing sensitive data from non-trusted computers.

Imagine a scenario where a user logs out from a shared or non-trusted computer assuming their data is safe. If a malicious actor exploits this flaw, they could easily turn off JavaScript and access all the props from the previous pages, compromising the user's privacy and security.

In my opinion, the suggested solution to check for no-store remains the most robust option. However, I've also experimented with an alternative approach using a custom implementation of router.ts. This implementation ensures that no props are saved, and upon handling a popstate event, all props are requested again. This approach adds an extra layer of security by preventing sensitive information from being retained in the browser's history.

    protected pushState(page: Page): void {
        this.page = page;
        const deepcopy = JSON.parse(JSON.stringify(page));
        deepcopy.props = {};
        window.history.pushState(deepcopy, "", page.url);
    }

    protected replaceState(page: Page): void {
        this.page = page;
        const deepcopy = JSON.parse(JSON.stringify(page));
        deepcopy.props = {};
        window.history.replaceState(deepcopy, "", page.url);
    }

    protected handlePopstateEvent(event: PopStateEvent): void {
        if (event.state !== null) {
            const page = event.state;
            const visitId = this.createVisitId();
            Axios({
                method: "get",
                url: hrefToUrl(page.url).href,
                headers: {
                    Accept: "text/html, application/xhtml+xml",
                    "X-Requested-With": "XMLHttpRequest",
                    "X-Inertia": true,
                    ...(page.version
                        ? { "X-Inertia-Version": page.version }
                        : {}),
                },
            })
                .then((response) => {
                    if (!this.isInertiaResponse(response)) {
                        return Promise.reject({ response });
                    }
                    const pageResponse: Page = response.data;

                    Promise.resolve(
                        this.resolveComponent(pageResponse.component),
                    ).then((component) => {
                        if (visitId === this.visitId) {
                            if (
                                hrefToUrl(pageResponse.url).href !==
                                window.location.href
                            ) {
                                window.location.href = pageResponse.url;
                                return;
                            }
                            this.swapComponent({
                                component,
                                page: pageResponse,
                                preserveState: false,
                            }).then(() => {
                                this.restoreScrollPositions();
                                fireNavigateEvent(pageResponse);
                            });
                        }
                    });
                })
                .catch(() => {
                    // redirect to the location
                    window.location.href = page.url;
                });
        } else {
            const url = hrefToUrl(this.page.url);
            url.hash = window.location.hash;
            this.replaceState({ ...this.page, url: url.href });
            this.resetScrollPositions();
        }
    }

I believe adopting a solution like this would better safeguard user data and address the concerns raised. What are your thoughts on this approach?

@driesvints
Copy link
Contributor

@prattcmp having such a passive/aggressive tone gets us nowhere. Instead of complaining why don't you help out by attempting a PR that could both solve the issue and still takes into account the concerns from the core team?

@reinink
Copy link
Member

reinink commented May 28, 2024

Hey folks, just left some comments about this particular concern in a related PR: #1784 (comment)

Hoping to find a way to address this 👍

@jeroen-fonky
Copy link

Thanks a lot @reinink!

gitbugr added a commit to gitbugr/inertia that referenced this issue Oct 5, 2024
gitbugr added a commit to gitbugr/inertia that referenced this issue Oct 5, 2024
gitbugr added a commit to gitbugr/inertia that referenced this issue Oct 5, 2024
gitbugr added a commit to gitbugr/inertia that referenced this issue Oct 5, 2024
gitbugr added a commit to gitbugr/inertia that referenced this issue Oct 5, 2024
gitbugr added a commit to gitbugr/inertia that referenced this issue Oct 5, 2024
gitbugr added a commit to gitbugr/inertia that referenced this issue Oct 5, 2024
gitbugr added a commit to gitbugr/inertia that referenced this issue Oct 5, 2024
gitbugr added a commit to gitbugr/inertia that referenced this issue Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to the core Inertia library enhancement New feature or request investigate The issue needs further investigating
Projects
Status: Closed 🚪
Development

No branches or pull requests