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

Add 'Enable All Folders' or selected folder list for LDAP user create #90

Merged
merged 11 commits into from
Nov 4, 2021

Conversation

jketreno
Copy link
Contributor

I needed to be able to limit new users to only have access to specific content on my network. I like the way editing of individual library access is performed, so extended that concept into the LDAP plugin configuration.

THe HTML view is derived from jellyfin-web/src/controllers/dashboard/users/userlibraryaccess.html

Signed-off-by: James Ketrenos james_github@ketrenos.com

Copy link
Member

@crobibero crobibero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested the changes yet, but brief review comments

LDAP-Auth/Config/PluginConfiguration.cs Outdated Show resolved Hide resolved
LDAP-Auth/Config/configPage.html Outdated Show resolved Hide resolved
@jketreno
Copy link
Contributor Author

jketreno commented Nov 1, 2021

It looks like the actions to build/run/test are requiring .net6, but the project is .net5 -- is there anything I can do on the PR to correct this? Is Jellyfin moving to .net6?

Thanks,
James

@crobibero
Copy link
Member

Yes, Jellyfin is now on dotnet6. If you rebase you'll have the 10.8 changes :)

@jketreno
Copy link
Contributor Author

jketreno commented Nov 1, 2021

Got it. Thanks. I rebased and updated my branch w/ a force push. Will the workflows auto re-trigger?

Cheers,
James

@crobibero
Copy link
Member

I haven't forgotten about this, just haven't had time to test the changes

@jketreno
Copy link
Contributor Author

jketreno commented Nov 3, 2021

No worries; I'll resolve the new merge conflict and push an update later today.

I needed to be able to limit new users to only have access to specific
content on my network. I like the way editing of individual library access
is performed, so extended that concept into the LDAP plugin configuration.

THe HTML view is derived from jellyfin-web/src/controllers/dashboard/users/userlibraryaccess.html

Signed-off-by: James Ketrenos <james_github@ketrenos.com>
Signed-off-by: James Ketrenos <james_github@ketrenos.com>
Signed-off-by: James Ketrenos <james_github@ketrenos.com>
Signed-off-by: James Ketrenos <james_github@ketrenos.com>
…ation' is disabled

The visibility of Library Access wasn't being set during load based
on the Enable User Creation state; instead it was only updating on change.

Moved the code into a helper function for the event handler and call it
during creation.

Signed-off-by: James Ketrenos <james_github@ketrenos.com>
Signed-off-by: James Ketrenos <james_github@ketrenos.com>
The 'pageshow' handler is executing any time any page is loaded. This
patch adds an event filter at the start of 'pageshow' to only execute
if the LDAP-Auth page is active.

Signed-off-by: James Ketrenos <james_github@ketrenos.com>
@jketreno
Copy link
Contributor Author

jketreno commented Nov 3, 2021

I just pushed a rebased series. The indentation from the fix for #92 caused some hiccups in the merge. I also pushed a patch to prevent the 'pageshow' handler from executing when LDAP-Auth isn't the active page (so the document.querySelector references are all valid -- switching in and out of different views was eventually causing an undefined reference without the change.) I have this all running locally again, and verified new LDAP users only get the configured libraries enabled.

Cheers,
James

Copy link
Member

@crobibero crobibero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great, just 2 changes

LDAP-Auth/Config/configPage.html Outdated Show resolved Hide resolved
LDAP-Auth/LDAPAuthenticationProviderPlugin.cs Outdated Show resolved Hide resolved
jketreno and others added 3 commits November 4, 2021 00:13
While this was working before, it has not been working with the latest version. Switching to filter from JavaScript instead of the selector is working now.

Signed-off-by: James Ketrenos <james_github@ketrenos.com>
Co-authored-by: Cody Robibero <cody@robibe.ro>
Co-authored-by: Cody Robibero <cody@robibe.ro>
@crobibero crobibero added the feature This PR or Issue requests or introduces a new feature label Nov 4, 2021
@crobibero crobibero merged commit 64d2f6b into jellyfin:master Nov 4, 2021
@jketreno jketreno deleted the folder-ldap-access branch November 4, 2021 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This PR or Issue requests or introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants