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

SSB auth cookies #145

Merged
merged 2 commits into from
Feb 11, 2020
Merged

SSB auth cookies #145

merged 2 commits into from
Feb 11, 2020

Conversation

marcotana
Copy link

Passed the auth cookies received from IdentityServer to/from browser and server to resolve browser refreshes that lose auth information.

@trafico-bot trafico-bot bot added the 🔍 Ready for Review Pull Request is not reviewed yet label Jan 30, 2020
@enkodellc
Copy link
Owner

@marcotana - First off thanks for the PR. I ALWAYS have a terrible time when debugging both CSB and SSB / switching. This time is no exception. Both are bombing. Please confirm they working for you. Test specifically the

  1. Login in then click refresh.
  2. Forum / Chat.

I am going to retest from my master to make sure everything is working from the update last night.

@Oleg26Dev can you please test this as a fix for #139 as well. Thanks

@rc-marcotana
Copy link

Just tested my fork and the SSB worked just fine. THE CSB also worked as you described above with the forum data showing when I switched and vice-versa. The only thing that didn't work for me is the Logout on CSB but worked on SSB. I will investigate that piece but I'm not sure if that's an issue before.

@enkodellc
Copy link
Owner

enkodellc commented Jan 31, 2020

I uploaded your changes to the demo site and it causes the same errors that I get on my local machine. Might not be from your changes but I cannot get it to work for the life of me. So frustrated with Blazor at the moment. I am walking away from it for today. I will try again on Saturday. Maybe some others can test. I will revert back to 0.6.1 on the demo site.

@enkodellc enkodellc merged commit d367cb7 into enkodellc:master Feb 11, 2020
@trafico-bot trafico-bot bot added ✨ Merged Pull Request has been merged successfully and removed 🔍 Ready for Review Pull Request is not reviewed yet labels Feb 11, 2020
@mobinseven
Copy link
Contributor

As I randomly got this exception:
System.InvalidOperationException: 'The given header was not found.'
in Logging out (Logout() in AthorizeApi.cs) operation, and I don't know why, I suggest this change:

            List<string> cookies = null;
            if (_httpClient.DefaultRequestHeaders.TryGetValues("Cookie", out IEnumerable<string> cookieEntries))
                cookies = cookieEntries.ToList();

enkodellc added a commit that referenced this pull request Feb 13, 2020
@enkodellc
Copy link
Owner

@mobinseven I just pushed your suggested changes. Thanks

@mobinseven
Copy link
Contributor

mobinseven commented Feb 14, 2020

Another issue in this commit which caused iOS Safari & Chrome problem:
I found that _http.HttpContext in App.razor returned null once while starting the app. So:

        if (_http != null && _http.HttpContext != null && _http.HttpContext.Request.Cookies.Any())
        {
           ...
        }

But this will ruin the cookie configuration and user will be logged out on refresh again...

enkodellc added a commit that referenced this pull request Feb 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Merged Pull Request has been merged successfully
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants