codidact / qpixel

Q&A-based community knowledge-sharing software
https://codidact.com
GNU Affero General Public License v3.0
385 stars 69 forks source link

SAML Single Sign On support #825

Closed Taeir closed 1 year ago

Taeir commented 2 years ago

Describe the solution you'd like Support for SAML sign-in solutions such that self-hosted instances of QPixel can use alternative sign in methods. For example, larger institutions often have their own single sign on service. Many universities for example are connected to the SurfConext network.

Describe alternatives you've considered Alternatively to SAML, Active Directory is also a commonly used sign-in system by institutions. Adding support for this would require a similar setup as for SAML and would raise the same questions. It does seem to be slightly less popular (though I have not done extensive research).

Additional context It is not technically difficult to add support for SAML into QPixel. The devise_saml_authenticatable gem integrates fully with the already used devise sign-in framework. A minimal amount of additional code and configuration will make actual sign ins/sign ups work.

The main work lies in the required configuration options (in the interface? in a file?), how to present the additional sign in methods to the user and the details of interaction with existing logic. Some examples:

I have already implemented required changes to support the minimal viable product of signing in using SAML (multi-login support). I can also create a tutorial on how to set this up for an instance if you are interested. I may contribute some code if I can make it integrated enough to add without breaking anything when SAML is not used. I'm mainly looking forward to hearing your thoughts and ideas on where you would like to see from such an integration.

cellio commented 2 years ago

Thanks for starting the ball rolling on this. As you note, there are some decisions that need to be made, and I'm not qualified to opine on the technical implications, so this comment is from more of a user/"product" perspective.

User accounts (including login credentials) are global within a network; if you're logged in to any community on the network you're logged into all of them. Currently there is one username (email address) and one password. I don't know what structures would need to change to allow one username and more than one sign-in path (password or SAML). It sounds like you've already gotten that working? Does the password continue to work if the SAML credential is revoked? I think if we had a way to add SAML to a password-based account, such that the user could log in with either and the absence of SAML doesn't break anything, that would be worth adding to the platform. I"m not sure how to test the SAML side of it without that access, but presumably we can test "make sure the existence of that code doesn't break passwords".

I think we want this to be explicitly enabled on a network, not available by default. You'd have to configure the SAML integration anywhere (server or endpoint or wherever the integration point is), so best if we don't leave a possible path for exploits open until the person setting up an instance decides to enable it.

Setting up a network includes seeding the database, so I'd guess that we'd want configuration of SAML to be through the database too. Does this need admin UI? After a SAML-enabled instance is running, do people administering that instance (who don't have database access) need to be able to see that SAML configuration? If so we'll need to think about where to add that in the admin tools, but I can't tell if that's needed.

I assume there would need to be UI changes for the user, both for account creation and for login.

A tutorial for setting up the MVP (multi-login with SAML) would be great -- it'd let others with this problem implement SAML without having to wait for the team to work out answers to the questions you've raised and implement something (or merge your implementation if that happens). There is an installation guide (linked from the readme), which seems like a good place to add a SAML section. Thanks for the help!

Taeir commented 2 years ago

I don't know what structures would need to change to allow one username and more than one sign-in path (password or SAML)

It (unfortunately) depends on the institution. Many institutions use email as a unique identifier. However, some institutions (like mine), use a different identifier as a unique one (its called netid, network-id), and actually randomly change email addresses of some users. In either case, it would be most flexible to add a single field to the database called saml_identifier (or similar) which will hold the unique key for the specific SAML server.

Does the password continue to work if the SAML credential is revoked? I think if we had a way to add SAML to a password-based account, such that the user could log in with either and the absence of SAML doesn't break anything, that would be worth adding to the platform.

The authentication library used by QPixel supports multiple parallel sign in methods. If you enable both database authentication (username + password, this is what you have now) and SAML authentication, either can be used to sign into an account and both will work (unless explicitly blocked by some additional code).

SAML is essentially just a QPixel instance asking a so-called identity provider (IDP) to share and confirm the identity of a user (i.e. email + possible other information). The flow is like:

If you replace identity provider in that story with Google, Facebook or GitHub, you will have probably used such a sign in method before somewhere (though that is technically oauth, not SAML).

What I wrote for our instance is a dual sign in where both password login and SAML login work side by side. The way I configured it though is that a SAML user cannot sign in anymore through normal password authentication, since that is what we want.

I think we want this to be explicitly enabled on a network, not available by default. You'd have to configure the SAML integration anywhere (server or endpoint or wherever the integration point is), so best if we don't leave a possible path for exploits open until the person setting up an instance decides to enable it.

Right, I think then a 4-choice setting makes sense:

Setting up a network includes seeding the database, so I'd guess that we'd want configuration of SAML to be through the database too.

It may be too complex to add into the database. It also requires encryption keys and encryption certificates which one may not want to have in the database. I think most of these settings would probably need to be in a configuration file, just like for example the storage setup (S3 / Local / Other) and the database setup.

Does this need admin UI? After a SAML-enabled instance is running, do people administering that instance (who don't have database access) need to be able to see that SAML configuration?

No, I think it's too technical for them.

I assume there would need to be UI changes for the user, both for account creation and for login.

If there is a 4-option configuration as suggested above:

A tutorial for setting up the MVP (multi-login with SAML) would be great

I will get on that then :) However, I feel like it would be a bit of a longer tutorial which may clutter up the installation guide. Perhaps it would also make sense as a separate article (i.e. on the Wiki of the repo?).

Taeir commented 2 years ago

Update, I just realized you do have some support/integration for OmniAuth, with which it would also be possible to do SAML authentication. Is that actually being used by any of the currently available communities?

cellio commented 2 years ago

We aren't using OmniAuth on our instance to the best of my knowledge. I don't know the history of that support in our code; don't know if it was an experiment, partial work then abandoned, or something that works fine but we decided not to use. Pinging @ArtOfCode- .

cellio commented 2 years ago

It may be too complex to add into the database. It also requires encryption keys and encryption certificates which one may not want to have in the database. I think most of these settings would probably need to be in a configuration file, just like for example the storage setup (S3 / Local / Other) and the database setup.

Makes sense to me -- definitely don't want keys in the database.

Does this need admin UI? After a SAML-enabled instance is running, do people administering that instance (who don't have database access) need to be able to see that SAML configuration?

No, I think it's too technical for them.

Great, then we don't need UI changes except for the sign-in/sign-up part.

A tutorial for setting up the MVP (multi-login with SAML) would be great

I will get on that then :) However, I feel like it would be a bit of a longer tutorial which may clutter up the installation guide. Perhaps it would also make sense as a separate article (i.e. on the Wiki of the repo?).

Wiki article with link from the installation doc sounds good to me. (I'm not sure how wiki access is managed -- happy to grant you whatever permissions are needed but I haven't found where to do that.)

ArtOfCode- commented 2 years ago

@Taeir Where are you seeing the OmniAuth support? We have an OAuth-like flow in place for Stack Exchange integration, though that's little used and from the client side... Other than that I can't think of anything?

Taeir commented 2 years ago

@ArtOfCode- Right, I just noticed the gem and noticed one "Sign In" button in the views/devise/shared/_links.html.erb menu. I didn't see any other logic for it though, so that's also why I found it late.