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

Store authentication data in cookies #154

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dmitrylyzo
Copy link
Contributor

@dmitrylyzo dmitrylyzo commented Mar 5, 2021

Required by jellyfin/jellyfin-web#2481

Changes
UserId and AccessToken are stored in cookies if EnableAutoLogin is false.

At current moment, this PR breaks Android app here.
App retrieves credentials from localStorage, but UserId and AccessToken are now in the cookies, and it crashes.
If storing credentials in the cookies is good enough ☺️, we can (? @Maxr1998 ) add reading of cookies if UserId and AccessToken from localStorage are empty. In this case we will have backward compatibility.

Also, I'm not sure about Tizen 2.3/2.4: they use WebKit and I don't know if they support session cookies for file: protocol.

@sonarcloud
Copy link

sonarcloud bot commented Mar 5, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sonarcloud
Copy link

sonarcloud bot commented Apr 11, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Maxr1998
Copy link

Maxr1998 commented Sep 3, 2021

Adding a fallback to extract cookies would certainly be possible. I'm actually planning to soon go the other way around and inject the credentials into the webapp from the Android app, which cookies would make a lot easier too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants