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.

Sign up for GitHub

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

Jump to bottom

Internal Error 500 on Login #514

Open
JagandeepBrar opened this issue Dec 14, 2017 · 10 comments · May be fixed by #533
Open

Internal Error 500 on Login #514

JagandeepBrar opened this issue Dec 14, 2017 · 10 comments · May be fixed by #533
Assignees
@RickyGrassmuck
Labels
active bug Major

Comments

@JagandeepBrar
Copy link
Contributor

JagandeepBrar commented Dec 14, 2017 •
edited
Loading

Issue:

Getting an internal error when trying to login with either a username or email when "Require users to login with their passwords" is NOT enabled, but "Enable user authentication" is enabled.

The error occurs in this file: https://github.com/lokenx/plexrequests-meteor/blob/master/server/methods/authentication/plexAuthentication.js

In the function 'checkPlexUser', specifically in the return case. This is because earlier, there is an if condition:

if (Settings.find({}).fetch()[0].plexAuthenticationPASSWORDS) {
    // If passwords are required check full login
    var userInfo = Meteor.call('plexLogin', plexLogin, plexPassword)
    var plexUsername = userInfo.username
}

plexUsername is not defined if plexAuthenticationPASSWORDS isn't enabled, and in the return:

return (friendsList.indexOf(plexUsername.toLowerCase()) > -1), the toLowerCase() function fails on a null variable.

Attempted Solution:

I set plexUsername before the if condition to the plexLogin supplied to the function, and this worked and allowed login via plex username but it now fails to login throwing an incorrect login error instead. This is because that if statement fetched the username via the login information.

I am not sure if the Plex API allows you to fetch a username from an email without a password.
@RickyGrassmuck
Copy link
Collaborator

RickyGrassmuck commented Dec 14, 2017 •
edited
Loading

Ahh, that's an oversight on my part.

Think about it, there are a couple ways to address this.

  1. Simply restrict user login without a password to using the user's username.

  2. Get rid of not requiring a password for user logins(I've never really seen the point of this anyways, logins should be authenticated otherwise all that's needed is a friend's username to be able to request)

  3. See if the friends list data has their Plex email somewhere in the data and go ahead and store both for logging in as a user.

Not sure which route I'll go with, may go ahead implement option 1 as a temporary measure to get it working until I can make up my mind.

@RickyGrassmuck
Copy link
Collaborator

Posting this here for myself to come back to. So it looks like we can get the email and username when looking up the friend's list.

  <User id="#######" title="rickys.guest" username="rickys.guest" email="[email protected]" recommendationsPlaylistId="<Bunch-o-numbers>" thumb="https://plex.tv/users/f82c6f979369d155/avatar?c=1511798278">
    <Server id="#######" serverId="########" machineIdentifier="<Allot-of-characters-here>" name="MyAwesomeServer" lastSeenAt="1513310941" numLibraries="2" owned="1"/>
  </User>

Knowing that we have that data to work with, we can store the email along with the username.

I wasn't the one that implemented the user permissions so I have started familiarizing myself with that functionalities codebase and it's honestly in major need of refactoring.

This is likely going to be a bit of a project as I would rather rework the logic behind this area of the application to make it more complete and stable.

In the meantime, I would recommend enable requiring password authentication for user login's.

@RickyGrassmuck RickyGrassmuck self-assigned this Dec 15, 2017
@gdavidp
Copy link

gdavidp commented Dec 16, 2017

How does that work with Managed Users though? They don't have an account on Plex.tv, or is this a local account that exists in the MongoDB?

@RickyGrassmuck
Copy link
Collaborator

RickyGrassmuck commented Dec 16, 2017 via email

@RickyGrassmuck
Copy link
Collaborator

Ok, just an update(I haven't forgotten about this), I found what looks to be the current endpoint for getting a plex accounts friends list that includes whether or not the user is a home user!!

https://plex.tv/api/users

{ id: '16396035',
    title: '#######',
    thumb: '######',
    protected: '0',
    home: '1',
    allowSync: '0',
    allowCameraUpload: '0',
    allowChannels: '0',
    allowTuners: '0',
    filterAll: '',
    filterMovies: '',
    filterMusic: '',
    filterPhotos: '',
    filterTelevision: '',
    restricted: '1',
    Server:
     { id: '#########',
       serverId: '',
       machineIdentifier: '',
       name: '',
       lastSeenAt: '',
       numLibraries: '2',
       allLibraries: '1',
       owned: '1',
       pending: '0' } },

This should give me what I need to be able to implement this the correct way and clean up the authentication codebase allot.

I'm trying to do this correctly so that we aren't revisiting this down the road so please bear with me.

@tresni tresni linked a pull request Apr 1, 2018 that will close this issue
Open
@Epod
Copy link

Epod commented Apr 16, 2018

I am also experiencing this issue with a fresh install. Using version 1.12.2

Similar logs to above. The only way to leverage this at the moment looks to be to disable authentication entirely.

@sossienl
Copy link

Same issue here.

@bondjw07
Copy link

Just updated yesterday and I'm seeing this issue now too. I agree that it's silly not to require a password, but some people find it insecure to type in their plex password into a non-plex site. ¯_(ツ)_/¯

@Corruptsector
Copy link

I just installed this and having the same issue. Auth has to be disabled.

@kalyway101
Copy link

Hello, I am curious, was this something that was ever fixed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@RickyGrassmuck RickyGrassmuck

Labels
active bug Major
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Don't use plexUsername without initialization tresni/plexrequests-meteor
8 participants
@Epod @sossienl @gdavidp @JagandeepBrar @bondjw07 @Corruptsector @RickyGrassmuck @kalyway101