UCL-INGI / INGInious

INGInious is a secure and automated exercises assessment platform using your own tests, also providing a pluggable interface with your existing LMS.
http://www.inginious.org
Other
202 stars 139 forks source link

[frontend] SAML auth no username #953

Closed javawolfpack closed 1 year ago

javawolfpack commented 1 year ago

Describe the bug So I got SAML2/Shibboleth to finally authenticate; however, upon authentication, the user is immediately sent to their profile page and prompted to enter a new username. The SAML request should have returned a uid/username, but this isn't used for some reason.

Also, the profile page provides an interface to change passwords, which should potentially be disabled for accounts from identity providers.

INGInious installation details

To Reproduce Steps to reproduce the behavior:

  1. Authenticate with SAML
  2. Attribute from documentation:
    uid: "urn:oid:0.9.2342.19200300.100.1.1"

Expected behavior Expect the user ID to already be set/not changeable. Password changing shouldn't be on the profile page.

Screenshots

Screenshot 2023-06-02 at 10 21 54 AM

Desktop (please complete the following information): N/A

Additional context Could potentially be a feature request vs bug, but this feels like a bug and not a feature.

javawolfpack commented 1 year ago

I just tested, and no username appears in the administration panel either unless they manually set one:

Screenshot 2023-06-02 at 10 29 54 AM
javawolfpack commented 1 year ago

I just tested adding the following to the configuration.yaml fixed the password change option, but the no username is still an issue:

allow_registration: false
anthonygego commented 1 year ago

INGInious works with local accounts and provides a way to bind one or more external authentication providers to accounts. Account unicity is based on email addresses, as it is expected that only one person is behind the same email address. Allowing registration simply enables users to create accounts without any binding, storing the password hash in INGInious database and letting the platform deal with password recovery. That's for the basics. Theoretically we don't need anything else.

Why usernames ? INGInious requires some users (typically course administrators) to search for other users (students and/or other course administrators) in the whole database. In order to avoid homonym issues, we need an additional field to differentiate them. Emails would fit perfectly as there are unique. However, email addresses are considered private information and we should use them only for functional purposes. Pre-GDPR and pre-"dot org" instance periods have made this concern irrelevant and some pages still exposes emails while we should ideally replace these stuff by integrated email mechanism. This issue is currently mitigated by the fact the "dot org" instance is currently readonly for visitors and the emails are therefore only exposed to the same people that have access to the database. In short, we clearly cannot use emails to help users search for the right person anymore and thus we need a username. Some web platforms allow for sharing things with other users by specifying their emails but do not allow search.

Why do not populate username automatically ? Basically because there is no obvious bijection to perform. When a binding is done, it is performed based on the email address as this is the common unique identifier. The platform-specific user identifier is then used to store the binding in the database. This identifier is sometimes textual, sometimes just a hash. Identifier, especially the textual ones, may conflict and make automatic username population tricky. A common workaround is to prepend the user id with the authentication provider but that would expose publicly the account information on the external platform, in addition to not being human friendly inc ase of hashes.

That may seem overkill for deployments where only one external identity provider is used, case in which a bijection is possible. One would say it can be simply implemented by setting a unique allowed external provider, but we need a more robust mechanism to prevent bad usage as this feature should be enabled at database initialization and never be changed.

javawolfpack commented 1 year ago

So basically, that is the expected behavior, not a bug. Thanks for clarifying.