fit-ctu-discord / honza-botner

Bot for FIT CTU Discord
https://discord.fit.cvut.cz/
MIT License
15 stars 10 forks source link

Better subject picker #446

Open SoptikHa2 opened 1 year ago

SoptikHa2 commented 1 year ago

Problem

Current subject picker is not good. There are way too many subjects. Because of this, the subject picker is confusing and it's hard to find the subject I want to choose. Also it takes so much time, especially since I have to search in two separate channels. And Ctrl+F doesn't help that much because of discord design.

Example solution

FEL:

image

I can sign up for a subject by code (after clicking a button), this adds (removes) a subject immediately if found. image

I can also search for a subject (after clicking a button): image

Result: image

I can both add and remove subjects by clicking the green / red button.

I would like to do something similar here.

SoptikHa2 commented 1 year ago

My current propsal is to implement the search button, and allow adding/deleting subjects based on it. The "sync subjects with current semester" might be good to implement in the future as well, but isn't the goal right now.

ostorc commented 1 year ago

For this better UX (semi-automatic sync), we would have to store the CTU login of users (we keep it now in some hashed form, which is not usable with this workflow).

SoptikHa2 commented 1 year ago

What is even the advantage of keeping the logins hashed and not in plaintext? It's pretty easy to bruteforce them if one has access to the database anyway. Would there be any security implications if we stored the CTU login in plaintext?

But for now, I'd like to implement just the "Search for subject" button, which is easier than working with KOS api as well.

tenhobi commented 1 year ago

Hmm, there is no simple solution to that. We would need some legal permission to store plain usernames, I think.

stepech commented 1 year ago

Not really. They grant us approval to do so when they authorize the app. Bigger issue would be with storing tokens.

tenhobi commented 1 year ago

Are tokens persistent anyway?

tenhobi commented 1 year ago

Maybe we can do it as we do now with verification, e.g., open the OAuth page, and the user needs to verify to complete the action. It has the extra step with the browser, but dunno, a good enough compromise for me.

ostorc commented 1 year ago

Why would we want to store tokens anyway? Cant we just use our own service account for this lookup by username?

stepech commented 1 year ago

Why would we want to store tokens anyway? Cant we just use our own service account for this lookup by username?

"Technically" you can, "practically" you can.... But if you will, you can get banned for using that open API endpoint and be threatened with disciplinary commission for like 6 months. So we really shouldn't even though we can.

Are tokens persistent anyway?

There are 2 tokens. 1 you receive when user authorizes your app. This token is persistent (afaik). When you start your communication with the server on behalf of this user, you then exchange this persistent token for another token, which is temporary (I think 1 hour) which you then use to authorise all requests.

stepech commented 1 year ago

Yes there is, but it's the disciplinary comission way, really :rtzw:

SoptikHa2 commented 1 year ago

But if you will, you can get banned for using that open API endpoint

What are the terms of service? Maybe the issue was with mining and publicly sharing the info, not using the service itself. Perhaps we could ask if that's okay. I can write an email ig.

stepech commented 1 year ago

Btw, this idea has maaany other difficulties to figure out.

Also deployment issues. If we start doing this (in general, not the auto role assignment), there might be some larger modifications to server needed, which would result in temporary server "shutdown". So we would have to either follow the already deployed system and bend the new implementation to use it as much as possible, or choose some period with lower server activity when not many people need it. This points to summer holidays :smile:. Or we can somehow automate the deployemnt with some script...? Do a template server, test it there and then apply it during update...?

stepech commented 1 year ago

But if you will, you can get banned for using that open API endpoint

What are the terms of service? Maybe the issue was with mining and publicly sharing the info, not using the service itself. Perhaps we could ask if that's okay. I can write an email ig.

Issue was I was using personal data without all users consent. Maybe if we do some text above the Auth button like "By clicking the button you grant us the permission to use your data in accordance with our ToS..." Yeah, we might need ToS for this, right? :sweat_smile: