Skip to content

Feature suggestion: Authentication #1151

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
baruchiro opened this issue Jul 6, 2018 · 10 comments
Closed

Feature suggestion: Authentication #1151

baruchiro opened this issue Jul 6, 2018 · 10 comments
Labels
📋 pending-reply work status: pending-reply

Comments

@baruchiro
Copy link

I wrote an authentication function to my private bots, to control who can use the bot.
I thought to add it to this project, but there are 2 problems:

  1. The bot should save a file, DB, to remember which user are authenticated.
  2. The manager- the first user, should insert his id directly to code or DB.
@jsmnbom
Copy link
Member

jsmnbom commented Jul 8, 2018

This is not currently in the scope of this project. We do provide our users with a simple way of doing this over on our wiki (Code Snippets -> Restrict access to a handler (decorator)).

@jsmnbom jsmnbom closed this as completed Jul 8, 2018
@baruchiro
Copy link
Author

That's my intention. Make a default decorator that will be included in the package.

@jsmnbom
Copy link
Member

jsmnbom commented Jul 8, 2018

Alright in that case.
I personally think it's okay to just have it on our wiki, but @python-telegram-bot/developers what do you guys think?

@jsmnbom jsmnbom reopened this Jul 8, 2018
@JosXa
Copy link
Contributor

JosXa commented Jul 9, 2018

Design proposal:

  • A Role can be any comparable object: Enum, custom class, ... It's just important that they implement the __gt__, __lt__, and __eq__ methods.
  • Dispatcher holds a mapping of Dict[Role, List[UserID] in dispatcher.auth_roles
  • We'll provide a default Enum (or static class in 2.7) that defines UserRole.Admin = 0, UserRole.Moderator = 10, and UserRole.None = 20 (dunno about the naming...)
  • Set authorization for a group of users as follows: dispatcher.auth_roles[UserRole.Admin] = [62056065, 1234567]
  • Default handler authorization works by *Handler(callback, ..., role=UserRole.Admin), where the default value is a very high number. All numbers lower than what the Handler defines will be applicable for the handler (so if you specify UserRole.Moderator, then Admins will also be able to access it).
  • We'll also provide an @auth (or @role?) decorator that takes the dispatcher singleton and forces the dispatcher to do an additional check before executing the handler. Not quite sure how this should play together with the argument to *Handler, but in any case I think decorators would be good for code locality (i.e. "you immediately see whom a handler applies to).

Comments appreciated.
Perhaps we can also get inspiration from how other frameworks do this...

@icekom
Copy link
Contributor

icekom commented Jul 11, 2018

A few suggestions:

  • Higher roles should have higher Enum value: UserRole.Banned = -10 can be used to ban abusive users if needed (global check that forbids all interactions to users with negative value?), UserRole.Default = 0 (Using None might not be the best idea), UserRole.Moderator = 10, UserRole.Admin = 20 and so on if there will be any higher roles.
  • Allow setting a custom function that will be called before every permission check which should return a role mapping. If no such function is set, use dispatcher.auth_roles. (Can be used to generate mapping from a database for example)
  • Maybe it will be easier to implement something like this using a Filter?

@JosXa
Copy link
Contributor

JosXa commented Jul 11, 2018

I like those suggestions.

Filters for roles should be there, of course. I fail to understand the custom function for role mappings so far, what stops you from simply adding it to the dispatcher or modifying the existing mapping in there?

Another thing:
Perhaps we could include the callback permission checks and datastructures already in the Updater so that forbidden updates won't even reach the dispatcher? In addition, the dispatcher is not available in the new CallbackContext, whereas the updater is - meaning that it becomes easier to add a new role in a callback:
context.updater.set_role(update.effective_user, UserRole.Moderator) or similar.
Updater also sounds like the right level to do this at, judging from its name.

@icekom
Copy link
Contributor

icekom commented Jul 24, 2018

Maybe a function is not the best way to do it. What I meant is that there should be a way to ensure that your role mapping is always up-to-date.

For example:
If you have a website, you may want to get users and their roles from your websites database. So if you have a new moderator on your website, he should also gain Moderator role in the mapping. And if you ban someone on your website, he should get Banned role in the mapping as well next time he tries to interact with the bot.

@Eldinnie
Copy link
Member

Eldinnie commented Oct 2, 2018

Is there anyone that is working on this, or any good reason to keep this open? Will close after 2 weeks without respons

@Eldinnie Eldinnie added the 📋 pending-reply work status: pending-reply label Oct 2, 2018
@markusressel
Copy link

markusressel commented Jul 18, 2019

I needed this (and a couple other things) myself so i wrote a lib for that.
Its more of a helper tool to quickly build telegram commands that act like shell commands but it also allows to setup permissions.
It's python 3.5+ only and uses the latest v12 beta of python-telegram-bot but if you are interested have a look:
https://github.com/markusressel/telegram-click

@Bibo-Joshi Bibo-Joshi mentioned this issue Feb 21, 2020
8 tasks
@Bibo-Joshi
Copy link
Member

Because this was due to be closed 2 years ago and we recently decided to move #1789 to a contrib repo (see #1912), I'll go ahead and close this.

@github-actions github-actions bot locked and limited conversation to collaborators Oct 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
📋 pending-reply work status: pending-reply
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants