Skip to content

Add a Telegram factor option#252

Open
Dagefoerde wants to merge 8 commits intocatalyst:MOODLE_400_STABLEfrom
Dagefoerde:mootdach-telegram-v2
Open

Add a Telegram factor option#252
Dagefoerde wants to merge 8 commits intocatalyst:MOODLE_400_STABLEfrom
Dagefoerde:mootdach-telegram-v2

Conversation

@Dagefoerde
Copy link

[Replaces #231]

This plugin lets users define their Telegram ID. Upon login, it generates a six-digit code and sends it to the defined Telegram ID. The plugin is a product of the #MootDACH20 DevCamp (https://moodlemootdach.org/course/view.php?id=13). @Laur0r and I have had a lot of fun adding this alternative to tool_mfa! 💯 Now that DevCamp is over, and some time has passed, I finally got around to do some more work. As there is a new secret manager API, I have created the plugin from scratch.

Still needs testing and a bit of improvement in the admin UI, so WIP.

@Dagefoerde Dagefoerde marked this pull request as ready for review January 21, 2021 09:56
@Dagefoerde
Copy link
Author

I have added help texts for admins and users, thus improving the UI overall. Also I have tested the plugin's functionality and believe that, in my opinion, it is production-ready now. Happy to hear your reviews.


// Logout button.
$url = new \moodle_url('\admin\tool\mfa\auth.php', ['logout' => 1]);
$url = new \moodle_url('/admin/tool/mfa/auth.php', ['logout' => 1]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've fixed bug this in master please remove it from here - thanks nice catch :)

@brendanheywood
Copy link
Contributor

brendanheywood commented Feb 5, 2021

hi @Dagefoerde this is looking pretty good. Setting up the bot as admin was smooth, and I really like the info bot which shows you your id. I did have some issues:

  1. I did run into a weird loop where I entered my setup code and then it asked for it again over and over. I revoked it and tried a second time and I got this error:
! ) Notice: Undefined property: stdClass::$telegramid in /var/www/moodle.local/admin/tool/mfa/factor/telegram/classes/factor.php on line 274
--


1 | 0.0000 | 365664 | {main}( ) | .../action.php:0
2 | 0.9307 | 7629320 | factor_telegram\factor->setup_user_factor( ) | .../action.php:93
  1. When you try to setup a telegram user, I could not get the username to work but I could use the telegram id. But if you try to register something which doesn't work, and you go back and start over, then it remembers the username or id from before and you can't enter a new one. I think both of these issues stem from using the session variable, I think this should be stateless and part of the form definition. ie the form has one element for the userid, you save it and it fails validation, re-renders the form and now adds the second form element based on the partial form $data rather than session. In theory you could register two telegram ids at the same this way and they won't clash.

After that gets fixed I think this is good to land, good stuff

@danmarsden danmarsden changed the base branch from master to MOODLE_400_STABLE June 7, 2023 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants