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

Add quick connect (login without typing password) #1096

Merged
merged 20 commits into from
Sep 15, 2020

Conversation

ConfusedPolarBear
Copy link
Contributor

@ConfusedPolarBear ConfusedPolarBear commented Apr 15, 2020

Changes

This PR adds a new login method for devices that do not have a keyboard to enter a username/password on (smart TVs) or have limited keyboard input. It is a recreation of other quick sign in solutions found in other products (such as Plex, YouTube and Amazon Prime).

Proposed login flow:

  • The server administrator enables quick connect (only done once)
  • A user opens the Jellyfin app on a device they want to watch content on, selects the quick connect login option and is displayed a 6 digit code
  • The user uses a device which is already authenticated to the Jellyfin server (such as their phone or computer) and enters the code displayed on the other device
  • The server authorizes the connecting device to login as the user account that typed in the code

Proposed changes, feedback, and UX changes are welcome.
Related PR: Server side code

Planned features for this PR:

  • Proof of concept
  • Save quick connect state between server restarts
  • Get new icons for quick connect links

Issues

Closes feature request 541.

@heyhippari heyhippari added the feature New feature or request label Apr 25, 2020
@ConfusedPolarBear ConfusedPolarBear marked this pull request as ready for review April 25, 2020 20:49
@ConfusedPolarBear ConfusedPolarBear changed the title WIP - Add quick connect (login without typing password) Add quick connect (login without typing password) Apr 25, 2020
src/mypreferencesquickconnect.html Outdated Show resolved Hide resolved
src/quickconnect.html Outdated Show resolved Hide resolved
@YouKnowBlom
Copy link
Contributor

Aside from that this looks great! It'll be really convenient to have :)

@dkanada
Copy link
Member

dkanada commented Apr 28, 2020

We are migrating to ES6, so if you want to update your files it would be excellent but not required. We are also fixing the casing of folders and files to use camelCase so please rename them to match that standard.

@ConfusedPolarBear
Copy link
Contributor Author

I've renamed the files to use camelCase but I'm not fluent enough in JavaScript to migrate to ES6

@heyhippari
Copy link
Contributor

I'm not fluent enough in JavaScript to migrate to ES6

No worries, that's not blocking. We'll take care of it ;)

@heyhippari
Copy link
Contributor

Do note that due to #1007, you might have to submit your API client changes to https://github.com/jellyfin/jellyfin-apiclient-javascript instead of here, though.

@dkanada
Copy link
Member

dkanada commented May 5, 2020

Apologies for all the merge conflicts, we have been going through some drastic changes from stricter linting rules and more CI additions.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
5.9% 5.9% Duplication

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 1 Security Hotspot to review)
Code Smell A 143 Code Smells

No Coverage information No Coverage information
4.3% 4.3% Duplication

@thornbill thornbill requested a review from a team September 9, 2020 21:24
Copy link
Member

@thornbill thornbill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look great overall! Just a couple small pieces of feedback.

src/controllers/dashboard/quickconnect.js Outdated Show resolved Hide resolved
src/controllers/dashboard/quickconnect.js Outdated Show resolved Hide resolved
src/scripts/libraryMenu.js Outdated Show resolved Hide resolved
src/controllers/session/login/index.html Outdated Show resolved Hide resolved
src/quickconnect.html Outdated Show resolved Hide resolved
@dkanada dkanada merged commit 1dad3e7 into jellyfin:master Sep 15, 2020
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
2.3% 2.3% Duplication

@ConfusedPolarBear ConfusedPolarBear deleted the quickconnect branch September 15, 2020 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants