Skip to content
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

Add authentication to routes #11

Closed
lukasbestle opened this issue Jun 27, 2016 · 7 comments
Closed

Add authentication to routes #11

lukasbestle opened this issue Jun 27, 2016 · 7 comments

Comments

@lukasbestle
Copy link
Contributor

The routes can currently be accessed without any authentication. I think you should make sure that a user is logged in and/or use HTTP basic auth for this.

@thathoff
Copy link
Owner

Hey! We actually thought about this but decided to go without authentication because the hooks don’t disclose any information and it should be totally safe to call them externally.

The only thing I can think of is that someone might try to DOS your server by executing the hook multiple times. Perhaps the easiest solution is to deny access from all IPs except localhost (or the server which runs the cron) via .htaccess or the webserver’s configuration.

@lukasbestle
Copy link
Contributor Author

It doesn't disclose information but it runs code that shouldn't be run by anonymous visitors.

My example is that pushing the content automatically triggers a deployment, so I need to control when pushes happen. That's why I decided to use version 1.0.0 of the plugin for now.

@thathoff
Copy link
Owner

thathoff commented Jun 27, 2016

Ah, ok. That’s a really good reason. Do you think it’s enough to implement an IP check? For example a configuration option like c::set('gcapc-allowed-ips', ["::1", "127.0.0.1", "1.2.3.4"]);.

This should be really easy to implement.

@lukasbestle
Copy link
Contributor Author

For me it would be enough to completely disable the route actually. Maybe an option like c::set('gcapc-routes', false);?

@thathoff
Copy link
Owner

OK! I’ve just created another issue (#12) for this. I think an IP check is a good option to so I leave this issue open, too.

Thanks for your input! :)

@CHE1RON
Copy link
Contributor

CHE1RON commented Mar 19, 2023

Hey there,
as this issue is quite old and seems to date back to Kirby v2, I'm eager to know if this got implemented in newer versions?

@thathoff
Copy link
Owner

Hi, yes you can set the cronHooksSecret config option. In this case the push/pull routes need an additional secret get parameter.

I don’t know why I kept this issue open 🧐🤣 maybe i thought about implementing Basis-Auth or something like this. Will close it for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants