MODELIX-1042 authorization for workspaces #1190

slisson · 2024-11-20T09:17:56Z

This is the result of replacing the keycloak based authorization in modelix.workspaces with our own authorization library.

github-actions · 2024-11-20T10:19:57Z

Test Results

188 files 188 suites 31m 42s ⏱️
1 052 tests 1 045 ✅ 7 💤 0 ❌
1 062 runs 1 055 ✅ 7 💤 0 ❌

Results for commit 817f454.

♻️ This comment has been updated with latest results.

github-actions · 2024-11-20T10:20:02Z

JVM coverage report

Overall Project	54.54% `-1.63%`	❌
Files changed	57.57%	❌

File	Coverage
ModelServerPermissionSchema.kt	100%	🍏
CompositeJWSKeySelector.kt	100%	🍏
KeycloakTokenConstants.kt	100%	🍏
ModelixTokenConstants.kt	100%	🍏
AccessTokenPrincipal.kt	96.3%	🍏
PermissionEvaluator.kt	95.97%	🍏
DiffView.kt	89.77% `-3.46%`	🍏
NonWrittenEntry.kt	89.55% `-2.99%`	❌
KeyValueLikeModelServer.kt	87.21% `-0.33%`	❌
ModelixJWTUtil.kt	85.09% `-14.91%`	🍏
SchemaBuilder.kt	84.33%	🍏
AuthorizationConfig.kt	82.9% `-13.47%`	❌
SchemaInstance.kt	80.6% `-12.46%`	❌
PermissionSchemaBase.kt	73.27% `-26.73%`	❌
WrittenEntry.kt	71.7%	🍏
AuthorizationPlugin.kt	67.69% `-10.22%`	❌
KtorAuthUtils.kt	67.49% `-9.36%`	❌
ContentExplorer.kt	66.77% `-23.29%`	❌
IAccessControlDataProvider.kt	50% `-50%`	❌
AccessControlData.kt	38.31% `-61.69%`	❌
PermissionManagementPage.kt	26.65% `-73.35%`	❌
Main.kt	19.36% `-0.32%`	❌
RepositoryOverview.kt	18.13% `-23.23%`	❌
HistoryHandler.kt	0% `-30.52%`	❌

github-advanced-security

detekt found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

odzhychko

I am troubled with the decision that "a user can grant his own permission to other users".
To me, it seems unconventional and not a good security practice.
But maybe I am missing some crucial step in reasoning about this decision, that you could explain.

Their other annotations are minor and mostly concerned with clean up and resource usage.

authorization/src/main/kotlin/org/modelix/authorization/AuthorizationConfig.kt

authorization/src/main/kotlin/org/modelix/authorization/PermissionManagementPage.kt

authorization/src/main/kotlin/org/modelix/authorization/AuthorizationPlugin.kt

authorization/build.gradle.kts

authorization/src/main/kotlin/org/modelix/authorization/AuthorizationConfig.kt

authorization/src/main/kotlin/org/modelix/authorization/AuthorizationPlugin.kt

…to Keycloak checks

fix MODELIX-1018

Identity tokens don't contain any permissions. The permissions are then loaded based on the user ID and roles.

… private key)

At `/permissions/manage` users can grant permissions to other users based on the user ID from the JWT identity token. The data is persisted to the file specified in the environment variable `MODELIX_ACCESS_CONTROL_FILE`.

… 401

When a user is assigned the role modelix-admin in keycloak then he should have admin permissions on the model-server.

RemoteJWKSet already caches keys from remote URLs, but all instances of key sources weren't reused.

…ssions The previous behavior of allowing to grant your own permission to others might be too risky. It's hardcoded to

slisson · 2024-12-12T07:45:51Z

🎉 This PR is included in version 10.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

slisson force-pushed the MODELIX-927-VNC branch from 1c1cd96 to 8b69523 Compare November 20, 2024 10:02

github-advanced-security bot found potential problems Nov 20, 2024

View reviewed changes

slisson force-pushed the MODELIX-927-VNC branch 9 times, most recently from f63e3ef to c6ab4b7 Compare November 25, 2024 12:11

slisson marked this pull request as ready for review December 5, 2024 17:58

slisson requested a review from a team as a code owner December 5, 2024 17:58

slisson enabled auto-merge December 5, 2024 18:16

odzhychko requested changes Dec 9, 2024

View reviewed changes

odzhychko reviewed Dec 9, 2024

View reviewed changes

authorization/src/main/kotlin/org/modelix/authorization/PermissionManagementPage.kt Outdated Show resolved Hide resolved

odzhychko reviewed Dec 10, 2024

View reviewed changes

authorization/src/main/kotlin/org/modelix/authorization/AuthorizationPlugin.kt Show resolved Hide resolved

languitar reviewed Dec 10, 2024

View reviewed changes

fix(authorization): MODELIX_PERMISSION_CHECKS_ENABLED wasn't applied …

8179c75

…to Keycloak checks

slisson force-pushed the MODELIX-927-VNC branch 3 times, most recently from 87a047a to 74ef1d4 Compare December 10, 2024 16:48

slisson added 6 commits December 11, 2024 10:25

feat(authorization): support for RSA keys

9d9a8bd

fix(authorization): ignore unknown granted permissions

e4b5415

fix MODELIX-1018

feat(authorization): support for identity tokens

31c2c16

Identity tokens don't contain any permissions. The permissions are then loaded based on the user ID and roles.

fix(authorization): trust own tokens (also add public key when adding…

165d453

… private key)

fix(authorization): publish sources jar

be8bd81

fix(authorization)!: remove unused keycloak based authorization

35a5518

feat(authorization): option to install status pages

16a31f3

slisson force-pushed the MODELIX-927-VNC branch from 74ef1d4 to 0c04276 Compare December 11, 2024 09:25

slisson added 10 commits December 11, 2024 11:32

feat(authorization): built-in permission management

5aeb53b

At `/permissions/manage` users can grant permissions to other users based on the user ID from the JWT identity token. The data is persisted to the file specified in the environment variable `MODELIX_ACCESS_CONTROL_FILE`.

fix(model-server): /history /content /diff and /repos always returned…

693f3bd

… 401

fix(model-server): RestWebModelClient couldn't request a clientId

ff08972

fix(authorization)!: remove unused keycloak based authorization

71dc994

fix(model-server): make modelix-admin also model-server admin

dd1fe1d

When a user is assigned the role modelix-admin in keycloak then he should have admin permissions on the model-server.

fix(model-datastructure): implement IKVEntryReference.toString

01812f5

fix(authorization): also install status page handler for Throwable

ce06948

docs(authorization): some more documentation after review

a6bf536

chore(authorization): deduplicate constant strings

2201fa0

fix(authorization): cache remote keys

64053df

RemoteJWKSet already caches keys from remote URLs, but all instances of key sources weren't reused.

slisson force-pushed the MODELIX-927-VNC branch from 03dcb35 to 64053df Compare December 11, 2024 10:32

chore(authorization): move extractUserId to companion object

11c6d46

slisson force-pushed the MODELIX-927-VNC branch 2 times, most recently from f2d2afb to 85a72b3 Compare December 11, 2024 19:35

fix(authorization): allow only resource owners/admins to manage permi…

817f454

…ssions The previous behavior of allowing to grant your own permission to others might be too risky. It's hardcoded to

slisson force-pushed the MODELIX-927-VNC branch from 85a72b3 to 817f454 Compare December 12, 2024 06:18

slisson disabled auto-merge December 12, 2024 07:44

slisson merged commit 9d36c70 into main Dec 12, 2024
22 checks passed

slisson deleted the MODELIX-927-VNC branch December 12, 2024 07:44

slisson added the released label Dec 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MODELIX-1042 authorization for workspaces #1190

MODELIX-1042 authorization for workspaces #1190

slisson commented Nov 20, 2024 •

edited

Loading

github-actions bot commented Nov 20, 2024 •

edited

Loading

github-actions bot commented Nov 20, 2024 •

edited

Loading

github-advanced-security bot left a comment

odzhychko left a comment

slisson commented Dec 12, 2024

MODELIX-1042 authorization for workspaces #1190

MODELIX-1042 authorization for workspaces #1190

Conversation

slisson commented Nov 20, 2024 • edited Loading

github-actions bot commented Nov 20, 2024 • edited Loading

Test Results

github-actions bot commented Nov 20, 2024 • edited Loading

JVM coverage report

github-advanced-security bot left a comment

Choose a reason for hiding this comment

odzhychko left a comment

Choose a reason for hiding this comment

slisson commented Dec 12, 2024

slisson commented Nov 20, 2024 •

edited

Loading

github-actions bot commented Nov 20, 2024 •

edited

Loading

github-actions bot commented Nov 20, 2024 •

edited

Loading