Fix issue #5 - add an option to prevent auth for all users if ip fail… #7
Conversation
|
Hi! I took a quick look at it. It seems promising. Since it is a major upgrade, I need to take a close revision of the code, and check that it works correspondignly. After accepting this PR, maintenance will be addressed by us, so we need to be sure about it. In addition, we have a lot of work in our job, so maybe I delay a bit about its revision and comments. I'll be asap for this, for sure. Thank you a lot for your contribution! Jordi |
|
No probs :) Thanks, |
… ip fails whitelist.
|
hi @jpahullo, One other thing, we add travis CI support to all our public plugins, we have done this in our fork here: https://travis-ci.org/CatalystIT-AU/moodle-auth_ip/branches We usually also put a badge into the readme file showing it's status and linking into the build page, but in this case it should point to your travis page not ours. It's as easy as just signing up, logging into travis and turning it on, and then pushing any commit to trigger it. See this moodle forum: https://moodle.org/mod/forum/discuss.php?d=323384 The badge should go in the very first line of the readme and look like this once you've got travis working: For an example of what this looks like see any of our other plugins eg: https://github.com/CatalystIT-AU/moodle-auth_saml2/blob/master/README.md Lastly if you are struggling with time, we'd be open to taking on maintenance, or be co-maintainers, of this plugin. We have a vested interest in making sure it's current and up to date. Thanks heaps again @jpahullo :) |
|
Hi! Thanks a lot for your contribution @dmitriim and @brendanheywood. Sorry for being so late. Do you think you could make a rebase onto last master branch? Regarding to travis, we have added last github actions for moodle plugins, and also for releasing it. If you think it would be interesting to add a badge yet, let me know. Hoping it would be interesting for you too. |
|
About being co-maintainers, I personally like the idea. But, I have to talk about it with my unit and boss. I'll let you know. |
|
hi @jpahullo I honestly can't even remember working on this or why we wanted it :/ I've just checked and we aren't using this plugin anywhere in our fleet. So sorry but pursuing this is very low on our agenda. Between the ip restrictions in core and tool_mfa I'd guess this plugin is close to redundant these days. |
|
Hi! Thanks for answering. Yes, we have also it installed but not really using it. Thanks. If you let us, we’ll see to rebase your changes if we think this could help us somehow. Kind regards, Jordi |
|
Absolutely go for it |
Hi @jpahullo,
Please find in this PR changes related to issue #5. Brandan has already reviewed it and he is pretty happy about the code.
Cheers,
Dmitrii