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

Canonical usernames and linking accounts #28

Closed
strazto opened this issue May 13, 2022 · 9 comments · Fixed by #34
Closed

Canonical usernames and linking accounts #28

strazto opened this issue May 13, 2022 · 9 comments · Fixed by #34
Labels
enhancement New feature or request

Comments

@strazto
Copy link
Collaborator

strazto commented May 13, 2022

From https://github.com/9p4/jellyfin-plugin-sso/projects/1#card-76230086

I believe this a release requirement, and in the long run, very important.

This issue can serve as a discussion of what this means, and how it should be implemented

@strazto strazto added the enhancement New feature or request label May 13, 2022
@9p4
Copy link
Owner

9p4 commented May 13, 2022

My vision is that SSO can be used to sign into an existing account when the usernames differ.

Furthermore, there has to be something for "Sign in with Google" to have better username support.

All providers can add metadata about the user (full name, etc)

@strazto
Copy link
Collaborator Author

strazto commented May 14, 2022

My vision is that SSO can be used to sign into an existing account when the usernames differ.

Furthermore, there has to be something for "Sign in with Google" to have better username support.

hm, yeah. Personally I now use google as a SSO provider for my authentik instance, so it no longer affects me.

There are a few options for something like this -

  • Allow configurable UID Claim in provider config.
    • Users can instead claim on some other property, for example, email address
    • There are plenty of caveats to this, and google warns against it
    • The main problem is that most "human-friendly" properties on a google OIDC payload are either:
      • non-unique to the account
      • not-guaranteed to be immutable
        • email address is an example of this
    • Although configurable UID claim might have some use-case (idk what right now), it's probably not a good solution
  • Allow some form of user-name registration flow when authenticating a new user via OIDC provider
    • This is probably more robust, but likely involves rolling-our-own UI
    • This raises the question:
      • How flexible is jellyfin's user-model when it comes to storing custom metadata + attributes?
        Answer: Not very
      • Where do we store this relationship?
        Answer: Probably in the plugin config, though I'm unsure of how resilient this would be to concurrency, but maybe that doesn't matter.
      • How should we structure this relationship?
        • A given provider has a 1-n relationship with users, perhaps each provider has a hashmap of canonical name -> jellyfin ID?
    • Does jellyfin allow plugins to provide user-facing configuration pages, so that we can give a UI to link existing accounts to providers?

All providers can add metadata about the user (full name, etc)

By "providers" do you mean OIDC providers, or Jellyfin Identity Providers? I know OIDC providers can give plenty of metadata, but I don't know much about jellyfin ID providers

@9p4
Copy link
Owner

9p4 commented May 15, 2022

By "providers" do you mean OIDC providers, or Jellyfin Identity Providers? I know OIDC providers can give plenty of metadata, but I don't know much about jellyfin ID providers

OIDC/SAML providers putting name, etc into JF.

@strazto
Copy link
Collaborator Author

strazto commented May 25, 2022

Here's a discussion I had about implementing user-facing views for user self-service

It was good news, the task seems straightforward

https://matrix.to/#/!YOoxJKhsHoXZiIHyBG:matrix.org/$ylce8_G_pQ5ygkH5Rz5cQHWZy4atA3TO_ntprzYNxd4?via=matrix.org&via=t2bot.io&via=kde.org

Matthew Strasiotto
Howdy! Continuing a discussion with Cody Robibero:
So I've implemented admin config client-side view for my plugin, it lives here
https://github.com/9p4/jellyfin-plugin-sso/tree/main/SSO-Auth/Config
I basically used trakt, ldapauth , and jellyfin-webs library views as references
Developing this admin config view is pretty straightforward for now
I want a user self-service view.
I don't think I can use pluginconfigpage API to register it, I think that's only for admin views
Cody Robibero
The view itself doesn't require permissions
Matthew Strasiotto
Ohhh, okay
Cody Robibero
It's the api the makes the requirements
Matthew Strasiotto
Does the link appear in the side menu for non-admins?
No Biggie if not
Cody Robibero
In 10.8 you can add a line to a json file with the link
Matthew Strasiotto
That makes my task much more straightforward!
Yep, that's what I thought
Cody Robibero
https://jellyfin.org/docs/general/clients/web-config.html#custom-menu-links
Though the server admin would need to set it
Matthew Strasiotto
Cody Robibero
Though the server admin would need to set it
That's no problem, the plugin is maturing anyway, and I get a feeling that someone will probably add user-side config for the custom menu link setting to jf eventually
(I would imagine that setting eventually living next to the disclaimer and branding stuff)
Awesome! All I need to do is use built-in self service endpoints as a reference for how to check role/permissions, and then implement a separate plugin config view that communicates with those endpoints

https://github.com/jellyfin/jellyfin/blob/433853b24cc9c3e931885818b515537befc946f3/Jellyfin.Api/Controllers/UserController.cs#L249-L305

Cody Robibero
That sounds about right
Matthew Strasiotto
So I guess presently if I manually type in the URL for a plugin config page on a non-admin account, the page will load correctly and everything will work until the ApiClient tries to access an endpoint that it doesn't have permission for
Awesome
Cody Robibero
Correct

@strazto
Copy link
Collaborator Author

strazto commented May 27, 2022

@9p4 I think I'd like your eyes on #34
In particular, I'm not sure about the best way to build an an endpoint for linking pre-existing users to idenity providers.

The current endpoint is probably not good enough - rigth now it just lets users manually set their canonical name for a given provider.

I think instead, we need something which initiates the flow, but the callback can differentiate between linking and auth requests, and we can validate the response based on that.

I kind of feel like there should be a GET endpoint that actually initiates the flow, client-side flow,

  • and then the client-side flow initiates the flows with the provider,
  • and accepts the callbacks they need to, except that, this time, (ideally)
  • the client passes some extra state to the provider which is passed back on callback to the client (Does saml and oidc support this) which flags that we're just linking here, not authorizing.

EDIT
Actually, for OIDC, we can just use statemanager to differentiate auth requests from linking requests and delegate the correct method.

For SAML, I'm less clear on the correct way to distinguish these flows.

EDIT

I actually think we can implement linking endpoints that mirror the existing
/sso/{mode}/Auth/{provider} endpoints, instead:
/sso/{mode}/Link/{provider}/{jellyfinUserId}

Just like with /sso/{mode}/Auth/{provider}, we generate a js client that includes the OIDC / SAML state data (XML, or state id), which queries our endpoint, /sso/{mode}/Link/{provider}/{jellyfinUserId}, but:

  • We also add the {jellyfinUserId} to the generated page
  • our endpoint requires basicAuth,
    • because the user already has a session, it can use the AssertCanUpdateUser helper to verify the user has permission to create this link
  • Our endpoint then verifies the auth response data

Third edit (lol)

Actually, for saml requests, we can use relayState to keep state & track the kind of request it is, and for OIDC, we can track whether each request is linking or not within the TimedAuthroizeState object

@9p4
Copy link
Owner

9p4 commented May 30, 2022

I think that the auth and linking would be ideally combined. On first login, the linking request will be created. Then, every other login would automatically work.

@strazto
Copy link
Collaborator Author

strazto commented May 31, 2022

I think that the auth and linking would be ideally combined. On first login, the linking request will be created. Then, every other login would automatically work.

That's what I wound up making, I believe

@9p4
Copy link
Owner

9p4 commented May 31, 2022

Ah I must have misunderstood the code. I'll have to take a more careful look through.

@strazto
Copy link
Collaborator Author

strazto commented Jun 4, 2022

Ah I must have misunderstood the code. I'll have to take a more careful look through.

Either that, or an old revision, I wound up re-doing it to go from linking req to flow, without an api call that explicitly links

@9p4 9p4 closed this as completed in #34 Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants