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

[Question] How to remove cookies? #14

Closed
Zerowalker opened this issue Apr 1, 2022 · 13 comments
Closed

[Question] How to remove cookies? #14

Zerowalker opened this issue Apr 1, 2022 · 13 comments

Comments

@Zerowalker
Copy link

I am having some problem figuring out how to remove cookies.

Cause you need to have the cookie in order to remove it, which to me is weird as i would expect to remove it by it's name.
But if you get the cookie beforehand you can't use that cookie to remove it cause of lifetime issues.
Are you supposed to just create a dummy cookie like in the Counter example?

    async fn handler(Extension(cookies): Extension<Cookies>) {
        let key = KEY.get().unwrap();
        let private_cookies = cookies.signed(key);
        cookies.remove(Cookie::new(COOKIE_NAME, ""));
    }
@imbolc
Copy link
Owner

imbolc commented Apr 2, 2022

Hey :)

Yep, a dummy cookie is ok. I see your point, but tower-cookies is just a tokio middleware around cookie crate, I guess they have their reasons for this kind of api. Here's the description of its remove method. Maybe make_removal is what you're looking for.

@Zerowalker
Copy link
Author

Zerowalker commented Apr 2, 2022

Hey :)

Thanks for the quick response.
It indeed looks like make_removal is what i am looking for.
Is it possible to expose it from tower-cookies?
same goes for SameSite, cause then one wouldn't need to have the cookie crate in parallel.

EDIT:

Thought make_removal was a separate function, but it was in the cookie itself, so it's already there, my bad.
But then i still use the cookie i grabbed which doesn't work cause of the lifetime right?

@imbolc
Copy link
Owner

imbolc commented Apr 2, 2022

But then i still use the cookie i grabbed which doesn't work cause of the lifetime right?

Right. I don't see an obvious way to fix this. And also I don't remember ever needing make_removal, it feels always easier to just "remove" potentially non-existing cookie (it won't even create unnecessary header if the cookie doesn't exist as there won't be any "delta").

As for removing cookies by name, it certainly feels more intuitive, thank you for the suggestion. Could you review the changes before I merge it: 56a1fc3

@imbolc
Copy link
Owner

imbolc commented Apr 2, 2022

Hmm... I don't think it's a correct approach though as cookie::CookieJar::remove says:

To properly generate the removal cookie, cookie must contain the same path and domain as the cookie that was initially set.

So I guess we should first check if the cookie is there and use its path and domain while removing it just by name.

@imbolc imbolc linked a pull request Apr 2, 2022 that will close this issue
@imbolc
Copy link
Owner

imbolc commented Apr 2, 2022

I think it's done: #15

@davidpdrsn @programatik29 would one of you guys have a few minutes to review it? It makes it possible to remove cookies by name, thakns 🙏

@Zerowalker
Copy link
Author

Oh you work quick.
Good catch with the Path part, otherwise it would be quite prone to "error".

Also, i remove some cookies in my error handler, but i don't know how to use the tower-cookies there as you just get access to the error and make a response.
Is there something i am might be missing you think, or might this just be a special case where i gotta use headers directly?

Thanks:)

@davidpdrsn
Copy link
Contributor

Also, i remove some cookies in my error handler, but i don't know how to use the tower-cookies there as you just get access to the error and make a response.

What specifically do you mean by "error handler"? Do you mean using HandleErrorLayer, because that also supports running extractors. async fn handle_error(cookies: tower_cookies::Cookies, err: axum::BoxError) should work.

@imbolc
Copy link
Owner

imbolc commented Apr 3, 2022

Or in case of a middleware, where you have only a request you can use Cookies::from_request:

async fn from_request(req: &mut RequestParts<B>) -> Result<Self, Self::Rejection> {

@Zerowalker
Copy link
Author

It's a impl IntoResponse for MyError
Perhaps no the correct way to do it?

@imbolc
Copy link
Owner

imbolc commented Apr 9, 2022

Jar is kept in request extensions, so no access to request - no cookies :)

@Zerowalker
Copy link
Author

make sense:)
So can i "solve" this by using a middleware or?
The purpose of the error handling is to be able to use Result.
But yeah this is clearly off topic so feel bad taking up even more of your time, so no need to really answer this.

@imbolc
Copy link
Owner

imbolc commented Apr 10, 2022

If you have request where you raise the error, you can process cookies there:

enum Error {
    LogOut
}

impl Error {
    log_out(req: RequestParts) -> Self {
        let cookies = Cookies::from_request(req).await?;
        ...
        Self::LogOut
    }
}

If you don't have the request, you can put something into the response extensions. And then in a middleware you can check the extensions and change cookies. Maybe there's a simpler way of doing this, you can ask in the axum discord: https://discord.com/channels/500028886025895936/870760546109116496

@imbolc
Copy link
Owner

imbolc commented Nov 25, 2022

So I guess we stick with the current behavior :)

@imbolc imbolc closed this as not planned Won't fix, can't repro, duplicate, stale Nov 25, 2022
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

Successfully merging a pull request may close this issue.

3 participants