search

SREd-URV / moodle-auth_ip

Manual authentication plugin that prevents access to your Moodle from different IPs than those allowed.
GNU General Public License v3.0
1 stars 6 forks source link

Fix issue #5 - add an option to prevent auth for all users if ip fail… #7

Open dmitriim opened 8 years ago

dmitriim commented 8 years ago

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

jpahullo commented 8 years ago

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

dmitriim commented 8 years ago

No probs :)

Thanks, Dmitrii

brendanheywood commented 8 years ago

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:

[![Build Status](https://travis-ci.org/SREd-URV/moodle-auth_ip.svg?branch=master)](https://travis-ci.org/SREd-URV/moodle-auth_ip)

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 :)

jpahullo commented 3 years ago

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.

jpahullo commented 3 years ago

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.

brendanheywood commented 3 years ago

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.

jpahullo commented 3 years ago

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

brendanheywood commented 3 years ago

Absolutely go for it