Fix login bugs, enable support for saving user's credentials, and add "Change User" and "Remember Me?" options

[cewert]

This fixes all of the bugs associated with navigating the select a server, select a user, and log in screens. Adds support for saving user credentials in the registry. Adds new option to home screen called "Change User". Adds new global user setting called "Remember Me?".

There were two ways our app was trying to save credentials: using token, username, and password registry entries and also by saving credentials to the "saved_servers" registry entry. I chose to implement this using the token, username, and password entries. To make this work, I am saving the entries to the user specific block of the registry with the other user settings instead of saving them to the main "Jellyfin" registry block like they were before.

EDIT: No longer saving password to registry. Auth tokens don't expire. Had to modify the deviceid in the auth header to make this work

Changes

• Remove the active server from the registry and wipe the server session when the user presses the back button on the user select screen.
• Fix back button not returning to server select when viewing the Login screen
• Fix bug where selecting a public user logged you in as a different user
• Save auth token and username permanently to registry when asked
• Attempt to use saved auth token
  • When starting the app with an "active_user"
  • When selecting a public user
• Add option to "Change User" on home screen
  • Don't erase credentials in registry if present
  • Show public users for currently selected server or login screen if there are no public users
• Add global "Remember Me?" option
  • Under a new root setting category named "Global"
  • Choose if the app should save the currently logged in user and attempt to log them in again the next time the app starts.
  • Defaults to false for security
• Create a registry migration for the next release (v.1.7.0)
  • If last run version is less than 1.7.0, delete deprecated settings and raw credentials from the registry

Fixes #310
Fixes #1372
Fixes #309

[dathbe]

There were two ways our app was trying to save credentials: using token, username, and password registry entries and also by saving credentials to the "saved_servers" registry entry. I chose to implement this using the token, username, and password entries. To make this work, I am saving the entries to the user specific block of the registry with the other user settings instead of saving them to the main "Jellyfin" registry block like they were before.

EDIT: No longer saving password to registry. Auth tokens don't expire. Had to modify the deviceid in the auth header to make this work

Some of this information went over my head, so maybe the explanation is in here, but when I try this out, it does not save my credentials at all. Each time I load the Dev app, I have to re-enter my credentials, even after checking the "Save Credentials?" box.

Jellyfin Roku v1.6 build 6 (51c629c unstable)
Jellyfin 10.8.10

[cewert]

@dathbe are you signing out before you exit the app? Did you enable the "Remember Me?" global user setting?

[dathbe]

Ah, that was it. The "Remember Me?" setting was disabled. Why is this default disabled!?

Answer to my question:
"Defaults to false for security"

[dathbe]

Now that I've been able to play with this for a bit, I can comment on the changes.

This update says it takes care of Issue 310. But I'm not seeing that it does. The new "Change user" option is an improvement, but what I'm seeing is that by clicking on that, it logs the current user out immediately (i.e., even if you just accidentally click on it and do not attempt to log a new user in). Thus, it does not seem possible to have two users logged in at the same time and be able to switch between them without re-entering the username and password.

As I said above, some of the description of how the changes affect credentials is beyond my knowledge level. But if I'm reading it correctly, I think what I just described in the previous paragraph is how this update is intended to work. If I'm reading the change log correctly, it seems like the change users function is supposed to give you a login screen or a list of public users. So if the user is not public, then it does not seem to retain the information as you switch back and forth.

But, as I understand issue 310, the goal is to have the option to select various user accounts after logging in the first time without having to log in a subsequent time. This would be consistent with the way most streaming services allow you to choose a profile when you open the app. (Some services require a PIN for 'adult' accounts; and some log into the last used user while others give you a prompt to select a user each time the app is opened. But all give the ability to select a profile without having to add full credentials again.)

Granted, Jellyfin works a bit differently in that each account has a single user. So we're talking about selecting between different accounts instead of different users/profiles within one account. But I would hope that storing credentials for multiple accounts would not be impossible.

Anyway, I guess my request is to un-close issue 310 because I don't think it's fully implemented.

...or, maybe I'm still using it wrong. But at least this time I went through all the settings options to make sure I wasn't missing something obvious.

FWIW, I do think this update takes care of the other two issues (309 and 1372). And I'm VERY glad the issue 309 was addressed with a settings option instead of forcing a logout for everyone...even if I'm less than thrilled with the default for that option ;-).

[cewert]

The new "Change user" option is an improvement, but what I'm seeing is that by clicking on that, it logs the current user out immediately (i.e., even if you just accidentally click on it and do not attempt to log a new user in). Thus, it does not seem possible to have two users logged in at the same time and be able to switch between them without re-entering the username and password.

As I said above, some of the description of how the changes affect credentials is beyond my knowledge level. But if I'm reading it correctly, I think what I just described in the previous paragraph is how this update is intended to work. If I'm reading the change log correctly, it seems like the change users function is supposed to give you a login screen or a list of public users. So if the user is not public, then it does not seem to retain the information as you switch back and forth.

It sounds like you are testing this with users that aren't public, is that right? Yes, you are right when you say in the second paragraph that this really only works for public users.

I suppose it's possible to make it work the way you are describing but that would be a security risk for the non-public user(s). If you want to save that users credentials why not just make him public? The whole point of having the user being private is to increase security - someone would have to know your password and username instead of just your password. This is mainly meant for the admin account(s) from my understanding. Then your normal non-admin users are all public.

So it doesn't make much sense to me to save a list of logged in private users in the app and let the user easily switch back and forth between them with one click and no password. That kind of defeats the purpose of them being private in the first place, yea?

We're not there yet, but imagine years down the road our app could support "admin" specific functions like scanning the library, rebooting the server, checking logs etc Some of those things show sensitive info or could affect other users etc. That's why I believe it makes sense to treat private users differently than public users.

PS: If your users are public then we are in major bug territory but I tested the crap out of this and I assume neil did too. That's why I'm assuming your users are private

[dathbe]

If you want to save that users credentials why not just make him public? The whole point of having the user being private is to increase security - someone would have to know your password and username instead of just your password. This is mainly meant for the admin account(s) from my understanding. Then your normal non-admin users are all public.

So it doesn't make much sense to me to save a list of logged in private users in the app and let the user easily switch back and forth between them with one click and no password. That kind of defeats the purpose of them being private in the first place, yea?

Yes, I think my users are all private. There's a big difference between "I want to keep users logged in and easily switchable on my own private TV in my own house" and "I want the entire world to be able to see my usernames any time they happen to stumble across my installation". If I go to netflix.com, it does not give me a list of every username on their system. Yet when I log into my tv and go to my netflix app, it knows who I am and allows me to switch profiles seamlessly. I suppose the risk is somewhat low even if I make my users public, but it's a lot easier to guess a password than it is to guess a username AND password. And presumably most usernames give away some private information about the user.

There's also the opposite problem that I don't necessarily need all my users listed on every app. I don't need my whole family listed on my iPad, but I might on my TV. Thus, I would handle it based on logins. If I log in with a user on a device, and I select the option to save the credentials on the login screen, and I have set the setting to remember me in the global options, I think I've made it clear enough that this is a safe enough device to remember me for the long haul.

Edit: I created some dummy profiles and made them public, and I can confirm this update does EXACTLY what I want with the public profiles. I just wish it would do that with private profiles too so I don't have to expose any more info to the interwebs.

Edit 2: I say "exactly," but that's not entirely true. Right now the switching profiles is locked behind the settings page using the "*" button. Most non-technical users don't ever push that button. I would suggest allowing a different key press pointing to something visual to allow user selection. To me, the most sensible is to allow the user to press the up button while on the home screen to select the username at the top right of the screen. Then pressing "ok" while the username is highlighted would take the user to the change user screen. That would be more intuitive, I think. It would be fine to have it in both places. But right now going up from the top menu item isn't being used, and I think this would be a good use.

[cewert]

Don't let perfect be the enemy of good.

I'm just a volunteer trying to improve the app in my free time and I believe this PR is a huge improvement. You are right when you say netflix has some things figured out that our app doesn't but they also have programmers on payrolls etc. So I don't think it's fair to expect the open source app coded by volunteers in a language no one else uses to match features with the biggest streaming platform that has ever existed.

If this PR doesn't meet your needs feel free to open an issue or a PR with the changes you'd like to see.

[dathbe]

I believe this PR is a huge improvement.

I agree. Please don't take my comments as criticism. I know and appreciate the work that everyone puts in to make this a wonderful app. I'm simply presenting a different use case and giving feedback...as I thought the beta channel was for.

[cewert]

I agree. Please don't take my comments as criticism. I know and appreciate the work that everyone puts in to make this a wonderful app. I'm simply presenting a different use case and giving feedback...as I thought the beta channel was for.

Like we mention on our readme, we want your feedback about when things break or to tell us about new features you want. The best way to do that is making a github issue and giving detailing information about what's wrong or what exactly you want the software to do. And we also have matrix if you don't want to use github for some reason. Feedback is always welcome but please understand that you are commenting on a PR that was already approved and merged and by default the PR author gets pinged for every comment.

If you don't think an issue should be closed that's totally fine, but please bring that up in the comments of that issue or you can always open up a new one. It's no big deal to open a new issue. We only really close issues if they're fixed or they're a duplicate.

If you want to test PRs and help find bugs etc. we love that. Once it's merged though, there's no going back and adding or changing one thing. You either revert the PR and redo it or make a new one with the changes.

So I don't mind you being critical of the PR and talking about it not meeting your needs I just don't understand why you are doing it here. The comments on this PR will get lost. No one is going to go back and read them. Issues on the other hand can be tagged, tracked, put into projects, searched etc.

Maybe I should have been more clear about private users not being supported? That's fair and can be fixed in the changelog whenever this gets released.

I think this PR will meet the needs of most people. It meets my needs and I'm very excited about using it. It would be very helpful if you would document your specific use case and wants in a new issue so they don't get buried or lost. Then someday someone else can pick up where I left off and hopefully move us closer to the netflix like experience you are looking for 👍

[cewert]

@dathbe It was a lot easier than I thought it'd be if you'd like to test #1386

[arcoast]

@cewert Can I just say thank you for all this work!

I'm watching this keenly as this has been the blocker to me using Jellyfin at home. I can't wait for your next PR to be merged and hopefully a new release pushed.

Really appreciated.

[dathbe]

@dathbe It was a lot easier than I thought it'd be if you'd like to test #1386

That's awesome. Thanks. I don't see anywhere I can download a full zip of this build pre-merge. And I've tried a couple times to edit the files manually with the new changes, but I haven't had any luck. I'm probably doing something wrong. Hopefully this change gets merged into the unstable channel and I will try it then if I can't figure out how to do it before that.