Implement Canonical Linking + Self Service

[strazto]

Resolves #28

MVP finished, enough to improve functionality & enable bigger changes.

Future work (Another PR)

Backend:

• Remove preferred_username from canonical linking

Frontend:

• remove link
• indicate existing links
• Removing links should probably have a modal or something, since it would be non-trivial to undo.
  • Maybe a UX like the OSX "lock toggle" could be good? Maybe I don't care that much
• Admin view for providers
  • Maybe a table view of users + providers, and add / remove links
  • This is lower priority

Thanks completed within this PR

Currently I've implemented an internal data structure for linking user accounts to canonical names, and an endpoint for users to register their own canonical names with these links.

The endpoint should allow admins to create links for any user, and any user can create links for only themselves.

The flow is as follows:

• Currently, any authenticated user can navigate to server/sso/{mode}/p/{provider}/isLinking=true to initiate a SSO flow - the provider redirects back to the "normal" callback page, which then posts to the account linking API which valdiates the auth state, and creates the link.
• To distinguish linking flows from auth flows, we need to retain some state
• For saml flows, I've used RelayState, which basically adds a query param that gets passed to the SAML server, which passes it back unmodified in the callback URI
  • The saml callback now checks for this query param to determine whether this flow is for linking or for auth
• For OID flows, I've added an extra field to the TimedAuthorizationState that indicates whether that flow state is for a linking flow, or auth flow.
• The callback checks whether this is a linking flow or not, and generates the client redirect
• If its a linking flow, the client redirect uses the credentials from the current session to post an authenticated linking request.
• If the session's credentials aren't valid, then the linking request should 402
• The rest of the flow proceeds normally

Task list:

Backend:

• Store mappings between name in IDP (Canonical Names) -> Jellyfin user ID
• Change relationship of mapping from CanonicalName : Jellyfin Name to CanonicalName : Jellyfin Id
  • If users are registered using this plugin using a provider that doesnt support friendly names, such as a google, the username of the new account will still be numeric, however, an admin can now change the username to something better manually afterwards, and the linking will be intact
• Allow existing users to add SSO providers to their account
• Allow user registration that support friendly names (Without admin fixup afterwards)
  • I'd rather save this for another PR, I dont care about this feature and it's probably a chore
• Deletion of SSO mappings
  • I guess I probably do need to implement this - This will also require admin or user auth, and will involve searching for the relevant jellyfin ID within the provider, and deleting it
  • It should be straightforward, and it's pretty important so I guess I'll do it now
• List canonical links for a given user (require auth)
  • This is also fairly important, as to manage one's registrations, one will also need to be able to list them
  • Presently, it's technically possible for a given JF user to be registered to multiple accounts on the same provider, as we map
    as follows:

    provider:
       cname_1: jellyfin_id1
       cname_2: jellyfin_name2
       cname_3: jellyfin_id1

Technically, `jellyfin_id1` could login with either `cname_1` or `cname_3`.
I dont think this is bad, but it does add some complexity to the view of managing one's logins.

Frontend

• User view for linking providers
  • List each provider,
  • option to add link

[strazto]

@9p4 ready for review - so far I've implemented the backend, and just need to implement the view for user linking, which should be straightforward.

(I also need to implement some un-linking stuff but it shouldn't be hard)

For now, I'd like to move the next part of this (frontend, deletion) to a separate issue, and get this merged, and a new release out.

When this is fully finished, maybe a major version increment would be appropriate.

[strazto]

Okay, I've implemented a rudimentary UI for users to link their accounts - For now it doesn't actually seem to be working on non-admins, I'm not sure about why

[9p4]

I think that the best way to go about this is to ignore the preferred_username attribute in the OpenID connect response altogether. instead, use the sub attribute to create a unique identifier and during the linking phase, allow the user to create an account if it doesn't exist but if it does, use the sub attribute to link to the user.

[strazto]

I think that the best way to go about this is to ignore the preferred_username attribute in the OpenID connect response altogether. instead, use the sub attribute to create a unique identifier and during the linking phase, allow the user to create an account if it doesn't exist but if it does, use the sub attribute to link to the user.

It sounds like you're suggesting we should remove the part of the code that attempts to use preferred_username for the initial link.
If we do this, it's likely we'll break backward compatibility with previous versions.

I'd like to leave breaking changes out of this PR, as presently, the behaviour introduced here should be backwards compatible with previous behaviour, while being resilient to user-name change ( CreateCanonicalLinkAndUserIfNotExist first looks for a link to a user ( name : jfId ), if that's missing, it searches for a direct cname: jfUsername correspondance, and if neither exists, creates a user with jfname = cname.

If it had to create a user, or match on name, it creates a link cname : jfId

Subsequent logins will find the linked cname : jfId, and skip the other steps

https://github.com/9p4/jellyfin-plugin-sso/pull/34/files#diff-dfecb7a97088edc83a36c769a058950b7dbd99dde1e44990682b3cde24381ef8R625-R656

The plugin is still in an early iteration, and we can semver the breaking change, so I don't think this is out of the question.
sub is more guaranteed in the OIDC spec, so it'd be wise to use it, and reduce linking complexity.

One solution is, if a request does indeed make a new account, after successful auth, redirect the user to a page where they update their username.

Although jellyfin doesn't appear to expose a view in the web-client for regular users to update their username, we have a few options:

Use their endpoint: https://github.com/jellyfin/jellyfin/blob/master/Jellyfin.Api/Controllers/UserController.cs#L349-L382 in a custom view

Or implement our own endpoint that only allows config of the username. Personally I'd rather this as it'll be a simpler API call, and it's pretty easy to write the method.

[9p4]

IMO it's fine to have breaking changes at the moment. This plugin isn't quite at release (or at beta for that matter), and I hope that everyone knows that. If making a breaking change during this part of the development phase improves the plugin, I would say to go for it.

As for updating the user, might as well use their API for it. During linking, I'm sure that the user would like to change more than just their username, so might as well allow it and reduce complexity on our end.

[strazto]

IMO it's fine to have breaking changes at the moment. This plugin isn't quite at release (or at beta for that matter), and I hope that everyone knows that. If making a breaking change during this part of the development phase improves the plugin, I would say to go for it.

You're probably right. I don't like the idea of having this weird funky fallback

As for updating the user, might as well use their API for it. During linking, I'm sure that the user would like to change more than just their username, so might as well allow it and reduce complexity on our end.

It wouldn't really reduce complexity tbh, we'd have to implement the userDto data structure on the frontend.
I'd like the front-end to do only the bare minimum for account administration if possible

[strazto]

Development has stalled slightly because I've been unable to access the config ui views as non-admin users, so I've been unable to implement a linking UI

Oddly enough, other people in the jf Dev matrix room haven't been able to reproduce the issue of not being able to access plugin views on non admin accounts.

Because of the breaking changes we proposed around what OIDC property to use as cname, we basically need to bring the linking UI functionality so this isn't a backwards step, but I need to figure out why I can't access plugin views, and whether we need to roll our own html webresponse as a workaround, similar to how you did the client-side redirect

UPDATE - Actually, others could reproduce this issue. I'm unclear if its a bug or a feature

[strazto]

Seems like this doesn't work when served from a subdirectory on the /SSOViews/linking path

Hm, this isn't the first time I've stuffed these paths up like this.
I'll try to reproduce this, but can you remind me, is this a reverse proxy / load balancer config thing, or does this belong to the jellyfin instance itself?

doesn't seem to work at all for non-admin users from the web-based dashboard.

That's not a bug (not our bug at least), that's expected, and the entire reason the SSOViews controller exists

[9p4]

I'll try to reproduce this, but can you remind me, is this a reverse proxy / load balancer config thing, or does this belong to the jellyfin instance itself?

It's a Jellyfin config under the networking admin page

[strazto]

It's a Jellyfin config under the networking admin page

Thanks, I'll check it out and debug what exactly is breaking - I'll do that now before I get distracted

[strazto]

Seems like this doesn't work when served from a subdirectory on the /SSOViews/linking path

This was a bug in the snippet I copied from https://github.com/jellyfin/jellyfin-web/blob/9067b0e397cc8b38635d661ce86ddd83194f3202/src/scripts/clientUtils.js#L19-L76

Basically, their implementation searched for a /web path-component to designate the end of the baseUrl, ( ie: {baseUrl}/web/{route} but when we serve from /SSOViews/, there is no such /web component (instead it's {baseUrl}/SSOViews/{route} )

Should be fixed by 510624f