From aeab1a3ae2fea464a21f6e1ad72171d5283b5679 Mon Sep 17 00:00:00 2001 From: Ole Wieners Date: Fri, 1 Nov 2024 13:05:08 +0100 Subject: [PATCH 1/8] Remove some leading/trailing whitespace Doing this whenever I spot some. --- backend/src/api/model/event.rs | 2 +- backend/src/api/model/realm/mod.rs | 2 +- backend/src/api/model/series.rs | 2 +- backend/src/api/mutation.rs | 2 +- backend/src/config/general.rs | 10 +++++----- docs/docs/setup/config.toml | 10 +++++----- frontend/src/i18n/locales/de.yaml | 5 +++-- frontend/src/i18n/locales/en.yaml | 4 ++-- frontend/src/routes/Embed.tsx | 2 +- 9 files changed, 20 insertions(+), 19 deletions(-) diff --git a/backend/src/api/model/event.rs b/backend/src/api/model/event.rs index a69487d90..697d5a692 100644 --- a/backend/src/api/model/event.rs +++ b/backend/src/api/model/event.rs @@ -339,7 +339,7 @@ impl AuthorizedEvent { pub(crate) async fn delete(id: Id, context: &Context) -> ApiResult { let event = Self::load_by_id(id, context) - .await? + .await? .ok_or_else(|| err::invalid_input!( key = "event.delete.not-found", "event not found", diff --git a/backend/src/api/model/realm/mod.rs b/backend/src/api/model/realm/mod.rs index 6d2788769..3455fc284 100644 --- a/backend/src/api/model/realm/mod.rs +++ b/backend/src/api/model/realm/mod.rs @@ -381,7 +381,7 @@ impl Realm { self.owner_display_name.as_deref() } - /// Returns the acl of this realm, combining moderator and admin roles and assigns + /// Returns the acl of this realm, combining moderator and admin roles and assigns /// the respective actions that are necessary for UI purposes. async fn own_acl(&self, context: &Context) -> ApiResult { let raw_roles_sql = " diff --git a/backend/src/api/model/series.rs b/backend/src/api/model/series.rs index 491412c05..aa3d8c23f 100644 --- a/backend/src/api/model/series.rs +++ b/backend/src/api/model/series.rs @@ -182,7 +182,7 @@ impl Series { return Ok(RemoveMountedSeriesOutcome::RemovedRealm(removed_realm)); } - + if old_realm.name_from_block.map(Id::block) == Some(blocks[0].id()) { // The realm has its name derived from the series block that is being removed - so the name // shouldn't be used anymore. Ideally this would restore the previous title, diff --git a/backend/src/api/mutation.rs b/backend/src/api/mutation.rs index 40c15f9b1..1a99cf9c8 100644 --- a/backend/src/api/mutation.rs +++ b/backend/src/api/mutation.rs @@ -58,7 +58,7 @@ impl Mutation { /// Deletes the given event. Meaning: a deletion request is sent to Opencast, the event /// is marked as "deletion pending" in Tobira, and fully removed once Opencast /// finished deleting the event. - /// + /// /// Returns the deletion timestamp in case of success and errors otherwise. /// Note that "success" in this case only means the request was successfully sent /// and accepted, not that the deletion itself succeeded, which is instead checked diff --git a/backend/src/config/general.rs b/backend/src/config/general.rs index b10fa2613..d3267b620 100644 --- a/backend/src/config/general.rs +++ b/backend/src/config/general.rs @@ -12,7 +12,7 @@ pub(crate) struct GeneralConfig { /// Public URL to Tobira (without path). /// Used for RSS feeds, as those require specifying absolute URLs to resources. - /// + /// /// Example: "https://tobira.my-uni.edu". pub(crate) tobira_url: HttpHost, @@ -22,12 +22,12 @@ pub(crate) struct GeneralConfig { /// These can be specified in multiple languages. /// Consent is prompted upon first use and only if this is configured. It is /// re-prompted when any of these values change. - /// + /// /// We recommend not to configure this unless absolutely necessary, /// in order to not degrade the user experience needlessly. - /// + /// /// Example: - /// + /// /// ``` /// initial_consent.title.en = "Terms & Conditions" /// initial_consent.button.en = "Agree" @@ -49,7 +49,7 @@ pub(crate) struct GeneralConfig { /// add custom ones. Note that these two default links are special and can /// be specified with only the shown string. To add custom ones, you need /// to define a label and a link. The link is either the same for every language - /// or can be specified for each language in the same manner as the label. + /// or can be specified for each language in the same manner as the label. /// Example: /// /// ``` diff --git a/docs/docs/setup/config.toml b/docs/docs/setup/config.toml index b78eae74d..5fba3e724 100644 --- a/docs/docs/setup/config.toml +++ b/docs/docs/setup/config.toml @@ -22,7 +22,7 @@ # Public URL to Tobira (without path). # Used for RSS feeds, as those require specifying absolute URLs to resources. -# +# # Example: "https://tobira.my-uni.edu". # # Required! This value must be specified. @@ -34,12 +34,12 @@ # These can be specified in multiple languages. # Consent is prompted upon first use and only if this is configured. It is # re-prompted when any of these values change. -# +# # We recommend not to configure this unless absolutely necessary, # in order to not degrade the user experience needlessly. -# +# # Example: -# +# # ``` # initial_consent.title.en = "Terms & Conditions" # initial_consent.button.en = "Agree" @@ -62,7 +62,7 @@ # add custom ones. Note that these two default links are special and can # be specified with only the shown string. To add custom ones, you need # to define a label and a link. The link is either the same for every language -# or can be specified for each language in the same manner as the label. +# or can be specified for each language in the same manner as the label. # Example: # # ``` diff --git a/frontend/src/i18n/locales/de.yaml b/frontend/src/i18n/locales/de.yaml index 401bc2b9b..a88ab8bc5 100644 --- a/frontend/src/i18n/locales/de.yaml +++ b/frontend/src/i18n/locales/de.yaml @@ -337,8 +337,9 @@ manage: yourself: Sie subset-warning: 'Diese Auswahl ist bereits in den folgenden Gruppen enthalten: {{groups}}.' inherited: 'Geerbte Berechtigungen:' - inherited-tooltip: > - Die folgenden Berechtigungen wurden auf einer übergeordneten Seite gewährt und werden auf alle Unterseiten vererbt. + inherited-tooltip: > + Die folgenden Berechtigungen wurden auf einer übergeordneten Seite gewährt und werden auf alle Unterseiten + vererbt. actions: title: Berechtigung read: Lesen diff --git a/frontend/src/i18n/locales/en.yaml b/frontend/src/i18n/locales/en.yaml index 3dd91daf6..da767027e 100644 --- a/frontend/src/i18n/locales/en.yaml +++ b/frontend/src/i18n/locales/en.yaml @@ -112,7 +112,7 @@ user: manage-content: Manage login-page: - heading: Login + heading: Login user-id: User ID password: Password bad-credentials: 'Login failed: invalid credentials.' @@ -161,7 +161,7 @@ video: title: Download presenter: Video (speaker) slides: Video (presentation) - info: > + info: > Here you can download the video(s) in different formats/qualities (Right click - Save as). manage: Manage start: Start diff --git a/frontend/src/routes/Embed.tsx b/frontend/src/routes/Embed.tsx index 79908e81b..43aa87c0c 100644 --- a/frontend/src/routes/Embed.tsx +++ b/frontend/src/routes/Embed.tsx @@ -33,7 +33,7 @@ export const EmbedVideoRoute = makeRoute({ query EmbedQuery($id: ID!) { event: eventById(id: $id) { ... EmbedEventData } } - `; + `; const queryRef = loadQuery(query, { id: eventId(videoId) }); From c5917a3fef90a42f0394b337be93634045318f4d Mon Sep 17 00:00:00 2001 From: Ole Wieners Date: Mon, 4 Nov 2024 17:43:36 +0100 Subject: [PATCH 2/8] Add endpoints to update acls in tobira (and OC) This adds a mutation that will send a PUT request to Opencast to update the acl of a given event. When the request responds with 204, it is considered successful and the updated acl is stored in Tobira's DB, without waiting for sync with OC. That allows us to pretend that things are happening much faster than they do in actuality, as Opencast needs to run the "republish metadata" workflow in order to propagate these changes back to Tobira. There were some discussions and efforts to change how that is done in Opencast, but the proposed changes didn't make the cut yet. Once that or sth similar happens, we should also adapt the mechanism implemented in this commit. --- backend/src/api/model/acl.rs | 10 +++- backend/src/api/model/event.rs | 86 ++++++++++++++++++++++++++++--- backend/src/api/mutation.rs | 10 ++++ backend/src/sync/client.rs | 16 +++++- frontend/src/i18n/locales/de.yaml | 3 ++ frontend/src/i18n/locales/en.yaml | 3 ++ frontend/src/schema.graphql | 14 +++++ 7 files changed, 132 insertions(+), 10 deletions(-) diff --git a/backend/src/api/model/acl.rs b/backend/src/api/model/acl.rs index 0c6607422..409bd61e3 100644 --- a/backend/src/api/model/acl.rs +++ b/backend/src/api/model/acl.rs @@ -1,5 +1,6 @@ -use juniper::GraphQLObject; +use juniper::{GraphQLInputObject, GraphQLObject}; use postgres_types::BorrowToSql; +use serde::Serialize; use crate::{api::{util::TranslatedString, Context, err::ApiResult}, db::util::select}; @@ -94,3 +95,10 @@ where } }).await.map_err(Into::into) } + +#[derive(Debug, GraphQLInputObject, Serialize)] +pub(crate) struct AclInput { + pub allow: bool, + pub action: String, + pub role: String, +} diff --git a/backend/src/api/model/event.rs b/backend/src/api/model/event.rs index 697d5a692..0f9c9108a 100644 --- a/backend/src/api/model/event.rs +++ b/backend/src/api/model/event.rs @@ -19,6 +19,8 @@ use crate::{ prelude::*, }; +use self::{acl::AclInput, err::ApiError}; + use super::playlist::VideoListEntry; @@ -337,21 +339,37 @@ impl AuthorizedEvent { .pipe(Ok) } - pub(crate) async fn delete(id: Id, context: &Context) -> ApiResult { + async fn load_for_api( + id: Id, + context: &Context, + not_found_error: ApiError, + not_authorized_error: ApiError, + ) -> ApiResult { let event = Self::load_by_id(id, context) .await? - .ok_or_else(|| err::invalid_input!( - key = "event.delete.not-found", - "event not found", - ))? + .ok_or_else(|| not_found_error)? .into_result()?; if !context.auth.overlaps_roles(&event.write_roles) { - return Err(err::not_authorized!( + return Err(not_authorized_error); + } + + Ok(event) + } + + pub(crate) async fn delete(id: Id, context: &Context) -> ApiResult { + let event = Self::load_for_api( + id, + context, + err::invalid_input!( + key = "event.delete.not-found", + "event not found" + ), + err::not_authorized!( key = "event.delete.not-allowed", "you are not allowed to delete this event", - )); - } + ) + ).await?; let response = context .oc_client @@ -381,6 +399,58 @@ impl AuthorizedEvent { } } + pub(crate) async fn update_acl(id: Id, acl: Vec, context: &Context) -> ApiResult { + let event = Self::load_for_api( + id, + context, + err::invalid_input!( + key = "event.acl.not-found", + "event not found", + ), + err::not_authorized!( + key = "event.acl.not-allowed", + "you are not allowed to update this event's acl", + ) + ).await?; + + let response = context + .oc_client + .update_event_acl(&event.opencast_id, &acl) + .await + .map_err(|e| { + error!("Failed to send acl update request: {}", e); + err::opencast_unavailable!("Failed to communicate with Opencast") + })?; + + if response.status() == StatusCode::NO_CONTENT { + // 204: The access control list for the specified event is updated. + let roles_for_action = |target_action: &str| -> Vec { + acl.iter() + .filter(|entry| entry.allow && entry.action == target_action) + .map(|entry| entry.role.clone()) + .collect() + }; + let read_roles = roles_for_action("read"); + let write_roles = roles_for_action("write"); + info!(event_id = %id, "Requested acl update of event"); + + // Todo: also update preview roles + context.db.execute("\ + update all_events \ + set read_roles = $2, write_roles = $3 \ + where id = $1 \ + ", &[&event.key, &read_roles, &write_roles]).await?; + Ok(event) + } else { + warn!( + event_id = %id, + "Failed to update event acl, OC returned status: {}", + response.status() + ); + Err(err::opencast_unavailable!("Opencast API error: {}", response.status())) + } + } + pub(crate) async fn load_writable_for_user( context: &Context, order: EventSortOrder, diff --git a/backend/src/api/mutation.rs b/backend/src/api/mutation.rs index 1a99cf9c8..7224cd409 100644 --- a/backend/src/api/mutation.rs +++ b/backend/src/api/mutation.rs @@ -6,6 +6,7 @@ use super::{ err::ApiResult, id::Id, model::{ + acl::AclInput, series::{Series, NewSeries}, realm::{ ChildIndex, @@ -67,6 +68,15 @@ impl Mutation { AuthorizedEvent::delete(id, context).await } + /// Updates the acl of a given event by sending a PUT request to Opencast. If the request is + /// successful (as indicated by the response code 204), the updated acl is already stored in Tobira + /// without waiting for an upcoming sync - however this means it might get overwritten again if + /// the update in Opencast failed for some reason. + /// This solution should be improved in the future. + async fn update_event_acl(id: Id, acl: Vec, context: &Context) -> ApiResult { + AuthorizedEvent::update_acl(id, acl, context).await + } + /// Sets the order of all children of a specific realm. /// /// `childIndices` must contain at least one element, i.e. do not call this diff --git a/backend/src/sync/client.rs b/backend/src/sync/client.rs index 771a8d251..099572d24 100644 --- a/backend/src/sync/client.rs +++ b/backend/src/sync/client.rs @@ -14,6 +14,7 @@ use serde::Deserialize; use tap::TapFallible; use crate::{ + api::model::acl::AclInput, config::{Config, HttpHost}, prelude::*, sync::harvest::HarvestResponse, @@ -140,7 +141,7 @@ impl OcClient { } pub async fn delete_event(&self, oc_id: &String) -> Result> { - let pq = format!("/api/events/{}", oc_id); + let pq = format!("/api/events/{oc_id}"); let req = self.authed_req_builder(&self.external_api_node, &pq) .method(http::Method::DELETE) .body(RequestBody::empty()) @@ -149,6 +150,19 @@ impl OcClient { self.http_client.request(req).await.map_err(Into::into) } + pub async fn update_event_acl(&self, oc_id: &String, acl: &Vec) -> Result> { + let pq = format!("/api/events/{oc_id}/acl"); + let req = self.authed_req_builder(&self.external_api_node, &pq) + .method(http::Method::PUT) + .header(http::header::CONTENT_TYPE, "application/x-www-form-urlencoded") + .body( + format!("acl={}", serde_json::to_string(&acl).expect("Failed to serialize")).into() + ) + .expect("failed to build request"); + + self.http_client.request(req).await.map_err(Into::into) + } + fn build_authed_req(&self, node: &HttpHost, path_and_query: &str) -> (Uri, Request) { let req = self.authed_req_builder(node, path_and_query) .body(RequestBody::empty()) diff --git a/frontend/src/i18n/locales/de.yaml b/frontend/src/i18n/locales/de.yaml index a88ab8bc5..a9bbacba5 100644 --- a/frontend/src/i18n/locales/de.yaml +++ b/frontend/src/i18n/locales/de.yaml @@ -649,6 +649,9 @@ api-remote-errors: delete: not-found: Das zu löschende Video existiert nicht. Möglicherweise wurde es bereits entfernt. not-allowed: Sie haben nicht die Berechtigung, dieses Video zu löschen. + acl: + not-found: "Zugriffsrechte konnten nicht geändert werden: Video nicht gefunden." + not-allowed: Sie haben nicht die Berechtigung, die Zugriffsreche dieses Videos zu ändern. embed: not-supported: Diese Seite kann nicht eingebettet werden. diff --git a/frontend/src/i18n/locales/en.yaml b/frontend/src/i18n/locales/en.yaml index da767027e..ef75cd4f7 100644 --- a/frontend/src/i18n/locales/en.yaml +++ b/frontend/src/i18n/locales/en.yaml @@ -623,6 +623,9 @@ api-remote-errors: The video you are trying to delete does not exist. It might have been removed already. not-allowed: You are not allowed to delete this video. + acl: + not-found: "Access policy update failed: video not found." + not-allowed: You are not allowed to update the access policies of this video. embed: not-supported: This page can't be embedded. diff --git a/frontend/src/schema.graphql b/frontend/src/schema.graphql index e9abdd11e..723a1e399 100644 --- a/frontend/src/schema.graphql +++ b/frontend/src/schema.graphql @@ -12,6 +12,12 @@ type EventSearchResults { duration: Int! } +input AclInput { + allow: Boolean! + action: String! + role: String! +} + input NewTitleBlock { content: String! } @@ -541,6 +547,14 @@ type Mutation { in subsequent harvesting results. """ deleteVideo(id: ID!): RemovedEvent! + """ + Updates the acl of a given event by sending a PUT request to Opencast. If the request is + successful (as indicated by the response code 204), the updated acl is already stored in Tobira + without waiting for an upcoming sync - however this means it might get overwritten again if + the update in Opencast failed for some reason. + This solution should be improved in the future. + """ + updateEventAcl(id: ID!, acl: [AclInput!]!): AuthorizedEvent! """ Sets the order of all children of a specific realm. From 86045de796727cfcb2cecf454082a9729ab1f432 Mon Sep 17 00:00:00 2001 From: Ole Wieners Date: Tue, 5 Nov 2024 15:04:30 +0100 Subject: [PATCH 3/8] Adapt ACL editing UI for actual functionality This adds the necessary changes for the interface to commit the mutation added in the previous commit. Also fixes some minor UX issues. --- frontend/src/routes/manage/Video/Access.tsx | 62 ++++++++++++++++++--- frontend/src/ui/Access.tsx | 52 +++++++++-------- 2 files changed, 79 insertions(+), 35 deletions(-) diff --git a/frontend/src/routes/manage/Video/Access.tsx b/frontend/src/routes/manage/Video/Access.tsx index 97c8ade49..ae9e9abf9 100644 --- a/frontend/src/routes/manage/Video/Access.tsx +++ b/frontend/src/routes/manage/Video/Access.tsx @@ -1,8 +1,8 @@ import { useTranslation } from "react-i18next"; -import { WithTooltip } from "@opencast/appkit"; +import { boxError, currentRef, WithTooltip } from "@opencast/appkit"; import { useRef, useState } from "react"; import { LuInfo } from "react-icons/lu"; -import { useFragment } from "react-relay"; +import { graphql, useFragment, useMutation } from "react-relay"; import { Breadcrumbs } from "../../../ui/Breadcrumbs"; import { AuthorizedEvent, makeManageVideoRoute } from "./Shared"; @@ -20,6 +20,8 @@ import { ManageVideosRoute } from "."; import { ManageVideoDetailsRoute } from "./Details"; import { READ_WRITE_ACTIONS } from "../../../util/permissionLevels"; import { ConfirmationModalHandle } from "../../../ui/Modal"; +import { displayCommitError } from "../Realm/util"; +import { AccessUpdateAclMutation } from "./__generated__/AccessUpdateAclMutation.graphql"; export const ManageVideoAccessRoute = makeManageVideoRoute( @@ -83,6 +85,16 @@ const UnlistedNote: React.FC = () => { ); }; +const updateVideoAcl = graphql` + mutation AccessUpdateAclMutation($id: ID!, $acl: [AclInput!]!) { + updateEventAcl(id: $id, acl: $acl) { + ...on AuthorizedEvent { + acl { role actions info { label implies large } } + } + } + } +`; + type AccessUIProps = { event: AuthorizedEvent; knownRoles: AccessKnownRolesData$data; @@ -90,6 +102,8 @@ type AccessUIProps = { const AccessUI: React.FC = ({ event, knownRoles }) => { const saveModalRef = useRef(null); + const [commitError, setCommitError] = useState(null); + const [commit, inFlight] = useMutation(updateVideoAcl); const initialAcl: Acl = new Map( event.acl.map(item => [item.role, { @@ -100,6 +114,35 @@ const AccessUI: React.FC = ({ event, knownRoles }) => { const [selections, setSelections] = useState(initialAcl); + const mapAclForMutation = (acl: Acl) => { + const result = []; + + for (const [role, { actions }] of acl) { + for (const action of actions) { + result.push({ + allow: true, + action, + role, + }); + } + } + + return result; + }; + + const onSubmit = async () => { + commit({ + variables: { + id: event.id, + acl: mapAclForMutation(selections), + }, + onCompleted: () => currentRef(saveModalRef).done(), + onError: error => setCommitError(displayCommitError(error)), + updater: store => store.invalidateStore(), + }); + }; + + return (
= ({ event, knownRoles }) => { permissionLevels={READ_WRITE_ACTIONS} /> { - // TODO: Actually save new ACL. - // eslint-disable-next-line no-console - console.log(acl); + {...{ + selections, + setSelections, + initialAcl, + inFlight, + saveModalRef, + onSubmit, }} + kind="write" /> + {boxError(commitError)}
); diff --git a/frontend/src/ui/Access.tsx b/frontend/src/ui/Access.tsx index e9a4e3e0d..acd1c2238 100644 --- a/frontend/src/ui/Access.tsx +++ b/frontend/src/ui/Access.tsx @@ -786,28 +786,26 @@ type AclEditButtonsProps = { saveModalRef: React.RefObject; } -export const AclEditButtons: React.FC = ( - { - selections, - setSelections, - initialAcl, - onSubmit, - className, - inFlight, - inheritedAcl, - userIsOwner, - saveModalRef, - kind, - } -) => { +export const AclEditButtons: React.FC = ({ + selections, + setSelections, + initialAcl, + onSubmit, + className, + inFlight, + inheritedAcl, + userIsOwner, + saveModalRef, + kind, +}) => { const { t } = useTranslation(); const user = useUser(); const resetModalRef = useRef(null); - const containsUser = (acl: Acl) => isRealUser(user) && (userIsOwner || user.roles.some(r => - r === COMMON_ROLES.ADMIN - || acl.get(r)?.actions.has(kind) - || inheritedAcl?.get(r)?.actions.has(kind)) + const containsUser = (acl: Acl) => isRealUser(user) && ( + userIsOwner || user.roles.some(r => r === COMMON_ROLES.ADMIN + || acl.get(r)?.actions.has(kind) + || inheritedAcl?.get(r)?.actions.has(kind)) ); const selectionIsInitial = selections.size === initialAcl.size @@ -817,8 +815,9 @@ export const AclEditButtons: React.FC = ( }); const submit = onSubmit; + const buttonsDisabled = inFlight || selectionIsInitial; - useNavBlocker(!selectionIsInitial); + useNavBlocker(!buttonsDisabled); return (
= ( }}> {/* Reset button */} @@ -856,13 +855,12 @@ export const AclEditButtons: React.FC = ( {/* Save button */} {inFlight &&
} Date: Tue, 5 Nov 2024 17:50:27 +0100 Subject: [PATCH 4/8] Add utility endpoint to start workflows in OC So far this is only needed for republishing metadata in case of acl updates. We might need this in other places, so I made a dedicated function for it. Of course we might also remove it again, if future Opencast changes make it redundant. We'll see. --- backend/src/api/model/event.rs | 37 +++++++++++++++++++++++++++++++++- backend/src/sync/client.rs | 14 +++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/backend/src/api/model/event.rs b/backend/src/api/model/event.rs index 0f9c9108a..7f88cda0a 100644 --- a/backend/src/api/model/event.rs +++ b/backend/src/api/model/event.rs @@ -440,12 +440,47 @@ impl AuthorizedEvent { set read_roles = $2, write_roles = $3 \ where id = $1 \ ", &[&event.key, &read_roles, &write_roles]).await?; + + Self::start_workflow(&event.opencast_id, "republish-metadata", &context).await?; Ok(event) } else { warn!( event_id = %id, "Failed to update event acl, OC returned status: {}", - response.status() + response.status(), + ); + Err(err::opencast_unavailable!("Opencast API error: {}", response.status())) + } + } + + /// Starts a workflow on the event. + async fn start_workflow(oc_id: &String, workflow_id: &str, context: &Context) -> ApiResult { + let response = context + .oc_client + .start_workflow(&oc_id, "republish-metadata") + .await + .map_err(|e| { + error!("Failed sending request to start workflow: {}", e); + err::opencast_unavailable!("Failed to communicate with Opencast") + })?; + + if response.status() == StatusCode::CREATED { + // 201: A new workflow is created. + info!(workflow = %workflow_id, "Requested creation of workflow"); + Ok(response.status()) + } else if response.status() == StatusCode::NOT_FOUND { + // 404: The specified workflow instance does not exist. + warn!( + workflow = %workflow_id, + "{}: The specified workflow instance does not exist.", + response.status(), + ); + Err(err::opencast_unavailable!("Opencast API error: {}", response.status())) + } else { + warn!( + workflow = %workflow_id, + "Failed to create workflow, OC returned status: {}", + response.status(), ); Err(err::opencast_unavailable!("Opencast API error: {}", response.status())) } diff --git a/backend/src/sync/client.rs b/backend/src/sync/client.rs index 099572d24..51791a12f 100644 --- a/backend/src/sync/client.rs +++ b/backend/src/sync/client.rs @@ -163,6 +163,20 @@ impl OcClient { self.http_client.request(req).await.map_err(Into::into) } + pub async fn start_workflow(&self, oc_id: &String, workflow_id: &str) -> Result> { + let params = format!("\ + event_identifier={oc_id}\ + &workflow_definition_identifier={workflow_id}\ + "); + let req = self.authed_req_builder(&self.external_api_node, "/api/workflows") + .method(http::Method::POST) + .header(http::header::CONTENT_TYPE, "application/x-www-form-urlencoded") + .body(params.into()) + .expect("failed to build request"); + + self.http_client.request(req).await.map_err(Into::into) + } + fn build_authed_req(&self, node: &HttpHost, path_and_query: &str) -> (Uri, Request) { let req = self.authed_req_builder(node, path_and_query) .body(RequestBody::empty()) From 79c92e35d186496cdf0bc883e0b6b1e657d80eaf Mon Sep 17 00:00:00 2001 From: Ole Wieners Date: Tue, 5 Nov 2024 18:22:58 +0100 Subject: [PATCH 5/8] Make the option of ACL editing configurable Not sure if the default should be `true` or `false` for this. --- backend/src/config/general.rs | 12 ++++++++++++ backend/src/http/assets.rs | 1 + docs/docs/setup/config.toml | 13 +++++++++++++ frontend/src/config.ts | 1 + frontend/src/routes/manage/Video/Shared.tsx | 5 +++-- 5 files changed, 30 insertions(+), 2 deletions(-) diff --git a/backend/src/config/general.rs b/backend/src/config/general.rs index d3267b620..5c5a0b4a1 100644 --- a/backend/src/config/general.rs +++ b/backend/src/config/general.rs @@ -110,6 +110,18 @@ pub(crate) struct GeneralConfig { /// (partial) name. #[config(default = false)] pub users_searchable: bool, + + /// This allows users to edit the ACL of events they have write access for. + /// Doing so will update these in Opencast and start the `republish-metadata` + /// workflow to propagate the changes to other publications as well. + /// Instead of waiting for the workflow however, Tobira will also immediately + /// store the updated ACL in its database. + /// + /// Note that this might lead to situations where the event ACL in Tobira is different + /// from that in other publications, mainly if the afore mentioned workflow fails + /// or takes an unusually long time to complete. + #[config(default = true)] + pub allow_acl_edit: bool, } const INTERNAL_RESERVED_PATHS: &[&str] = &["favicon.ico", "robots.txt", ".well-known"]; diff --git a/backend/src/http/assets.rs b/backend/src/http/assets.rs index fcd168e8b..a46fe4f5a 100644 --- a/backend/src/http/assets.rs +++ b/backend/src/http/assets.rs @@ -278,6 +278,7 @@ fn frontend_config(config: &Config) -> serde_json::Value { "initialConsent": config.general.initial_consent, "showDownloadButton": config.general.show_download_button, "usersSearchable": config.general.users_searchable, + "allowAclEdit": config.general.allow_acl_edit, "footerLinks": config.general.footer_links, "metadataLabels": config.general.metadata, "paellaPluginConfig": config.player.paella_plugin_config, diff --git a/docs/docs/setup/config.toml b/docs/docs/setup/config.toml index 5fba3e724..ceb3e3414 100644 --- a/docs/docs/setup/config.toml +++ b/docs/docs/setup/config.toml @@ -123,6 +123,19 @@ # Default value: false #users_searchable = false +# This allows users to edit the ACL of events they have write access for. +# Doing so will update these in Opencast and start the `republish-metadata` +# workflow to propagate the changes to other publications as well. +# Instead of waiting for the workflow however, Tobira will also immediately +# store the updated ACL in its database. +# +# Note that this might lead to situations where the event ACL in Tobira is different +# from that in other publications, mainly if the afore mentioned workflow fails +# or takes an unusually long time to complete. +# +# Default value: true +#allow_acl_edit = true + [db] # The username of the database user. diff --git a/frontend/src/config.ts b/frontend/src/config.ts index 978c0078d..4fba06ea0 100644 --- a/frontend/src/config.ts +++ b/frontend/src/config.ts @@ -27,6 +27,7 @@ type Config = { initialConsent: InitialConsent | null; showDownloadButton: boolean; usersSearchable: boolean; + allowAclEdit: boolean; opencast: OpencastConfig; footerLinks: FooterLink[]; metadataLabels: Record>; diff --git a/frontend/src/routes/manage/Video/Shared.tsx b/frontend/src/routes/manage/Video/Shared.tsx index 7f9f13ac9..8199eb7fd 100644 --- a/frontend/src/routes/manage/Video/Shared.tsx +++ b/frontend/src/routes/manage/Video/Shared.tsx @@ -12,9 +12,10 @@ import { b64regex } from "../../util"; import { Thumbnail } from "../../../ui/Video"; import { SharedVideoManageQuery } from "./__generated__/SharedVideoManageQuery.graphql"; import { Link } from "../../../router"; -import { eventId, isExperimentalFlagSet, keyOfId } from "../../../util"; +import { eventId, keyOfId } from "../../../util"; import { DirectVideoRoute, VideoRoute } from "../../Video"; import { ManageVideosRoute } from "."; +import CONFIG from "../../../config"; export const PAGE_WIDTH = 1100; @@ -151,7 +152,7 @@ const ManageVideoNav: React.FC = ({ event, active }) => { }, ]; - if (isExperimentalFlagSet()) { + if (CONFIG.allowAclEdit) { entries.splice(1, 0, { url: `/~manage/videos/${id}/access`, page: "acl", From beead64fac038752f1f3671252f3f8bfc69d4c25 Mon Sep 17 00:00:00 2001 From: Ole Wieners Date: Mon, 11 Nov 2024 21:21:08 +0100 Subject: [PATCH 6/8] Add endpoint to check workflow activity This endpoint allows us to check whether there are any workflows running in Opencast for an event. Also added are error handling and blocking of acl editing based on that check. --- backend/src/api/model/event.rs | 32 ++++++++++++ backend/src/sync/client.rs | 15 ++++++ frontend/src/i18n/locales/de.yaml | 7 +++ frontend/src/i18n/locales/en.yaml | 7 +++ frontend/src/routes/manage/Video/Access.tsx | 56 +++++++++++++-------- frontend/src/routes/manage/Video/Shared.tsx | 9 +++- frontend/src/schema.graphql | 2 + 7 files changed, 104 insertions(+), 24 deletions(-) diff --git a/backend/src/api/model/event.rs b/backend/src/api/model/event.rs index 7f88cda0a..0ba180dba 100644 --- a/backend/src/api/model/event.rs +++ b/backend/src/api/model/event.rs @@ -214,6 +214,11 @@ impl AuthorizedEvent { &self.tobira_deletion_timestamp } + /// Whether the event has active workflows. + async fn has_active_workflows(&self, context: &Context) -> ApiResult { + Self::has_active_workflows(&self, context).await + } + async fn series(&self, context: &Context) -> ApiResult> { if let Some(series) = self.series { Ok(Series::load_by_key(series, context).await?) @@ -399,6 +404,26 @@ impl AuthorizedEvent { } } + async fn has_active_workflows(&self, context: &Context) -> ApiResult { + if !context.auth.overlaps_roles(&self.write_roles) { + return Err(err::not_authorized!( + key = "event.workflow.not-allowed", + "you are not allowed to inquire about this event's workflow activity", + )); + } + + let response = context + .oc_client + .has_active_workflows(&self.opencast_id) + .await + .map_err(|e| { + error!("Failed to get workflow activity: {}", e); + err::opencast_unavailable!("Failed to communicate with Opencast") + })?; + + Ok(response) + } + pub(crate) async fn update_acl(id: Id, acl: Vec, context: &Context) -> ApiResult { let event = Self::load_for_api( id, @@ -413,6 +438,13 @@ impl AuthorizedEvent { ) ).await?; + if Self::has_active_workflows(&event, context).await? { + return Err(err::not_authorized!( + key = "event.workflow.active", + "acl change blocked by another workflow", + )); + } + let response = context .oc_client .update_event_acl(&event.opencast_id, &acl) diff --git a/backend/src/sync/client.rs b/backend/src/sync/client.rs index 51791a12f..e2e670b99 100644 --- a/backend/src/sync/client.rs +++ b/backend/src/sync/client.rs @@ -177,6 +177,21 @@ impl OcClient { self.http_client.request(req).await.map_err(Into::into) } + pub async fn has_active_workflows(&self, oc_id: &String) -> Result { + let pq = format!("/workflow/mediaPackage/{oc_id}/hasActiveWorkflows"); + let req = self.authed_req_builder(&self.external_api_node, &pq) + .header(http::header::CONTENT_TYPE, "application/x-www-form-urlencoded") + .body(RequestBody::empty()) + .expect("failed to build request"); + let uri = req.uri().clone(); + let response = self.http_client.request(req) + .await + .with_context(|| format!("HTTP request failed (uri: '{uri}')"))?; + + let (out, _) = self.deserialize_response(response, &uri).await?; + Ok(out) + } + fn build_authed_req(&self, node: &HttpHost, path_and_query: &str) -> (Uri, Request) { let req = self.authed_req_builder(node, path_and_query) .body(RequestBody::empty()) diff --git a/frontend/src/i18n/locales/de.yaml b/frontend/src/i18n/locales/de.yaml index a9bbacba5..9eea0e864 100644 --- a/frontend/src/i18n/locales/de.yaml +++ b/frontend/src/i18n/locales/de.yaml @@ -322,6 +322,10 @@ manage: access: authorized-groups: Autorisierte Gruppen authorized-users: Autorisierte Personen + workflow-active: > + Änderung der Berechtigungen ist zurzeit nicht möglich, da das Video im Hintergrund verarbeitet wird. +
+ Bitte versuchen Sie es später nochmal. Um es jetzt nochmal zu versuchen, laden Sie bitte diese Seite neu. users-no-options: initial-searchable: Nach Name suchen oder exakten Nutzernamen/exakte E-Mail angeben none-found-searchable: Keine Personen gefunden @@ -652,6 +656,9 @@ api-remote-errors: acl: not-found: "Zugriffsrechte konnten nicht geändert werden: Video nicht gefunden." not-allowed: Sie haben nicht die Berechtigung, die Zugriffsreche dieses Videos zu ändern. + workflow: + not-allowed: Sie haben nicht die Berechtigung, die Workflowaktivität für dieses Video abzufragen. + active: $t(manage.access.workflow-active) embed: not-supported: Diese Seite kann nicht eingebettet werden. diff --git a/frontend/src/i18n/locales/en.yaml b/frontend/src/i18n/locales/en.yaml index ef75cd4f7..86799dca3 100644 --- a/frontend/src/i18n/locales/en.yaml +++ b/frontend/src/i18n/locales/en.yaml @@ -318,6 +318,10 @@ manage: access: authorized-groups: Authorized groups authorized-users: Authorized users + workflow-active: > + Changing the access policy is not possible at this time, since the video is being processed in the background. +
+ Please try again later. To try again now, please reload the page. users-no-options: initial-searchable: Type to search for users by name (or enter exact email/username) none-found-searchable: No user found @@ -626,6 +630,9 @@ api-remote-errors: acl: not-found: "Access policy update failed: video not found." not-allowed: You are not allowed to update the access policies of this video. + workflow: + not-allowed: You are not allowed to inquire about workflow activity of this video. + active: $t(manage.access.workflow-active) embed: not-supported: This page can't be embedded. diff --git a/frontend/src/routes/manage/Video/Access.tsx b/frontend/src/routes/manage/Video/Access.tsx index ae9e9abf9..05ee7d672 100644 --- a/frontend/src/routes/manage/Video/Access.tsx +++ b/frontend/src/routes/manage/Video/Access.tsx @@ -1,5 +1,5 @@ -import { useTranslation } from "react-i18next"; -import { boxError, currentRef, WithTooltip } from "@opencast/appkit"; +import { Trans, useTranslation } from "react-i18next"; +import { boxError, Card, currentRef, WithTooltip } from "@opencast/appkit"; import { useRef, useState } from "react"; import { LuInfo } from "react-icons/lu"; import { graphql, useFragment, useMutation } from "react-relay"; @@ -28,6 +28,7 @@ export const ManageVideoAccessRoute = makeManageVideoRoute( "acl", "/access", (event, data) => , + { onAccessRoute: true }, ); type AclPageProps = { @@ -104,6 +105,7 @@ const AccessUI: React.FC = ({ event, knownRoles }) => { const saveModalRef = useRef(null); const [commitError, setCommitError] = useState(null); const [commit, inFlight] = useMutation(updateVideoAcl); + const [editingBlocked, setEditingBlocked] = useState(event.hasActiveWorkflows); const initialAcl: Acl = new Map( event.acl.map(item => [item.role, { @@ -137,39 +139,49 @@ const AccessUI: React.FC = ({ event, knownRoles }) => { acl: mapAclForMutation(selections), }, onCompleted: () => currentRef(saveModalRef).done(), - onError: error => setCommitError(displayCommitError(error)), + onError: error => { + setEditingBlocked(true); + setCommitError(displayCommitError(error)); + }, updater: store => store.invalidateStore(), }); }; - return ( + return <> + {event.hasActiveWorkflows && + + }
- - +
+ + +
{boxError(commitError)}
- ); + ; }; diff --git a/frontend/src/routes/manage/Video/Shared.tsx b/frontend/src/routes/manage/Video/Shared.tsx index 8199eb7fd..fdb014c8d 100644 --- a/frontend/src/routes/manage/Video/Shared.tsx +++ b/frontend/src/routes/manage/Video/Shared.tsx @@ -31,6 +31,7 @@ export const makeManageVideoRoute = ( page: ManageVideoSubPageType, path: string, render: (event: AuthorizedEvent, data: QueryResponse) => JSX.Element, + options?: { onAccessRoute?: boolean }, ): Route & { url: (args: { videoId: string }) => string } => ( makeRoute({ url: ({ videoId }: { videoId: string }) => `/~manage/videos/${keyOfId(videoId)}/${path}`, @@ -42,7 +43,10 @@ export const makeManageVideoRoute = ( } const videoId = decodeURIComponent(params[1]); - const queryRef = loadQuery(query, { id: eventId(videoId) }); + const queryRef = loadQuery(query, { + id: eventId(videoId), + onAccessRoute: options?.onAccessRoute ?? false, + }); return { render: () => Date: Mon, 11 Nov 2024 21:42:58 +0100 Subject: [PATCH 7/8] Change error display to allow more html tags Previously this used the good old i18next t function, but that doesn't correctly display some html tags like `
` when used inside the translation strings. This changes the component to use i18next Trans component instead, with which the tags are displayed correctly. --- frontend/src/util/err.tsx | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/frontend/src/util/err.tsx b/frontend/src/util/err.tsx index 8e9cbfe91..6bd900f8c 100644 --- a/frontend/src/util/err.tsx +++ b/frontend/src/util/err.tsx @@ -1,7 +1,7 @@ import { i18n, ParseKeys } from "i18next"; import React from "react"; import { ReactNode } from "react"; -import { useTranslation } from "react-i18next"; +import { Trans, useTranslation } from "react-i18next"; import { match } from "@opencast/appkit"; import { APIError, NotJson, ServerError } from "../relay"; @@ -40,18 +40,16 @@ type ErrorDisplayInfo = { }; export const errorDisplayInfo = (error: unknown, i18n: i18n): ErrorDisplayInfo => { - const t = i18n.t.bind(i18n); - if (error instanceof NetworkError) { return { - causes: new Set([t("errors.network-error")]), + causes: new Set(["errors.network-error"]), probablyOurFault: false, potentiallyInternetProblem: true, }; } else if (error instanceof ServerError) { const cause = error.response.status >= 500 && error.response.status < 600 - ? t("errors.internal-server-error") - : t("errors.unexpected-server-error"); + ? "errors.internal-server-error" + : "errors.unexpected-server-error"; return { causes: new Set([cause]), @@ -60,7 +58,7 @@ export const errorDisplayInfo = (error: unknown, i18n: i18n): ErrorDisplayInfo = }; } else if (error instanceof NotJson) { return { - causes: new Set([t("errors.unexpected-response")]), + causes: new Set(["errors.unexpected-response"]), probablyOurFault: true, potentiallyInternetProblem: false, }; @@ -73,7 +71,7 @@ export const errorDisplayInfo = (error: unknown, i18n: i18n): ErrorDisplayInfo = // Use a message fitting to the exact error key, if it is present. const translationKey = err.key ? `api-remote-errors.${err.key}` : null; if (translationKey && i18n.exists(translationKey)) { - const msg = t(translationKey as ParseKeys); + const msg = translationKey as ParseKeys; causes.add(msg); continue; } @@ -92,18 +90,18 @@ export const errorDisplayInfo = (error: unknown, i18n: i18n): ErrorDisplayInfo = // careful and handle this case, too. if (!err.kind) { notOurFault = false; - causes.add(t("errors.unexpected-server-error")); + causes.add("errors.unexpected-server-error"); } else { const msg = match(err.kind, { INTERNAL_SERVER_ERROR: () => { notOurFault = false; - return t("errors.internal-server-error"); + return "errors.internal-server-error"; }, - INVALID_INPUT: () => t("errors.invalid-input"), - NOT_AUTHORIZED: () => t("errors.not-authorized"), + INVALID_INPUT: () => "errors.invalid-input", + NOT_AUTHORIZED: () => "errors.not-authorized", OPENCAST_UNAVAILABLE: () => { notOurFault = false; - return t("errors.opencast-unavailable"); + return "errors.opencast-unavailable"; }, }); causes.add(msg); @@ -114,7 +112,7 @@ export const errorDisplayInfo = (error: unknown, i18n: i18n): ErrorDisplayInfo = if (causes.size === 0) { // This should never happen? return { - causes: new Set([t("errors.unexpected-server-error")]), + causes: new Set(["errors.unexpected-server-error"]), probablyOurFault: true, potentiallyInternetProblem: false, }; @@ -127,7 +125,7 @@ export const errorDisplayInfo = (error: unknown, i18n: i18n): ErrorDisplayInfo = } } else { return { - causes: new Set([t("errors.unknown")]), + causes: new Set(["errors.unknown"]), probablyOurFault: true, potentiallyInternetProblem: false, }; @@ -150,8 +148,14 @@ export const ErrorDisplay: React.FC = ({ failedAction, ...pro

{failedAction && failedAction + " "} {causes.length === 1 - ? causes[0] + " " - :

    {causes.map(cause =>
  • {cause}
    • )}
+ // @ts-expect-error: Dynamically passed i18next keys need to be typed + // more strictly than just `string`. But I don't want to do that for all + // possible errors. + ? + :
    {causes.map(cause =>
  • + {/* @ts-expect-error: I don't wanna type all these errors. */} + +
    • )}
} {info.potentiallyInternetProblem && t("errors.are-you-connected-to-internet")}

From cce8143c552bdd43d1ef903933e4c930a88e39c1 Mon Sep 17 00:00:00 2001 From: Ole Wieners Date: Mon, 11 Nov 2024 21:56:00 +0100 Subject: [PATCH 8/8] Change acl icon and button colors --- frontend/src/routes/manage/Video/Shared.tsx | 4 ++-- frontend/src/ui/Access.tsx | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/frontend/src/routes/manage/Video/Shared.tsx b/frontend/src/routes/manage/Video/Shared.tsx index fdb014c8d..aa4f182b9 100644 --- a/frontend/src/routes/manage/Video/Shared.tsx +++ b/frontend/src/routes/manage/Video/Shared.tsx @@ -1,5 +1,5 @@ import { useTranslation } from "react-i18next"; -import { LuCornerLeftUp, LuInfo, LuShield, LuPlay, LuPenLine } from "react-icons/lu"; +import { LuCornerLeftUp, LuInfo, LuShieldCheck, LuPlay, LuPenLine } from "react-icons/lu"; import { graphql } from "react-relay"; import { RootLoader } from "../../../layout/Root"; @@ -161,7 +161,7 @@ const ManageVideoNav: React.FC = ({ event, active }) => { entries.splice(1, 0, { url: `/~manage/videos/${id}/access`, page: "acl", - body: <>{t("manage.my-videos.acl.title")}, + body: <>{t("manage.my-videos.acl.title")}, }); } diff --git a/frontend/src/ui/Access.tsx b/frontend/src/ui/Access.tsx index acd1c2238..1f346942c 100644 --- a/frontend/src/ui/Access.tsx +++ b/frontend/src/ui/Access.tsx @@ -828,6 +828,7 @@ export const AclEditButtons: React.FC = ({ }}> {/* Reset button */}