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

Use Cookies from CookieContainer when RestoreCookies is not set #200

Closed
SSchulze1989 opened this issue Apr 19, 2024 · 3 comments · Fixed by #201
Closed

Use Cookies from CookieContainer when RestoreCookies is not set #200

SSchulze1989 opened this issue Apr 19, 2024 · 3 comments · Fixed by #201
Assignees

Comments

@SSchulze1989
Copy link

SSchulze1989 commented Apr 19, 2024

Why is there a need to change?

Currently when the client is not logged in yet, the cookies from the container are only checked if also the RestoreCookies method is set in the Client options. This ignores the cookies that may already be stored and valid inside the registered CookieContainer

Why is this a problem?

If the client is injected in a scoped service, the service will always receive a new instance where LoggedIn is set to false. This causes the client to make an extra call to the auth page even if valid cookies are already present.

What should be changed?

The CookieContainer can be checked for valid cookies even if RestoreCookies method is not set. Then the existing logic can be applied to determine whether the client is already logged in or not.

This may need an additional check for the lifetime of the cookie to prevent sending an expired cookie in the requests.

@SSchulze1989
Copy link
Author

@AdrianJSClark if you want i can provide a pr for this feature.

@AdrianJSClark
Copy link
Owner

I've just published a new release which should resolve this issue, @SSchulze1989. Let me know if there are troubles.

@SSchulze1989
Copy link
Author

@AdrianJSClark working like a charm. The HttpClient automatically stores the Cookies in the container and the client stays logged in without the need to do anything else.

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.

2 participants