Skip to content

Commit

Permalink
Make auth system more flexible & convenient and add "callbacks" (#1032)
Browse files Browse the repository at this point in the history
This PR changes Tobira's auth system to be more flexible and hopefully
easier to understand, rewriting the auth docs in the process. It also
adds a new "callback" feature where Tobira can ask an HTTP endpoint for
auth information, instead of only getting them from "auth headers".


## Callbacks

The callback solution is preferred over auth headers now as HTTP headers
are usually limited in size, which is a problem for users with many
roles. It should also simplify setting up custom auth logic a lot.
Further, Tobira only calls the callback when necessary, preventing auth
logic to run uselessly, and allows you to remove special cases like
`/~assets` from your reverse proxy config. Finally, Tobira can cache
callback responses for some time to make everything even faster.

There are login callbacks and auth callbacks. The former gets login
credentials, the latter gets a forwarded HTTP request with headers. Both
need to return a JSON describing the authorization outcome. See the docs
for the details.

## Auth system changes

These are some breaking changes in term of configuration file, but all
previous "modes" are still supported. You just have to change your
configuration slightly. See "Migration" section.

Previously we had different "modes" in which Tobira could operate. These
modes would determine what Tobira would do in a small number of
different cases (e.g. when receiving login data, or an incoming request,
...). In a previous version, this PR added two more modes. But due to
additional requirements, we rethought everything and figured that we can
let admins configure the behavior in those cases individually. That way,
they can mix & match.

So instead of the `auth.mode` parameter, we have `auth.source` parameter
which controls what Tobira does for any normal incoming request that
needs authorization. Possible values are `none`, `tobira-session`,
`trust-auth-headers` and `callback:...`. When using `tobira-session`,
two additional behaviors can be configured, which control how Tobira's
sessions are created: `auth.session.from_login_credentials` and
`auth.session.from_session_endpoint`.

With this change, a lot more systems can be configured. Like using
Tobira session with Shibboleth.


### Migration

(this will be copied to the changelog of the next release)

You currently have `auth.mode = ...`

#### `"opencast"`

```toml
[auth]
source = "tobira-session"
session.from_login_credentials = "opencast"
```

#### `"login-proxy"`

```toml
[auth]
source = "tobira-session"
session.from_session_endpoint = "trust-auth-headers"
```

#### `"full-auth-proxy"`

```toml
[auth]
source = "trust-auth-headers"
```


## Other changes

Some other auth-related changes are part of this PR. Some other breaking
config changes were included:

- All role related config was moved into the section `auth.roles`.
- The configs to change the auth header names was removed.



-----

## How to review

Good question. 

The commits are atomic and you should certainly read the commit
messages. Reviewing commit by commit could be done: to understand the
history and skip some of the very refactor/moving code around commits.
But on the other hand, most of the code of the first commits has been
moved and rewritten later again.

It might make sense to read all the rendered auth docs first. And then
look at the changed files in the `auth` module in their current form,
and look at the remaining changes in diff view?

---

## Things that can be done in follow up PRs:

- Does the callback need the ability to redirect the user? I.e. reply `{
"outcome": "redirect", "location": "..." }` and Tobira then answers with
302?
- Make callbacks work with unix sockets
- Add ability to update user data of a Tobira session regularly somehow
- Always add `ROLE_ANONYMOUS` and `ROLE_USER` to users. 
- Maybe add some useful features to authkit? Though I really don't think
it's necessary. At most we can add a type declaration for the output
type.

Closes #666
  • Loading branch information
LukasKalbertodt authored Feb 22, 2024
2 parents a9d06eb + 5f6b4db commit 7829194
Show file tree
Hide file tree
Showing 34 changed files with 1,835 additions and 986 deletions.
3 changes: 2 additions & 1 deletion .deployment/templates/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ unix_socket = "/opt/tobira/{{ id }}/socket/tobira.sock"
unix_socket_permissions = 0o777

[auth]
mode = "login-proxy"
source = "tobira-session"
session.from_session_endpoint = "trust-auth-headers"
login_page.note.en = 'Dummy users: "jose", "morgan", "björk", "sabine" and "admin". Password for all: "tobira".'
login_page.note.de = 'Testnutzer: "jose", "morgan", "björk", "sabine" und "admin". Passwort für alle: "tobira".'

Expand Down
1 change: 1 addition & 0 deletions backend/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions backend/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ time = "0.3"
tokio = { version = "=1.28", features = ["fs", "rt-multi-thread", "macros", "time"] }
tokio-postgres = { version = "0.7", features = ["with-chrono-0_4", "with-serde_json-1"] }
tokio-postgres-rustls = "0.10.0"
url = "2.4.1"

[target.'cfg(target_os = "linux")'.dependencies]
procfs = "0.15.1"
Expand Down
166 changes: 160 additions & 6 deletions backend/src/auth/cache.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,83 @@
use std::{time::{Instant, Duration}, collections::HashSet};
use std::{collections::HashSet, hash::Hash, time::{Instant, Duration}};

use dashmap::{DashMap, mapref::entry::Entry};
use deadpool_postgres::Client;
use hyper::HeaderMap;
use prometheus_client::metrics::counter::Counter;

use crate::prelude::*;
use crate::{config::Config, prelude::*};

use super::{config::CallbackConfig, User};

pub struct Caches {
pub(crate) user: UserCache,
pub(crate) callback: AuthCallbackCache,
}

impl Caches {
pub fn new() -> Self {
Self {
user: UserCache::new(),
callback: AuthCallbackCache::new(),
}
}

/// Starts a daemon that regularly removes outdated entries from the cache.
pub(crate) async fn maintainence_task(&self, config: &Config) -> ! {
fn cleanup<K: Eq + Hash, V>(
now: Instant,
map: &DashMap<K, V>,
cache_duration: Duration,
mut timestamp: impl FnMut(&V) -> Instant,
) -> Option<Instant> {
let mut out = None;
map.retain(|_, v| {
let instant = timestamp(v);
let is_outdated = now.saturating_duration_since(instant) > cache_duration;
if !is_outdated {
out = match out {
None => Some(instant),
Some(existing) => Some(std::cmp::min(existing, instant)),
};
}
!is_outdated
});
out.map(|out| out + cache_duration)
}

let empty_wait_time = std::cmp::min(CACHE_DURATION, config.auth.callback.cache_duration);
tokio::time::sleep(empty_wait_time).await;

loop {
let now = Instant::now();
let next_user_action =
cleanup(now, &self.user.0, CACHE_DURATION, |v| v.last_written_to_db);
let next_callback_action =
cleanup(now, &self.callback.map, config.auth.callback.cache_duration, |v| v.timestamp);

// We will wait until the next entry in the hashmap gets stale, but
// at least 30s to not do cleanup too often. In case there are no
// entries currently, it will also retry in 30s. But we will wait
// at most as long as we would do for an empty cache.
let next_action = [next_user_action, next_callback_action].into_iter()
.filter_map(|x| x)
.min();
let wait_duration = std::cmp::min(
std::cmp::max(
next_action.map(|i| i.saturating_duration_since(now))
.unwrap_or(empty_wait_time),
Duration::from_secs(30),
),
empty_wait_time,
);
tokio::time::sleep(wait_duration).await;
}
}
}

const CACHE_DURATION: Duration = Duration::from_secs(60 * 10);

struct CacheEntry {
struct UserCacheEntry {
display_name: String,
email: Option<String>,
roles: HashSet<String>,
Expand All @@ -23,10 +92,10 @@ struct CacheEntry {
/// This works fine in multi-node setups: each node just has its local cache and
/// prevents some DB writes. But as this data is never used otherwise, we don't
/// run into data inconsistency problems.
pub(crate) struct UserCache(DashMap<String, CacheEntry>);
pub(crate) struct UserCache(DashMap<String, UserCacheEntry>);

impl UserCache {
pub(crate) fn new() -> Self {
fn new() -> Self {
Self(DashMap::new())
}

Expand All @@ -50,7 +119,7 @@ impl UserCache {
Entry::Vacant(vacant) => {
let res = Self::write_to_db(user, db).await;
if res.is_ok() {
vacant.insert(CacheEntry {
vacant.insert(UserCacheEntry {
display_name: user.display_name.clone(),
email: user.email.clone(),
roles: user.roles.clone(),
Expand Down Expand Up @@ -95,3 +164,88 @@ impl UserCache {
}
}


// ---------------------------------------------------------------------------

#[derive(PartialEq, Eq)]
struct AuthCallbackCacheKey(HeaderMap);

impl Hash for AuthCallbackCacheKey {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
// Sigh, unfortunately this requires us to sort the headers in order to
// get the same hash for the same set of headers. Well, there might be
// some clever ways to avoid that, but the `Hasher` trait is quite
// limited and does not allow these clever tricks, at least not without
// basically writing your own hashing logic.
let mut keys = self.0.keys().collect::<Vec<_>>();
keys.sort_by_key(|hn| hn.as_str());

for key in keys {
for value in self.0.get_all(key) {
key.hash(state);
b": ".hash(state);
value.hash(state);
state.write_u8(b'\n');
}
}
}
}

struct AuthCallbackCacheEntry {
user: Option<User>,
timestamp: Instant,
}


/// Cache for `auth-callback` calls.
pub(crate) struct AuthCallbackCache {
map: DashMap<AuthCallbackCacheKey, AuthCallbackCacheEntry>,
// Metrics
hits: Counter,
misses: Counter,
}

impl AuthCallbackCache {
fn new() -> Self {
Self {
map: DashMap::new(),
hits: Counter::default(),
misses: Counter::default(),
}
}

pub(crate) fn hits(&self) -> &Counter {
&self.hits
}

pub(crate) fn misses(&self) -> &Counter {
&self.misses
}
pub(crate) fn size(&self) -> usize {
self.map.len()
}

pub(super) fn get(&self, key: &HeaderMap, config: &CallbackConfig) -> Option<Option<User>> {
// TODO: this `clone` should not be necessary. It can be removed with
// `#[repr(transparent)]` and an `unsafe`, but I don't want to just
// throw around `unsafe` here.
let out = self.map.get(&AuthCallbackCacheKey(key.clone()))
.filter(|e| e.timestamp.elapsed() < config.cache_duration)
.map(|e| e.user.clone());

match out.is_some() {
true => self.hits.inc(),
false => self.misses.inc(),
};

out
}

pub(super) fn insert(&self, key: HeaderMap, user: Option<User>) {
self.map.insert(AuthCallbackCacheKey(key), AuthCallbackCacheEntry {
user,
timestamp: Instant::now(),
});
}
}

Loading

0 comments on commit 7829194

Please sign in to comment.