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

RestAPI installer integrated #1519

Merged
merged 51 commits into from
Mar 9, 2024
Merged

RestAPI installer integrated #1519

merged 51 commits into from
Mar 9, 2024

Conversation

NL-TCH
Copy link
Contributor

@NL-TCH NL-TCH commented Feb 9, 2024

Hi,

I am creating this PR because:

  • all the GET requests are in place
  • i am getting really busy next months
  • i am a little burned out on this project :)

Feel free to create more post request functions etc...

Gr. TCH

*this is also tested in docker
/resolves #1498 #1507 #1372 #602 #393

api/modules/ap.py Fixed Show fixed Hide fixed
api/modules/ap.py Fixed Show fixed Hide fixed
api/modules/ap.py Fixed Show fixed Hide fixed
api/modules/ap.py Fixed Show fixed Hide fixed
api/modules/ap.py Fixed Show fixed Hide fixed
api/modules/ap.py Fixed Show fixed Hide fixed
api/modules/client.py Fixed Show fixed Hide fixed
api/modules/client.py Fixed Show fixed Hide fixed
api/modules/openvpn.py Fixed Show fixed Hide fixed
api/modules/wireguard.py Fixed Show fixed Hide fixed
@billz
Copy link
Member

billz commented Feb 9, 2024

What's your take on the CodeQL scan results?
Should shlex.quote() be used to sanitize arguments when specifying shell=True?

@NL-TCH
Copy link
Contributor Author

NL-TCH commented Feb 9, 2024

What's your take on the CodeQL scan results? Should shlex.quote() be used to sanitize arguments when specifying shell=True?

Yeah that would sanitize the input, good for security!

@NL-TCH
Copy link
Contributor Author

NL-TCH commented Feb 9, 2024

Things to do before merge:

  • Sanitize input of API (shlex.quote())
  • Create documentation

Things to improve after merge:

  • Add "insider" tags for paths like firewall and ddns
  • Put more paths behind auth module for secret info (like /ap that contains the wifi password) (maybe everything has to be put behind auth for overall security)
  • Add more POST functions, for DDNS, DHCP, DNS
  • Add docker option --rest 1 to auto install api on docker install
  • Expose port 8081 in dockerfile for external API access

@NL-TCH NL-TCH marked this pull request as draft February 9, 2024 21:08
@NL-TCH NL-TCH marked this pull request as ready for review February 17, 2024 19:59
@NL-TCH
Copy link
Contributor Author

NL-TCH commented Feb 17, 2024

@billz checks now pass, everything is placed behind api auth, and post calls are removed.
can you provide the documentation + gui part for enabling, disabling and setting the api key?

@billz
Copy link
Member

billz commented Feb 18, 2024

can you provide the documentation + gui part for enabling, disabling and setting the api key?

@NL-TCH absolutely! this is the least I can do 😅 excellent work in this PR

@billz
Copy link
Member

billz commented Mar 7, 2024

Got it working.

  1. The restapi.service file specified User=root while the python modules are installed under the current user.
  2. Adding dotenv as a dependency permitting reading of .env file.
  3. main.py raised exceptions with each @app.get ... api_key. Fixed these (version related?)
  4. Opted to move, rather than copy, /api to /etc/raspap to reduce confusion for users.

Service loads and API is fully functional. Some minor work left on the UI side to read/set API key.

api/modules/wireguard.py Fixed Show fixed Hide fixed
api/modules/wireguard.py Fixed Show fixed Hide fixed
api/modules/wireguard.py Fixed Show resolved Hide resolved
@billz
Copy link
Member

billz commented Mar 9, 2024

@NL-TCH feature branch installs cleanly 🎉

restapi-settings

With the last commits I think we're good to merge.

@NL-TCH
Copy link
Contributor Author

NL-TCH commented Mar 9, 2024

amazing work @billz !
Sorry i was not available for helping out on python code and for testing.

Do you have a link for the docs so that i can check them as well?

@billz
Copy link
Member

billz commented Mar 9, 2024

@NL-TCH doc link is fastapi's autogenerated docs at [host]:8081/docs. This could probably be clarified.

you took the lead on this, so kudos is all yours :)

@NL-TCH
Copy link
Contributor Author

NL-TCH commented Mar 9, 2024

@NL-TCH doc link is fastapi's autogenerated docs at [host]:8081/docs. This could probably be clarified.

you took the lead on this, so kudos is all yours :)

Smart! yeah that might be self explanatory
I'm down to merge 🥳

@billz billz merged commit 2de012c into RaspAP:master Mar 9, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

FEATURE REQUEST: Rest API for AP management
2 participants