Skip to content

Commit c4cd079

Browse files
tgloeggltgloeggl
authored
Cleanup ACLs, fixes #998 (#1014)
* check for endpoints * working on removing STUDIP_ namespace * add todo * fix permissions for admin * remove fixed todos * fix admin roles, handel deputies * correctly compare and filter acls
1 parent 3f9bf45 commit c4cd079

File tree

9 files changed

+104
-53
lines changed

9 files changed

+104
-53
lines changed

‎lib/Models/Helpers.php‎

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -280,19 +280,19 @@ public static function createACLsForCourses($course_ids)
280280
foreach ($course_ids as $course_id) {
281281
$acl[] = [
282282
'allow' => true,
283-
'role' => 'STUDIP_'. $course_id . '_Instructor',
283+
'role' => $course_id . '_Instructor',
284284
'action' => 'read'
285285
];
286286

287287
$acl[] = [
288288
'allow' => true,
289-
'role' => 'STUDIP_'. $course_id . '_Instructor',
289+
'role' => $course_id . '_Instructor',
290290
'action' => 'write'
291291
];
292292

293293
$acl[] = [
294294
'allow' => true,
295-
'role' => 'STUDIP_'. $course_id . '_Learner',
295+
'role' => $course_id . '_Learner',
296296
'action' => 'read'
297297
];
298298
}
@@ -307,7 +307,7 @@ public static function createACLsForCourses($course_ids)
307307
*
308308
* @return array
309309
*/
310-
public static function filterACLs($acls)
310+
public static function filterACLs($acls, $studip_acls)
311311
{
312312
if (!is_array($acls)) {
313313
return [
@@ -316,25 +316,43 @@ public static function filterACLs($acls)
316316
];
317317
}
318318

319-
$possible_roles = [
320-
'ROLE_ANONYMOUS'
321-
];
319+
// prevent duplicate ACLs
320+
$temp_acls = [];
321+
foreach ($acls as $acl) {
322+
$temp_acls[$acl['allow'] .'#'. $acl['role'] . $acl['action']] = $acl;
323+
}
324+
325+
$acls = array_values($temp_acls);
326+
327+
$possible_roles = array_column($studip_acls, 'role');
328+
329+
sort($acls);
322330

323331
$result = [];
324332
foreach ($acls as $entry) {
325-
if (in_array($entry['role'], $possible_roles) !== false
326-
|| strpos($entry['role'], 'STUDIP_') === 0
327-
) {
333+
if (in_array($entry['role'], $possible_roles) !== false) {
328334
$result[$entry['role'] .'_'. $entry['action']] = $entry;
329335
}
330336
}
331337

332338
$result = array_values($result);
333339
sort($result);
334340

341+
342+
$diff = array_udiff($acls, $result, ['Opencast\Models\Helpers', 'compareACLs']);
343+
335344
return [
336345
'studip' => $result,
337-
'other' => array_diff($acls, $result)
346+
'other' => $diff
338347
];
339348
}
349+
350+
public static function compareACLs($a, $b)
351+
{
352+
$comp_a = implode('#', $a);
353+
$comp_b = implode('#', $b);
354+
355+
return $comp_a <=> $comp_b;
356+
357+
}
340358
}

‎lib/Models/Playlists.php‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,10 @@ public function getUserPerm()
316316
if ($course->getParticipantStatus($user->id)) {
317317
return 'read';
318318
}
319+
320+
if ($perm->have_studip_perm('dozent', $course->id)) {
321+
return 'owner';
322+
}
319323
}
320324
}
321325

@@ -346,17 +350,17 @@ public static function getDefaultACL($playlistId, $user_id = null)
346350
return [
347351
[
348352
'allow' => true,
349-
'role' => "STUDIP_PLAYLIST_{$playlistId}_read",
353+
'role' => "PLAYLIST_{$playlistId}_read",
350354
'action' => 'read'
351355
],
352356
[
353357
'allow' => true,
354-
'role' => "STUDIP_PLAYLIST_{$playlistId}_write",
358+
'role' => "PLAYLIST_{$playlistId}_write",
355359
'action' => 'read'
356360
],
357361
[
358362
'allow' => true,
359-
'role' => "STUDIP_PLAYLIST_{$playlistId}_write",
363+
'role' => "PLAYLIST_{$playlistId}_write",
360364
'action' => 'write'
361365
]
362366
];
@@ -377,23 +381,19 @@ public static function checkPlaylistACL($oc_playlist, $playlist)
377381
unset($entry['id']);
378382
});
379383

380-
$old_acls = Helpers::filterACLs($old_acl);
381-
382-
$current_acl = $old_acls['studip'];
383-
384384
$acl = self::getDefaultACL($oc_playlist->id);
385385

386386
// add user acls
387387
foreach ($playlist->courses as $course) {
388388
foreach($course->getMembersWithStatus('dozent') as $member) {
389389
$acl[] = [
390390
'allow' => true,
391-
'role' => "STUDIP_" . $member->user_id,
391+
'role' => $member->user_id .'_Instructor',
392392
'action' => 'read'
393393
];
394394
$acl[] = [
395395
'allow' => true,
396-
'role' => "STUDIP_" . $member->user_id,
396+
'role' => $member->user_id .'_Instructor',
397397
'action' => 'write'
398398
];
399399
}
@@ -404,6 +404,10 @@ public static function checkPlaylistACL($oc_playlist, $playlist)
404404

405405
$acl = array_merge($acl, Helpers::createACLsForCourses($courses));
406406

407+
$old_acls = Helpers::filterACLs($old_acl, $acl);
408+
409+
$current_acl = $old_acls['studip'];
410+
407411
foreach ($acl as $entry) {
408412
$found = false;
409413
foreach ($current_acl as $current_key => &$current_entry) {

‎lib/Models/REST/SeriesClient.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,16 +125,18 @@ private static function getSeriesDC($course_id)
125125
*/
126126
public function createSeriesForUser($user_id)
127127
{
128+
$username = strtoupper(get_username($user_id));
129+
128130
$acl = [
129131
[
130132
'allow' => true,
131-
'role' => 'STUDIP_' . $user_id,
133+
'role' => 'ROLE_USER_'. $username,
132134
'action' => 'read'
133135
],
134136

135137
[
136138
'allow' => true,
137-
'role' => 'STUDIP_' . $user_id,
139+
'role' => 'ROLE_USER_'. $username,
138140
'action' => 'write'
139141
]
140142
];

‎lib/Models/SeminarSeries.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ private static function checkSeries($course_id, $series_id)
4040
$courses = \SimpleCollection::createFromArray($series)->pluck('seminar_id');
4141

4242
$acl = Helpers::createACLsForCourses($courses);
43-
$oc_acls = Helpers::filterACLs($series_client->getACL($series_id));
43+
$oc_acls = Helpers::filterACLs($series_client->getACL($series_id), $acl);
4444

4545
if ($acl <> $oc_acls['studip']) {
4646
$new_acl = array_merge($oc_acls['other'], $acl);

‎lib/Models/Videos.php‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -689,8 +689,8 @@ public function removeVideo()
689689
private static function addEpisodeAcl($episode_id, $add_acl, $acl)
690690
{
691691
$possible_roles = [
692-
'STUDIP_' . $episode_id . '_read',
693-
'STUDIP_' . $episode_id . '_write',
692+
$episode_id . '_read',
693+
$episode_id . '_write',
694694
'ROLE_ANONYMOUS'
695695
];
696696

@@ -747,19 +747,19 @@ public function updateAcl($new_vis = null)
747747
$acl = [
748748
[
749749
'allow' => true,
750-
'role' => 'STUDIP_' . $this->episode .'_read',
750+
'role' => $this->episode .'_read',
751751
'action' => 'read'
752752
],
753753

754754
[
755755
'allow' => true,
756-
'role' => 'STUDIP_' . $this->episode .'_write',
756+
'role' => $this->episode .'_write',
757757
'action' => 'read'
758758
],
759759

760760
[
761761
'allow' => true,
762-
'role' => 'STUDIP_' . $this->episode .'_write',
762+
'role' => $this->episode .'_write',
763763
'action' => 'write'
764764
]
765765
];
@@ -773,8 +773,6 @@ public function updateAcl($new_vis = null)
773773

774774
$acl = array_merge($acl, Helpers::createACLsForCourses($courses));
775775

776-
$oc_acls = Helpers::filterACLs($current_acl);
777-
778776
// add anonymous role if video is world visible
779777
if (($new_vis && $new_vis == 'public') || (!$new_vis && $this->visibility == 'public')) {
780778
$acl[] = [
@@ -786,6 +784,8 @@ public function updateAcl($new_vis = null)
786784

787785
sort($acl);
788786

787+
$oc_acls = Helpers::filterACLs($current_acl, $acl);
788+
789789
if ($acl <> $oc_acls['studip']) {
790790
$new_acl = array_merge(
791791
$oc_acls['other'],

‎lib/Routes/Config/ConfigList.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,15 @@ public function __invoke(Request $request, Response $response, $args)
4747

4848
// Checker to provide recourses.
4949
$resources = Resources::getStudipResources();
50-
if (!empty(Endpoints::getEndpoints()) && !empty($resources)) {
50+
$endpoints = Endpoints::getEndpoints();
51+
$response_data['endpoints'] = $endpoints;
52+
53+
if (!empty($endpoints) && !empty($resources)) {
5154
$response_data['scheduling'] = ScheduleHelper::prepareSchedulingConfig($config_list, $resources);
5255
}
5356

57+
58+
5459
if (!PlaylistMigration::isConverted() && count($config_list) &&
5560
version_compare(
5661
OCConfig::getOCBaseVersion(\Config::get()->OPENCAST_DEFAULT_SERVER),

‎lib/Routes/Opencast/UserRoles.php‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public function __invoke(Request $request, Response $response, $args)
4848
if (!empty($share_uuid)) {
4949
$video_share = VideosShares::findByUuid($share_uuid);
5050
if (!empty($video_share)) {
51-
$roles[] = 'STUDIP_' . $video_share->video->episode . '_read';
51+
$roles[] = $video_share->video->episode . '_read';
5252
} else {
5353
throw new Error('Share not found', 404);
5454
}
@@ -63,12 +63,28 @@ public function __invoke(Request $request, Response $response, $args)
6363
$email = $user->email;
6464
$fullname = $user->getFullName();
6565

66-
// Add user permission to access user-bound series and own playlists
67-
$roles[] = 'STUDIP_' . $user_id;
68-
6966
// Stud.IP-root has access to all videos and playlists
7067
if ($GLOBALS['perm']->have_perm('root', $user_id)) {
7168
$roles[] = 'ROLE_ADMIN';
69+
}
70+
71+
// Admin users have permissions on videos of all administrated courses
72+
else if ($GLOBALS['perm']->have_perm('admin', $user_id)) {
73+
74+
$sem_user = new \Seminar_User($user_id);
75+
76+
$nobody = $GLOBALS['user'];
77+
$GLOBALS['user'] = $sem_user;
78+
79+
$filter = \AdminCourseFilter::get();
80+
$courses = array_column($filter->getCourses(), 'seminar_id');
81+
82+
$GLOBALS['user'] = $nobody;
83+
84+
foreach ($courses as $course_id) {
85+
$roles[$course_id . '_Instructor'] = $course_id . '_Instructor';
86+
}
87+
7288
} else {
7389
// Handle video roles
7490

@@ -77,9 +93,9 @@ public function __invoke(Request $request, Response $response, $args)
7793
if (!$vperm->video->episode) continue;
7894

7995
if ($vperm->perm == 'owner' || $vperm->perm == 'write') {
80-
$roles[$vperm->video->episode . '_write'] = 'STUDIP_' . $vperm->video->episode . '_write';
96+
$roles[$vperm->video->episode . '_write'] = $vperm->video->episode . '_write';
8197
} else {
82-
$roles[$vperm->video->episode . '_read'] = 'STUDIP_' . $vperm->video->episode . '_read';
98+
$roles[$vperm->video->episode . '_read'] = $vperm->video->episode . '_read';
8399
}
84100
}
85101

@@ -89,9 +105,15 @@ public function __invoke(Request $request, Response $response, $args)
89105
$stmt->execute([$user_id]);
90106
$courses_write = $stmt->fetchAll(\PDO::FETCH_COLUMN);
91107

108+
// Handle deputies ("Dozentenvertretung") as well
109+
$courses_write = array_merge(
110+
$courses_write,
111+
array_column(\Deputy::findDeputyCourses($user_id)->toArray(), 'range_id')
112+
);
113+
92114
// add instructor roles
93115
foreach ($courses_write as $course_id) {
94-
$roles[$course_id . '_Instructor'] = 'STUDIP_' . $course_id . '_Instructor';
116+
$roles[$course_id . '_Instructor'] = $course_id . '_Instructor';
95117
}
96118

97119
// Get courses with read access ('autor', 'user')
@@ -102,17 +124,17 @@ public function __invoke(Request $request, Response $response, $args)
102124

103125
// add learner roles
104126
foreach ($courses_read as $course_id) {
105-
$roles[$course_id . '_Learner'] = 'STUDIP_' . $course_id . '_Learner';
127+
$roles[$course_id . '_Learner'] = $course_id . '_Learner';
106128
}
107129

108130
// Handle playlist roles
109131

110132
// get all playlists the user has permissions on
111133
foreach (PlaylistsUserPerms::findByUser_id($user_id) as $pperm) {
112134
if ($pperm->perm == 'owner' || $pperm->perm == 'write') {
113-
$roles[$pperm->playlist->service_playlist_id . '_write'] = 'STUDIP_PLAYLIST_' . $pperm->playlist->service_playlist_id . '_write';
135+
$roles[$pperm->playlist->service_playlist_id . '_write'] = 'PLAYLIST_' . $pperm->playlist->service_playlist_id . '_write';
114136
} else {
115-
$roles[$pperm->playlist->service_playlist_id . '_read'] = 'STUDIP_PLAYLIST_' . $pperm->playlist->service_playlist_id . '_read';
137+
$roles[$pperm->playlist->service_playlist_id . '_read'] = 'PLAYLIST_' . $pperm->playlist->service_playlist_id . '_read';
116138
}
117139
}
118140

@@ -125,7 +147,7 @@ public function __invoke(Request $request, Response $response, $args)
125147
$stmt->execute();
126148

127149
foreach ($stmt->fetchAll(\PDO::FETCH_COLUMN) as $service_playlist_id) {
128-
$roles[$service_playlist_id . '_write'] = 'STUDIP_PLAYLIST_' . $service_playlist_id . '_write';
150+
$roles[$service_playlist_id . '_write'] = 'PLAYLIST_' . $service_playlist_id . '_write';
129151
}
130152

131153
// find playlists with read access
@@ -139,7 +161,7 @@ public function __invoke(Request $request, Response $response, $args)
139161

140162
foreach ($stmt->fetchAll(\PDO::FETCH_COLUMN) as $service_playlist_id) {
141163
// All seminar members have read permission on visible playlists
142-
$roles[$service_playlist_id . '_read'] = 'STUDIP_PLAYLIST_' . $service_playlist_id . '_read';
164+
$roles[$service_playlist_id . '_read'] = 'PLAYLIST_' . $service_playlist_id . '_read';
143165
}
144166
}
145167
} else {

0 commit comments

Comments
 (0)