Add framework for authentication (authkit) and use it for local dev setup & test deployments #645
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This replace `login-handler.py`, which will be removed in future commits. It works slightly different as it takes the deploy ID as CLI arg instead of a socket path.
Since the login handler does not rely on `X-Accel-Redirect` anymore, we can remove parts of the nginx config.
LukasKalbertodt
force-pushed
the
auth-framework
branch
from
9bfcd6a
to
f341336
Compare
LukasKalbertodt
force-pushed
the
auth-framework
branch
2 times, most recently
from
1b6a4f4
to
f44e105
Compare
I think having this mode is very useful for multiple reason. For one, some organizations want exactly this auth-system and now don't need to build it themselves anymore. We wanted to be auth-system agnostic in Tobira, but I think it's fair to have an exception for Opencast as that is _the obvious_ system to support out of the box. Communication with Opencast is setup anyway. Moreover, I think this is a great starting point for anyone setting up Tobira. Because then, it's super easy to get Tobira auth up and running. Setting up the auth otherwise is usually fairly involved. And until then, you only have an empty Tobira and can't even add pages or blocks. So I think this vastly improves the initial setup experience. We are careful in describing this feature: there won't be any configurable behaviors about this. We just take the most obvious route and take exactly the data Opencast provides. Anything else needs to be done by writing your own login handler.
JulianKniephoff
requested changes
Jan 11, 2023
JulianKniephoff
approved these changes
Jan 11, 2023
LukasKalbertodt
force-pushed
the
auth-framework
branch
from
79d7ee6
to
bac2e97
Compare
LukasKalbertodt
force-pushed
the
auth-framework
branch
from
bac2e97
to
be3ecd1
Compare
This is done to make them hopefully easier to understand and also to reflect the recent additions: `auth.mode = "opencast"` and the authkit.
LukasKalbertodt
force-pushed
the
auth-framework
branch
from
e6f35c5
to
2302516
Compare
While the former is slightly nicer (because shorter), it's easier to just manage one NPM scope. From a user perspective it's also easier to "trust" just one scope instead of checking of `@tobira` is legit too.
JulianKniephoff
requested changes
Jan 18, 2023
There was a problem hiding this comment.
Mostly language and style notes, but also some about the structure, which we talked about a bit already.
After some more thinking and reading I think the main thing that bugs me is the "User information Tobira needs" in the user login section. That doesn't belong there I think. Maybe put it in the "in depth" document?
And then yes, maybe move that to the end as we said, because that reads very much like a reference document.
Also maybe make it a bit clearer which parts are expected to be read by which users. Specifically maybe mention when ppl using the opencast mode can stop. 🤔
LukasKalbertodt
force-pushed
the
auth-framework
branch
from
c433998
to
cf1aa00
Compare
LukasKalbertodt
force-pushed
the
auth-framework
branch
from
cf1aa00
to
544c7b6
Compare
JulianKniephoff
approved these changes
Jan 18, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #550
See the referenced issue for motivation. Can be reviewed commit by commit, but reviewing all in one isn't bad either.
As you can see from the changes, our framework does more than the previous
login-handler.py: it sends thePOST /~sessionrequest to Tobira itself. That means noX-Accel-Redirectmagic is needed, making the nginx simpler.The main part is done, but this is still a draft for a number of reasons:
@opencast/tobira-authkitauth.mode = "opencast-users". In that mode, Tobira would handlePOST /~loginrequests itself by just asking Opencast if the login data is correct. That should be fairly easy to do. And I think it's very useful: it's the obvious auth system Tobira should support and I assume quite a few institutions want to use it. Further, even if an institution wants a different auth system in the long run, just using Opencast users is a very simple way to get everything running. Without auth system, you only see an empty Tobira which is quite underwhelming. So this would drastically improve the "first setup" experience IMO.I also noticed that we might want to simplify login handlers even more by adding yet another mode. In that mode, Tobira would forward-> I still think it's useful, but we won't do it now.POST /~loginrequests to a configured HTTP service which would send back appropriate auth headers. This is little work in Tobira and that way, the nginx config could be a lot smaller still, again making it easier to set up Tobira.@tobira/authkitfor now.So yeah, I'd like to talk about the two additional auth modes and whether we want them. Of course they could be implemented in a separate PR. But it would also be nice to only needing to rewrite the docs once.