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

OpenID: Integrate permissions (Fixes #2523) #2769

Merged
merged 11 commits into from
Mar 30, 2024

Conversation

Sapd
Copy link
Contributor

@Sapd Sapd commented Mar 19, 2024

Integrate full permission support.

The user can configure (all optional and independent):

  • A group claim: This will set the user type of the user to either admin, or user or guest. If it is set, the claim must exist also it must include in the list either admin, user or guest or login will be denied. If multiple groups match, the highest group gets priority (e.g. admin).
    If not configured, will like before simply assign "user"
  • An advanced permissions claim (for example named abspermissions). The content should look like this:
{
  "canDownload": false,
  "canUpload": false,
  "canDelete": false,
  "canUpdate": false,
  "canAccessExplicitContent": false,
  "canAccessAllLibraries": false,
  "canAccessAllTags": false,
  "tagsAreDenylist": false,
  "allowedLibraries": ["ExampleLibrary", "AnotherLibrary" ],
  "allowedTags": ["ExampleTag", "AnotherTag", "ThirdTag"
  ]
}

If the whole claim (if configured) is missing, login will be denied. If a parameter like canDownload is missing, it will be treated as false. If a unknown parameter is provided, login will be denied. If user is an admin, it will be ignored.

Screenshot 2024-03-19 at 19 27 06

Can be tested with Authentik like this:

  1. Create a OIDC Property Mapping (Customization -> Property Mappings)
  2. Name it however you want
  3. Name Scope Name for example "groups"

Here is an example expression, which adds the "admin" group if the user is in the "Dev" authentik group:

groups = [group.name for group in user.ak_groups.all()]
if "Dev" in groups:
  groups.append("admin")
return { "groups": groups }

For the advanced permissions, do the same and name the scope something like "abspermissions".

return {
  "abspermissions": {
  "canUpdate": True,
  "canDownload": True,
  "canUpload": False,
  "canDelete": True,
  "canAccessExplicitContent": False,
  "canAccessAllLibraries": True,
  "canAccessAllTags": True,
  "allowedLibraries": [
    "ExampleLibrary",
    "AnotherLibrary"
  ],
  "allowedTags": [
    "ExampleTag",
    "AnotherTag",
    "ThirdTag"
  ],
  "tagsAreDenylist": False
  }
}

Note that here (in python?) the booleans must be upper case.

Screenshot 2024-03-19 at 19 36 18

Also make sure that after saving you click on "Test" beside the mapping and select a test user. It should show for the first mapping the correct groups including for example user or admin. For the second the claim.

Screenshot 2024-03-19 at 19 38 22

Then go to Providers -> Select your ABS provider -> Edit -> Advanced Protocol Settings. And select additionally your new mappings.

I tested it extensively but make sure to also do some tests.


Also whats a bit weird in code I noticed, we use:

global.ServerSettings.

around line 83. But at other places

Database.serverSettings.

Not sure if this has a specific purpose but we should probably make it consistent, esp. as the first S has different case but means the same variable.

* Ability to set group
* Ability to set more advanced permissions
* Modified TextInputWithLabel to provide an ability to specify a different placeholder then the name
@advplyr
Copy link
Owner

advplyr commented Mar 22, 2024

In testing I came across an issue where I set the root user type to admin. We need to make sure the root user type cannot be changed

Only disallow when email_verified explicitly false
Also check username besides preferred_username, even when its not included in OIDC checks (synology uses username)
@Drakon74
Copy link

I tested this on my instance and it fixed the SSO Error i had with Synology for me. Tested it on 3 users, all of them working now.

…hanges

- Update permissions example to use UUIDv4 strings for allowedLibraries
- More validation on advanced permission JSON to ensure arrays are array of strings
- Only set allowedTags and allowedLibraries if the corresponding access all permission is false
@advplyr
Copy link
Owner

advplyr commented Mar 30, 2024

I did a lot more testing and it is working great for me.

I updated the advanced permission check so it will only perform an update to the DB if changes were actually made. It will also only log that is updating the group/permissions if an update is required.

I added more validation to the permission check so we ensure the arrays are valid. The allowedLibraries only gets set if the canAccessAllLibraries is false and same with allowedTags.

The example permissions getting shown now show a UUIDv4 for the allowedLibraries so users don't try putting the library name there.

Thanks!

@advplyr advplyr merged commit a9c9c44 into advplyr:master Mar 30, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants