Skip to content

Commit

Permalink
Make sure all users always have ROLE_USER and ROLE_ANONYMOUS (#1116)
Browse files Browse the repository at this point in the history
These roles have a fixed meaning in Opencast and everything expects
users to have those roles. Until now, the auth integration had to make
sure that's true. But Tobira can just add those roles as well (which was
already partially done sometimes).

Technical a breaking change, but I don't expect any installation to have
a problem with this change.
  • Loading branch information
owi92 authored Feb 22, 2024
2 parents 7458127 + 2327843 commit 89ce517
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 10 deletions.
11 changes: 6 additions & 5 deletions backend/src/auth/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ use crate::{
db,
http::{self, Context, Request, Response, response::bad_request},
prelude::*,
config::OpencastConfig, auth::{config::LoginCredentialsHandler, ROLE_ANONYMOUS},
config::OpencastConfig,
auth::config::LoginCredentialsHandler,
};
use super::{config::SessionEndpointHandler, AuthSource, SessionId, User};

Expand Down Expand Up @@ -252,15 +253,15 @@ async fn check_opencast_login(
username: info.user.username,
display_name: info.user.name,
email: info.user.email,
// Sometimes, Opencast does not include `ROLE_ANONYMOUS` in the
// response, so we just add it here to be sure.
roles: info.roles.into_iter().chain([ROLE_ANONYMOUS.to_owned()]).collect(),
roles: info.roles.into_iter().collect(),
user_role: info.user_role,
}))
}

/// Creates a session for the given user and persists it in the DB.
async fn create_session(user: User, ctx: &Context) -> Result<Response, Response> {
async fn create_session(mut user: User, ctx: &Context) -> Result<Response, Response> {
user.add_default_roles();

// TODO: check if a user is already logged in? And remove that session then?

let db = db::get_conn_or_service_unavailable(&ctx.db_pool).await?;
Expand Down
24 changes: 19 additions & 5 deletions backend/src/auth/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ pub(crate) use self::{
pub(crate) const ROLE_ADMIN: &str = "ROLE_ADMIN";

const ROLE_ANONYMOUS: &str = "ROLE_ANONYMOUS";
const ROLE_USER: &str = "ROLE_USER";

const SESSION_COOKIE: &str = "tobira-session";

Expand Down Expand Up @@ -108,7 +109,7 @@ impl User {
db: &Client,
caches: &Caches,
) -> Result<Option<Self>, Response> {
let out = match &auth_config.source {
let mut out = match &auth_config.source {
AuthSource::None => None,
AuthSource::TobiraSession => Self::from_session(headers, db, auth_config).await?,
AuthSource::TrustAuthHeaders => Self::from_auth_headers(headers, auth_config),
Expand All @@ -117,7 +118,8 @@ impl User {
}
};

if let Some(user) = &out {
if let Some(user) = &mut out {
user.add_default_roles();
caches.user.upsert_user_info(user, db).await;
}

Expand Down Expand Up @@ -148,9 +150,10 @@ impl User {
let email = get_header(AUTH_HEADER_EMAIL);

// Get roles from the user.
let mut roles = HashSet::from([ROLE_ANONYMOUS.to_string()]);
let roles_raw = get_header(AUTH_HEADER_ROLES)?;
roles.extend(roles_raw.split(',').map(|role| role.trim().to_owned()));
let roles: HashSet<_> = get_header(AUTH_HEADER_ROLES)?
.split(',')
.map(|role| role.trim().to_owned())
.collect();
let user_role = auth_config
.find_user_role(&username, roles.iter().map(|s| s.as_str()))?
.to_owned();
Expand Down Expand Up @@ -360,6 +363,17 @@ impl User {

Ok(session_id)
}

/// Makes sure this user has the roles `ROLE_ANONYMOUS` and `ROLE_USER`.
fn add_default_roles(&mut self) {
// The conditionals are to prevent heap allocations when unneceesary.
if !self.roles.contains(ROLE_ANONYMOUS) {
self.roles.insert(ROLE_ANONYMOUS.into());
}
if !self.roles.contains(ROLE_USER) {
self.roles.insert(ROLE_USER.into());
}
}
}


Expand Down

0 comments on commit 89ce517

Please sign in to comment.