ctu-fee-group / Christofel

Modular Discord bot for unofficial CTU FEE server. Handles authentication, user interaction etc. Allows for attaching plugins during runtime using AssemblyLoadContext.
MIT License
5 stars 0 forks source link

Auth process for Discord users #1

Closed Rutherther closed 3 years ago

Rutherther commented 3 years ago

Discord users can be authenticated using Discord oauth2 + CTU (https://auth.fit.cvut.cz/) oauth2 to link Discord account with CTU account. If linking was successful and the user is member of CTU, appropriate roles will be added.

The flow is described using markdown and also a diagram will be attached later.

Auth flow

Flow goes as follows:

  1. User loads /auth, code GET parameter can be passed with existing auth code, then jump to CTU auth is made (step 4, disable button for discord)
  2. User has to authenticate using Discord oauth2 by clicking on a button
  3. On return, access code for discord is retrieved. If there are errors, error page is shown, otherwise record is saved to the database. The record is saved with appropriate discord id and generated auth code that will be passed to CTU oauth state param to return it in GET param so we can link the account.
  4. User has to authenticate using CTU Discord oauth2 by clicking second button
  5. On return, access code is retrieved along with information from usermap API and KOSapi. In case of an error, error page is shown. Information about the user are stored in database and/or used to add roles on the Discord server. Duplicities must be handled correctly. The entire process can be seen below Auth finish heading

Error page

Error page should display error text and a button with return back button. This button will return user back to the auth flow before failure.

This feature was implemented using return GET parameter in the previous version of Christofel.

Discord data retrieval

Data about user can be retrieved from: https://discord.com/api/v8/users/@me We are interested in disocrd id and discord user.

Common errors

This is a list of errors that are known for happening sometimes and we should notify the user about it without displaying just generic error message.

For other errors, generic error should be shown to the user and more information should be logged. (http codes, responses, what part of the auth process)

User model

Clearing data

When using auth process, data are saved to the database and could be abandoned. For example if the user authenticates using Discord, then closes the page and authenticates once more.

Because of this, cron should be set up so it will take care of unfinished auth processes older than 2 days (Authenticated flag is set to false and the record is older than 2 days)

Auth finish (after both Discord and CTU auths are done)

The access token both for CTU and Discord should be thrown away right after it is not needed. We do not save it anywhere. We also don't save any data we don't need. Just the bare minimum to be able to tell apart duplicities s + be able to identify the person using UDB.

After auth is finished, AuthCode field is made NULL/empty, Authenticated is set to true and AuthenticatedAt is set to current time (could be used to replace Authenticated)

  1. Check for duplicities and handle them accordingly
  2. Get all user information we can get
  3. Save information we need to save
  4. Add roles
  5. Finish process by saving correct flags (Authenticated, AuthenticatedAt, AuthCode)

If any of these steps fails, the process shall not continue and previous auth finish steps should be reverted if possible.

Duplicities

There are 4 types of duplicities that can arise.

  1. Discord side duplicity (there are 2 Discord accounts linked to 1 CTU account)
  2. CTU side duplicity (there are 2 CTU accounts linked to 1 Discord account)
  3. Both sides duplicity (CTU matches Discord in one record!) (there is already record for this user)
  4. Both sides duplicity but for different users

Handling is different for each case.

Discord side duplicity

Discord side duplicity is rare, but can happen. Possibly because of ban or when losing account access. We can allow this duplicity manually (DuplicityAllowed flag), if it is not allowed, authentication process results in an error telling the user there is a duplicity and that he should notify administrators.

If it is allowed, it must be ensured that the ctu account we allowed it for is the one logging in. So user cannot authenticate using another ctu account with duplicity. For example, user Bob logins with ctu account and the app detects this kind of duplicity. He will text us and we will allow the duplicity with the flag. Then we send him url with the correct auth code parameter and he logs in with account of Alice. This should result in an error, because the specified auth code is for Bot, not for Alice. If Bob logged in using his account, it wouldn't result in an error.

CTU side duplicity

This duplicity shouldn't happen, at least there is not known case when it would. We will not allow this duplicity unless proper explanation for this is given (maybe there could be an error in KOS so it assigns another account to someone who studied CTU few years ago?). This duplicity should be handled the same way as Discord side duplicity is.

Both sides duplicity (meaning one record with same CTU and Discord is found)

Both sides duplicity means the user relogged. The case for this could be desire to update Discord roles, reset kick timer etc. This duplicity is allowed automatically. Record with the same user is found and updated. The user is given new roles and some old can be deleted.

Different users both sides duplicity

Just treat it as CTU/Discord side duplicity, this particular use case shouldn't really happen except for someone making fun of us.

Retrieve user CTU information

To retrieve correct information about the person, usermap API and (or) KOSapi is used.

Data should be first retrieved from usermap API and then missing data should be retrieved from KOSapi. If no data are loaded from both APIs (returning 404 etc.) should the auth process end up in error? What are critical data that must be loaded for everyone? (cases when both usermap and KOSapi returned 404 already happened)

Retrieving user information from usermap API

We will be using /user/{username} endpoint. The documentation is located here: https://kosapi.fit.cvut.cz/usermap/doc/rest-api-v1.html#people__username__get

Data are retrieved from the roles. Roles look like this: (B|T)-([0-9]{5})-(.)-(.)-...

More info about roles (czech language) https://rozvoj.fit.cvut.cz/Main/role-v-idm-cvut

Retrieving user information from KOSapi

KOSapi documentation: https://kosapi.fit.cvut.cz/projects/kosapi/wiki/Resources

  1. Load /people/{usernameOrId}, check for roles/teacher, roles/student/
  2. Get Bakalar, Magistr, Doktor from titlesPre, titlesPost
  3. If there is any roles/teacher, it is a teacher, otherwise student
  4. For teachers, load /teachers/{usernameOrId}, for students /students/{studyCoreOrId} (For both teachers and students load students so programme can be retrieved)
  5. Get faculty from division (field faculty for students, division for teachers), for students get year (startDate) and programme (field programme)

Note: try to get the earliest study of the student (of the same type he/she is currently studying) for accurate year. For simplicity first student role can be selected, that is the latest one. Correct year retrieval can be implemented in future

Assigning roles

This should be done async after the user was notified that his roles will be added. That's because of the rate limiting.

Roles that could be assigned (grouped to categories):

Array of current roles (only those in the list above) vs new roles should be made and then new roles should be added, the old ones removed. We do not want to remove all the roles and add it back. That would make the bot stuck for couple or seconds or even half minute because of the rate limiting.

Rutherther commented 3 years ago

Questions that need answers

What if the user is from FEL and cannot be found in KOSapi? Possibly a new user Should the process end up in error if no data are loaded both from usermap and kosapi?

Rutherther commented 3 years ago

Discuss better duplicity allow flow - delete old account along with removing access to the server?

Rutherther commented 3 years ago

Deletion of old duplicate accounts will be done manually if it is needed.

Rutherther commented 3 years ago

What if the user is from FEL and cannot be found in KOSapi? Possibly a new user Should the process end up in error if no data are loaded both from usermap and kosapi? In this case the user will end up only with Authenticated role.

The thing is that there could be someone who isn't a student (doesn't have start year, doesn't have current study programme), doesn't have any titles, isn't a teacher and doesn't belong to any faculty.

Then it would be the same case - only Authenticated role.

Therefore I don't think we can distinguish between these cases and we should allow people to authenticate even if only Authenticated role will be added.

This should still be logged, prefferrably as a warning, though. That way we can notify these users. DM message could be sent automatically as well