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

Create api #322

Merged
merged 25 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
85b8f65
chore: install Pydantic
Jenselme Nov 20, 2024
6d7f9e9
ref(wallabag): switch to Pydantic for validation
Jenselme Nov 20, 2024
8e3a3b2
ref(validators): switch to Pydantic for models JSON validation
Jenselme Nov 20, 2024
653ef30
chore: remove jsonschema
Jenselme Nov 20, 2024
38bc3a5
ref: switch to Pydantic when validating feed or article data
Jenselme Nov 23, 2024
d0f52e5
fix: correct our usage of Pydantic
Jenselme Nov 23, 2024
5aa17de
chore: correct app template
Jenselme Nov 23, 2024
90de7d4
chore: install django-ninja
Jenselme Nov 23, 2024
5d127dc
feat(api): setup API for reading app
Jenselme Nov 23, 2024
8e9399b
feat(api): setup API to create articles
Jenselme Nov 23, 2024
51ff5fd
fix(article): correct list data concatenation when updating article
Jenselme Nov 23, 2024
b8cbbd6
chore: add a datetime-local widget
Jenselme Nov 23, 2024
66b1c03
feat(timezone): improve model behavior
Jenselme Nov 23, 2024
9831340
feat(users): allow users to manage application tokens
Jenselme Nov 23, 2024
68e70f1
chore: install PyJWT
Jenselme Nov 23, 2024
d2b8cfe
feat(api): setup login with tokens
Jenselme Nov 23, 2024
7fcb210
feat(api): make API async
Jenselme Nov 23, 2024
609d24a
test(api): add tests for untested API related code
Jenselme Nov 24, 2024
302bdb6
test(feeds): check that articles of a feed are not deleted when feed is
Jenselme Nov 24, 2024
3508333
feat(api): can manipulate articles
Jenselme Nov 24, 2024
8a102b7
feat(api): can manipulate feeds and feed categories
Jenselme Nov 24, 2024
f9982ae
doc(api): document users views
Jenselme Nov 26, 2024
2b987ef
test(feed): stabilize some tests
Jenselme Nov 26, 2024
cadb689
chore(adr): add an ADR for the API
Jenselme Nov 28, 2024
5a423ab
chore: prevent issue with shellcheck on env files
Jenselme Nov 28, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions .app-template/urls.py-tpl
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
from django.urls import path

from . import views
from django.urls import URLPattern

app_name = "{{ app_name }}"

urlpatterns = [
urlpatterns: list[URLPattern] = [
]
2 changes: 2 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,14 @@ repos:
rev: v1.36.1
hooks:
- id: djlint-reformat-django
exclude: "ninja/swagger.html"
- id: djlint-django

- repo: https://github.com/shellcheck-py/shellcheck-py
rev: v0.10.0.1
hooks:
- id: shellcheck
exclude: ".envrc"
args: [-e, SC1091]

- repo: https://github.com/thibaudcolas/pre-commit-stylelint
Expand Down
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
# Changelog

## 24.11.1

- Reduced allow times in which daily updates are run. We still support bi-hourly cron runs.
- Display a contact email to all authenticated users.
- Add an API:
- The documentation is available at `/api/docs/`.
- You can manage application tokens in your profile.
- You can get auth tokens from these applications tokens to use the API.

## 24.10.3

- Correct display of titles with HTML entities when adding an article.
Expand Down
11 changes: 11 additions & 0 deletions config/api.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
from ninja import NinjaAPI
from ninja.security import django_auth

from legadilo.feeds.api import feeds_api_router
from legadilo.reading.api import reading_api_router
from legadilo.users.api import AuthBearer, users_api_router

api = NinjaAPI(title="Legadilo API", auth=[django_auth, AuthBearer()], docs_url="/docs/")
api.add_router("reading/", reading_api_router)
api.add_router("feeds/", feeds_api_router)
api.add_router("users/", users_api_router)
12 changes: 12 additions & 0 deletions config/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import concurrent
import warnings
from datetime import timedelta
from pathlib import Path

import asgiref
Expand Down Expand Up @@ -113,6 +114,7 @@
"django.forms",
]
THIRD_PARTY_APPS = [
"ninja",
"django_version_checks",
"extra_checks",
"anymail",
Expand Down Expand Up @@ -607,8 +609,18 @@ def before_send_to_sentry(event, hint):
print("Failed to import sentry_sdk") # noqa: T201 print found


# django-ninja
# ------------------------------------------------------------------------------
# See https://django-ninja.dev/reference/settings/
NINJA_PAGINATION_MAX_LIMIT = 500
NINJA_PAGINATION_CLASS = "legadilo.utils.pagination.LimitOffsetPagination"


# Your stuff...
# ------------------------------------------------------------------------------
ARTICLE_FETCH_TIMEOUT = env.int("LEGADILO_ARTICLE_FETCH_TIMEOUT", default=50)
RSS_FETCH_TIMEOUT = env.int("LEGADILO_RSS_FETCH_TIMEOUT", default=300)
CONTACT_EMAIL = env.str("LEGADILO_CONTACT_EMAIL", default=None)
TOKEN_LENGTH = 50
JWT_ALGORITHM = "HS256"
JWT_MAX_AGE = timedelta(hours=4)
3 changes: 3 additions & 0 deletions config/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
from django.urls import include, path
from django.views import defaults as default_views

from config.api import api


def _correct_admin_url(path: str) -> str:
path = path.removeprefix("/")
Expand All @@ -26,6 +28,7 @@ def _correct_admin_url(path: str) -> str:
path("feeds/", include("legadilo.feeds.urls", namespace="feeds")),
path("reading/", include("legadilo.reading.urls", namespace="reading")),
path("import-export/", include("legadilo.import_export.urls", namespace="import_export")),
path("api/", api.urls),
] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT)


Expand Down
103 changes: 103 additions & 0 deletions docs/adrs/0007-api.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
# 7 - API

* **Date:** 2024-10-26
* **Status:** Accepted

## Context

I’d like to build a browser extension to save articles and subscribe to feeds more easily directly on the page we are on.
I think it’s a pretty common feature of feed aggregators and links savers (mostly links savers to be honest).
We have two options:
* Build a proper API and make the browser extension use this API:
* We have several possibilities to help us build the backend.
DRF and Django Ninja being the two real pretenders.
Since we have only a simple feature set and little times, I think Django Ninja is a better fit here: it’s easier to use and should allow us to develop the API faster.
It’s also async compatible out of the box and based on Pydantic (a package I use at work) which we can reuse for other validation!
* We will have to develop the API and dig a bit into how to do it properly with Django Ninja.
* It will unlok other possibilities in the long run in how to use the app (mobile app, integration with 3rd parties…).
* It should be easier to authenticate to the app: we can manage authentication differently and let the user configure the extension with an application token.
We could let the cookies be transmitted and rely on that (not sure how it will behave though).
And it makes the extension very tied to the connection to the app in the browser.
Whereas normal flow in this case is to never be disconnected.
Handling connection with MFA might be a problem too: we can’t display much in the extension and may have to redirect the user the app anyway to login.
That would be a very poor UX.
* It should also be easier to post and retrieve data to manipulate it as part of the browser extension.
* Call the standard views (ie the views that powers the app):
* We will have to adapt how we manage CSRF tokens to be able to supply them to our views.
It’s doable, I’ve done it in the past, but I always disliked it.
* We will have to post data as form data.
Not the cleanest way, but manageable.
Having a view that accepts both JSON and form is too much of a hassle in bare Django for me to do that.
And if I’m not building an API, there isn’t really a point into bringing a package just for that.
* We will manipulate HTML.
It may ease display (but I don’t think we will have anything fancy to display) at the cost of harder manipulations.
And we won’t be able to use the "normal" templates since we won’t have the same display.
This implies to make some views or templates even more complex.

I think it’s worth trying to develop the API and see how it goes.
If it’s not too much work, I think we should commit to it.

See:
* https://github.com/Jenselme/legadilo/issues/318
* https://github.com/Jenselme/legadilo/issues/320
* https://github.com/Jenselme/legadilo/issues/156


## Decisions

The test is a success and I think I achieved something good.
Let’s commit the API with Ninja!

### Tokens and API protection

Auth: Django Ninja doesn’t provide anything out of the box to build an auth with tokens.
It does however allow us to protect endpoints, routers or the full API with authentication.
It also gives us the basic tooling to fetch a token from the `Authorization` header and validate it.
If it’s valid, access is granted to the endpoint, if not the user gets an error message.
Django Ninja also allows us to have a list of authentication methods to use, so we can use token based auth for the extension and cookie auth as usual to try stuff in the browser (and in tests).

How to create tokens to access the API?
* We could create them from username and password.
But as part of the extension, this would involve to store them as part of the configuration.
I don’t think it’s desirable.
It would also make working with MFA harder.
And if the password needs to be changed, it would impact all consumers of the API.
* I think it’s safer to have the user create application "passwords" like in many other apps and create the access tokens from that.
These applications passwords would act as a refresh token.
* These passwords won’t be transmitted much over the network: we will transmit them only to get an access token.
They can be revoked easily from the user profile without impacting anything else.
We should be able to give them an expiration date just in case.
* They may be transmitted a bit too much and I may be relying too much on my knowledge to build this.
Not a problem right now, but I’ll have to recheck all this when I have more time.
Right now, I think it’s safe enough for myself.

### Summary

* We will build the API with Django Ninja.
* We will authenticate to the API with application tokens that will be used to create an actual access token usable to access the API.
They will:
* Be a long random string stored in the database generated with the `secrets` module.
* Be visible to the user only at creation to prevent stealing.
* Have an optional expiration date.
If unset, they are valid until manually deleted.
* The access tokens:
* Will be relatively short-lived to prevent attacks if it leaks.
* Will be in the JWT format and will contain the name of the application token used to generate it (mostly for debug purpose) and will contain the id of the user to use.
This will enable us to store and validate the token using well a known format with well known libraries.


## Consequences

* Let’s use Pydantic instead of JSON schemas to validate our JSON model fields and the data we read from external sources.
This will help us limit the number of libraries we use.
At this stage, I think Pydantic is a safe choice: it’s well known and maintained.
It’s also used a lot in the community nowadays and has become very popular.
* The API won’t allow all capabilities in the first time to gain time.
We will develop first and foremost what we need for the extension.
* We already unlock some API usage for everybody!
We will improve it later as part of https://github.com/Jenselme/legadilo/issues/320.
* The API will be documented more or less automatically and browsable thanks to Swagger.
* We should dig further to make sure our model is secured.
This is not a problem *right now* since we don’t have users, but can become in the future.
I’m mostly thinking of [Wallabag](https://doc.wallabag.org/en/developer/api/oauth) which has a different way to handle tokens.
This is logged here: https://github.com/Jenselme/legadilo/issues/325
4 changes: 4 additions & 0 deletions legadilo/core/forms/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,7 @@ def format_value(self, value):
return value
except (JSONDecodeError, ValueError, TypeError):
return super().format_value(value)


class DateTimeWidget(widgets.DateTimeInput):
input_type = "datetime-local"
5 changes: 5 additions & 0 deletions legadilo/core/models/timezone.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.

from typing import TYPE_CHECKING
from zoneinfo import ZoneInfo

from django.db import models

Expand Down Expand Up @@ -45,3 +46,7 @@ def __str__(self):

def __repr__(self):
return f"Timezone(name={self.name})"

@property
def zone_info(self) -> ZoneInfo:
return ZoneInfo(self.name)
Loading