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

Get records API route #178

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Get records API route #178

wants to merge 9 commits into from

Conversation

qdm12
Copy link
Owner

@qdm12 qdm12 commented Mar 15, 2021

  • JSON marshal settings for each provider
  • Setup route GET /api/v1/records
  • Unit test for HTTP handler with an example record
  • Document API routes in readme for now. Maybe add swagger later ❓
  • Add auth as this introduces a security hole otherwise. Force auth to access this route

Working on a (Flutter 2) UI to consume it, that should be a first step to have a 'dynamic' new UI with the same features as the current one.

case "", "application/json":
contentType = "application/json"
default:
httpError(w, http.StatusBadRequest, `content type "`+accept+`" is not supported`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

should you not return here?

Copy link
Owner Author

Choose a reason for hiding this comment

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

No, this is just sending an http response as json {"error": "message"} with the status code given 😉
I'll get to authentication soon to merge that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

fair point 🤣. I reviewed the rest, all seem alright to me. I learnt about Read Locks, it seemed weird to me to have to lock in order to read. Regarding authentication, a cool thing could be to make ddns-updater a microservice that implements oauth2. We could interface it with the user management interface we want, we wouldn't have to maintain user management. I'm personally using Authelia which has oauth2 in beta. There is also https://github.com/netlify/gotrue

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.

2 participants