Make auth system more flexible & convenient and add "callbacks"

[LukasKalbertodt]

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"

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

"login-proxy"

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

"full-auth-proxy"

[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

[geichelberger]

The caching of the callback results per node needs to be revised with a distributed setup. This can have funny side effects if the cached data is inconsistent.

[LukasKalbertodt]

Fair point. However, I would note that usually, your load balancer would always forward the same user (as identified by IP address or whatever) to the same node. And more importantly: with this kind of caching, that can hold stale data, there can always be "funny" side effects.

Making this caching shared between all Tobira nodes basically defeats the purpose, i.e. performance, if the cache. It also adds more complexity. If this caching is not acceptable in your situation, it should be disabled in Tobira. And you can instead have your own cache in the auth callback, for example. All I'm saying is: I had this in mind when deciding for an in-memory/in-process cache.

Thanks for taking a look at this PR!

[LukasKalbertodt]

But I suppose adding a note to the docs about this is a good idea 🤔

[geichelberger]

Sticky sessions should be avoided if possible; they have some drawbacks.

If both nodes have the same stale data, that's not so problematic, but if one node allows you to edit pages and the other doesn't, it results in weird behavior with a bad user experience.

[LukasKalbertodt]

Sticky sessions should be avoided if possible; they have some drawbacks.

Ah, I didn't know.

If both nodes have the same stale data, that's not so problematic, but if one node allows you to edit pages and the other doesn't, it results in weird behavior with a bad user experience.

Certainly. So as I said, then I would disable the Tobira cache to avoid these problems. I don't think it's a good idea to adjust the Tobira cache to be shared between nodes, as it adds complexity and defeats the purpose of the cache as it's likely not much faster than just calling the callback. If you use a multi-node setup and your callback performs expensive operations, then the callback should cache those operations itself.

[JulianKniephoff]

Haven't looked at the code, yet, but went through the docs rather thoroughly. Overall they are very nice docs. I still added quite a few more or less nitpicky comments. Specifically the "writing advice" ones are probably debatable. I stand by them, but none of them need to block this PR, I guess. 🤷‍♀️

[JulianKniephoff]

Maybe cross-link to the relevant section about intercepting those?

[JulianKniephoff]

So I wanted to review this initially, mostly because I was also slightly involved in the conceptual design of this. Unfortunately I don't really have the time at the moment to do a proper in-depth code review. 😕

I did let myself do a rather thorough read of the docs, giving the whole design a final sanity check in the process. After that review (see above) I also got to do some basic testing with a local Opencast. Everything seemed to be working as expected.

I also skimmed large chunks of the code, but not everything. FWIW I didn't find any glaring mistakes doing that. The few small nitpicks I had might also just be missing some minor detail, so I don't think they'd add any value here. 🤷‍♀️

[LukasKalbertodt]

So, then I think this is good to merge! Multiple people had a look at this and this is deployed on an ETH test system for a while now, working as expected.