Login error handling

[RA341]

Changes

Added support for autofill so that password managers can autofill credentials.

Ui changes

Default error handling UI.

Specific error messages for certain HTTP codes. If a response does not match a specific code, it defaults to the normal UI.

Added a check for empty inputs.

Bug fix

There was also a bug in the auth logic

The if condition adds a port regardless of the protocol, which will cause a tls mismatch error if we put https address and the error handler adds the port.

relevant line

if (user.serverAdress!.split(":").last != "8096" &&
          user.serverAdress!.split(":").length == 2 

adds a HTTP check

if (user.serverAdress!.split(":").last != "8096" &&
          user.serverAdress!.split(":").length == 2 &&
          user.serverAdress!.split(":").first == 'http')

[sevenrats]

These need to reflect more standard representations of the code status. 401 is fine because this is a totally standard way of using it in jellyland, but the other two are a bit misleading. just go with "400 - Bad Request" or "403 - Forbidden"

[RA341]

I don't mind leaving them out, but here's my reasoning

400 - can only happen if the client library (tentacle) messes up a request or the server doesn't recognize the path for whatever reason either way its a bug within the app

403 - I guess is debatable but this only occurs if the server has banned your device IP or ratelimit I guess either way still relevant message.

My assumptions could be wrong so feel free to correct me.

[sevenrats]

imho it depends where in the flow we are making this determination. if we have already validated that we are talking to a jellyfin server and not something else in error, I think this 400 is ok. the 403 is a problem though because lots of different reverse proxy stuff could cause this.

[sevenrats]

tbh i have problems with the 400 too because of the potential issues with proxies. if there is a 400 or 403, it would be really nice to give the user as much info about it as possible instead of generalized lines. what do you think about using the response content itself in these cases?
@jdk-21 do you have an opinion on this one?

[RA341]

ah i hadn't considered proxies, we could validate that we are talking to jellyfin first, based on this auth logic, if we get 400ed before that we show a normal exception for the proxy

[sevenrats]

truth be told I don't see the wisdom in this function at all. this is a bit backwards in general because 1) you are covering over information about the real error that might be useful to the user or troubleshooter and then 2) you funnel these people into the support channels with less info than they might have had. this has negative impacts both on the UX and the support burden.
why isn't it better to unconditionally use Text(e.toString())

[RA341]

To be clear, the function does not hide the HTTP error, it adds this message on top of the HTTP response (see image in the PR)

e.tostring will use dios inbuilt error response, which is a long (relatively)useless string you can see here, which is the reason why I added the messages

[sevenrats]

ah, well I take it back, I see the wisdom in that. how do you feel about the ux/support burden element?

[RA341]

If we can workshop the messages, and add like a FAQ section, I feel like it shouldn't be too bad.

[RA341]

this is the final message that gets displayed, let me know if you want additional info included

return '$message\n\n'
            'Http Code: ${resp.statusCode ?? 'Unknown'}\n\n'
            'Http Response: ${resp.statusMessage ?? 'Unknown'}\n\n'
        .trim();

[sevenrats]

this will not work. what if my server is at http://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]
its true that the previous code was broken in this way as well, but not taking this opportunity to fix it would be silly. are you completely against using the regex I linked you? its really one of very few correct ways of approaching this issue.

[sevenrats]

I strongly recommend following an approach like this (but avoiding the brightscriptisms, obvi):
https://github.com/jellyfin/jellyfin-roku/blob/4e872f42936e02ca2134933dc1b3186d219c14e9/source/utils/misc.bs#L185
https://github.com/jellyfin/jellyfin-roku/blob/4e872f42936e02ca2134933dc1b3186d219c14e9/source/utils/misc.bs#L238
using this regex:
https://github.com/jellyfin/jellyfin-roku/blob/4e872f42936e02ca2134933dc1b3186d219c14e9/source/utils/misc.bs#L364

[sevenrats]

if you were open to fixing this while you were mucking about in this view that would be very helpful:
#63

[RA341]

if you were open to fixing this while you were mucking about in this view that woul

if you were open to fixing this while you were mucking about in this view that would be very helpful: #63

I got it

#63 is implemented

[sevenrats]

looks great works great.
sorry this didn't occur to me earlier, but, jellyfin actually supports empty passwords.
for example this branch now prevents jellyflix from accessing the demo server at https://demo.jellyfin.org/stable

[RA341]

removed the password check

[sevenrats]

@jdk-21 I'm plus one on this when you are ready to review.

[sevenrats]

looks great, works great, responds properly to my modest fuzzing attempts.

[RA341]

adding this one last change, to show URLs tried

[jdk-21]

Thank you for your contribution. LGTM
And @sevenrats for the review