diff --git a/cbreader/settings/base.py b/cbreader/settings/base.py index 34ce601..3e7b018 100644 --- a/cbreader/settings/base.py +++ b/cbreader/settings/base.py @@ -231,11 +231,24 @@ REST_FRAMEWORK = { ], 'DEFAULT_AUTHENTICATION_CLASSES': ( 'rest_framework_simplejwt.authentication.JWTAuthentication', + 'comic.auth.CookieJWTAuthentication', 'rest_framework.authentication.SessionAuthentication', ) } CORS_ALLOW_ALL_ORIGINS = True +# Cookie based JWT authentication (see comic/auth.py). Tokens are stored in HttpOnly +# cookies so that browser initiated requests, such as the page images in the reader, +# can be authenticated - an tag cannot send an Authorization header. +JWT_ACCESS_COOKIE_NAME = os.getenv('JWT_ACCESS_COOKIE_NAME', 'access_token') +JWT_REFRESH_COOKIE_NAME = os.getenv('JWT_REFRESH_COOKIE_NAME', 'refresh_token') +JWT_COOKIE_PATH = os.getenv('JWT_COOKIE_PATH', '/') +# Only mark the cookies Secure when the site is actually served over HTTPS, otherwise +# browsers drop them on plain http. +JWT_COOKIE_SECURE = os.getenv('JWT_COOKIE_SECURE', 'False') == 'True' +# Lax blocks cross-site POSTs, which is the CSRF protection for cookie authentication. +JWT_COOKIE_SAMESITE = os.getenv('JWT_COOKIE_SAMESITE', 'Lax') + SIMPLE_JWT = { "ROTATE_REFRESH_TOKENS": True, "BLACKLIST_AFTER_ROTATION": True, diff --git a/cbreader/urls.py b/cbreader/urls.py index df51409..b0d45e5 100644 --- a/cbreader/urls.py +++ b/cbreader/urls.py @@ -25,9 +25,7 @@ from rest_framework import permissions from rest_framework.routers import DefaultRouter # from rest_framework_extensions.routers import ExtendedDefaultRouter -from rest_framework_simplejwt.views import TokenBlacklistView, TokenObtainPairView, TokenRefreshView - -from comic import feeds, rest +from comic import auth_views, feeds, rest schema_view = get_schema_view( openapi.Info( @@ -61,9 +59,9 @@ urlpatterns = [ re_path(r'^swagger\.(?Pjson|yaml)$', schema_view.without_ui(cache_timeout=0), name='schema-json'), re_path(r'^swagger/$', schema_view.with_ui('swagger', cache_timeout=0), name='schema-swagger-ui'), re_path(r'^redoc/$', schema_view.with_ui('redoc', cache_timeout=0), name='schema-redoc'), - path('api/token/', TokenObtainPairView.as_view(), name='token_obtain_pair'), - path('api/token/refresh/', TokenRefreshView.as_view(), name='token_refresh'), - path('api/token/blacklist/', TokenBlacklistView.as_view(), name='token_blacklist'), + path('api/token/', auth_views.CookieTokenObtainPairView.as_view(), name='token_obtain_pair'), + path('api/token/refresh/', auth_views.CookieTokenRefreshView.as_view(), name='token_refresh'), + path('api/token/blacklist/', auth_views.CookieTokenBlacklistView.as_view(), name='token_blacklist'), path('api/', include(router.urls)), path("", TemplateView.as_view(template_name="application.html"), diff --git a/comic/auth.py b/comic/auth.py new file mode 100644 index 0000000..8ff11f4 --- /dev/null +++ b/comic/auth.py @@ -0,0 +1,66 @@ +"""Cookie based JWT authentication. + +Tokens are kept in HttpOnly cookies rather than in JavaScript storage. That is +what makes browser initiated requests work: an `` tag cannot send an +`Authorization` header, so protected page images can only be served through +cookie authentication. + +The views live in comic/auth_views.py: importing rest_framework.views here would +resolve DEFAULT_AUTHENTICATION_CLASSES while this module is still being imported. +""" + +from typing import Any + +from django.conf import settings +from rest_framework.request import Request +from rest_framework.response import Response +from rest_framework_simplejwt.authentication import AUTH_HEADER_TYPES, JWTAuthentication + + +def access_cookie_name() -> str: + return settings.JWT_ACCESS_COOKIE_NAME + + +def refresh_cookie_name() -> str: + return settings.JWT_REFRESH_COOKIE_NAME + + +def _cookie_options() -> dict[str, Any]: + return { + 'httponly': True, + 'secure': settings.JWT_COOKIE_SECURE, + 'samesite': settings.JWT_COOKIE_SAMESITE, + 'path': settings.JWT_COOKIE_PATH, + } + + +def set_auth_cookies(response: Response, tokens: dict[str, Any]) -> None: + """Write the access and refresh tokens into HttpOnly cookies.""" + simple_jwt = settings.SIMPLE_JWT + for name, token, lifetime in ( + (access_cookie_name(), tokens.get('access'), simple_jwt['ACCESS_TOKEN_LIFETIME']), + (refresh_cookie_name(), tokens.get('refresh'), simple_jwt['REFRESH_TOKEN_LIFETIME']), + ): + if token: + response.set_cookie(name, token, max_age=int(lifetime.total_seconds()), **_cookie_options()) + + +def clear_auth_cookies(response: Response) -> None: + for name in (access_cookie_name(), refresh_cookie_name()): + response.delete_cookie(name, path=settings.JWT_COOKIE_PATH) + + +class CookieJWTAuthentication(JWTAuthentication): + """Authenticate from the Authorization header, or from the access cookie. + + Keeping the header path means API clients that send a bearer token still work. + """ + + def get_header(self, request: Request) -> bytes | None: + header = super().get_header(request) + if header: + return header + token = request.COOKIES.get(access_cookie_name()) + if not token: + return None + return f'{AUTH_HEADER_TYPES[0]} {token}'.encode() diff --git a/comic/auth_views.py b/comic/auth_views.py new file mode 100644 index 0000000..68514e4 --- /dev/null +++ b/comic/auth_views.py @@ -0,0 +1,72 @@ +"""Token views that store JWTs in HttpOnly cookies (see comic/auth.py).""" + +from rest_framework import status +from rest_framework.exceptions import AuthenticationFailed +from rest_framework.request import Request +from rest_framework.response import Response +from rest_framework_simplejwt.exceptions import TokenError +from rest_framework_simplejwt.serializers import TokenBlacklistSerializer, TokenRefreshSerializer +from rest_framework_simplejwt.views import TokenObtainPairView + +from comic.auth import clear_auth_cookies, refresh_cookie_name, set_auth_cookies + + +def _refresh_value(request: Request) -> str | None: + """The refresh token from the request body, or from the refresh cookie.""" + value = request.data.get('refresh') if isinstance(request.data, dict) else None + return value or request.COOKIES.get(refresh_cookie_name()) + + +class CookieTokenObtainPairView(TokenObtainPairView): + """Log in: issue tokens and store them in HttpOnly cookies.""" + + def post(self, request: Request, *args: object, **kwargs: object) -> Response: + response = super().post(request, *args, **kwargs) + if response.status_code == status.HTTP_200_OK and isinstance(response.data, dict): + set_auth_cookies(response, response.data) + # The tokens live in HttpOnly cookies, so they are not returned to JavaScript. + response.data = {} + return response + + +class CookieTokenRefreshView(TokenObtainPairView): + """Refresh using the refresh cookie and update both cookies.""" + + serializer_class = TokenRefreshSerializer + + def post(self, request: Request, *args: object, **kwargs: object) -> Response: + refresh = _refresh_value(request) + if not refresh: + return Response(status=status.HTTP_401_UNAUTHORIZED, data={'errors': 'No refresh token.'}) + + serializer = self.get_serializer(data={'refresh': refresh}) + try: + serializer.is_valid(raise_exception=True) + except TokenError as exc: + raise AuthenticationFailed(exc.args[0]) from exc + + response = Response(status=status.HTTP_200_OK) + set_auth_cookies(response, serializer.validated_data) + # Keep the cookies out of the body so JavaScript never sees the tokens. + return response + + +class CookieTokenBlacklistView(TokenObtainPairView): + """Log out: blacklist the refresh cookie and clear both cookies.""" + + serializer_class = TokenBlacklistSerializer + + def post(self, request: Request, *args: object, **kwargs: object) -> Response: + refresh = _refresh_value(request) + if not refresh: + return Response(status=status.HTTP_401_UNAUTHORIZED, data={'errors': 'No refresh token.'}) + + serializer = self.get_serializer(data={'refresh': refresh}) + try: + serializer.is_valid(raise_exception=True) + except TokenError as exc: + raise AuthenticationFailed(exc.args[0]) from exc + + response = Response(status=status.HTTP_204_NO_CONTENT) + clear_auth_cookies(response) + return response diff --git a/comic/tests/test_auth.py b/comic/tests/test_auth.py new file mode 100644 index 0000000..793fc87 --- /dev/null +++ b/comic/tests/test_auth.py @@ -0,0 +1,87 @@ +"""Tests for cookie based JWT authentication.""" + +import json + +import pytest +from django.test import Client + +from comic import auth + +CREDENTIALS = {'username': 'reader', 'password': 'password12345'} + + +def _store_cookies(client: Client, response) -> None: + """Copy Set-Cookie headers into the test client, which does not do it itself.""" + for name, morsel in response.cookies.items(): + if morsel.value: + client.cookies[name] = morsel.value + elif name in client.cookies: + del client.cookies[name] + + +@pytest.mark.django_db +def test_login_sets_http_only_cookies(client: Client, reader_user) -> None: + response = client.post('/api/token/', CREDENTIALS) + assert response.status_code == 200 + # The tokens are not returned to JavaScript, only stored as cookies. + assert json.loads(response.content) == {} + for name in (auth.access_cookie_name(), auth.refresh_cookie_name()): + assert response.cookies[name].value + assert response.cookies[name]['httponly'] + + +@pytest.mark.django_db +def test_a_cookie_authenticates_a_request(client: Client, reader_user) -> None: + _store_cookies(client, client.post('/api/token/', CREDENTIALS)) + response = client.get('/api/account/') + assert response.status_code == 200 + assert json.loads(response.content)['username'] == reader_user.username + + +@pytest.mark.django_db +def test_anonymous_request_without_a_cookie_is_rejected(client: Client) -> None: + assert client.get('/api/account/').status_code == 401 + + +@pytest.mark.django_db +def test_the_authorization_header_still_works(client: Client, reader_user) -> None: + response = client.post('/api/token/', CREDENTIALS) + token = response.cookies[auth.access_cookie_name()].value + response = client.get('/api/account/', headers={'Authorization': f'Bearer {token}'}) + assert response.status_code == 200 + + +@pytest.mark.django_db +def test_refresh_rotates_the_access_cookie(client: Client, reader_user) -> None: + _store_cookies(client, client.post('/api/token/', CREDENTIALS)) + old_access = client.cookies[auth.access_cookie_name()].value + old_refresh = client.cookies[auth.refresh_cookie_name()].value + + response = client.post('/api/token/refresh/', {}) + assert response.status_code == 200 + _store_cookies(client, response) + assert client.cookies[auth.access_cookie_name()].value != old_access + assert client.cookies[auth.refresh_cookie_name()].value != old_refresh + + # The rotated refresh token is blacklisted, so the original one no longer works. + assert client.post('/api/token/refresh/', {'refresh': old_refresh}).status_code == 401 + + +@pytest.mark.django_db +def test_blacklisting_clears_the_cookies(client: Client, reader_user) -> None: + _store_cookies(client, client.post('/api/token/', CREDENTIALS)) + response = client.post('/api/token/blacklist/', {}) + assert response.status_code == 204 + _store_cookies(client, response) + + for name in (auth.access_cookie_name(), auth.refresh_cookie_name()): + assert name not in client.cookies + + assert client.get('/api/account/').status_code == 401 + + +@pytest.mark.django_db +def test_page_images_can_be_loaded_with_a_cookie(client: Client, book, reader_user) -> None: + """Browser initiated requests cannot send a header, so they rely on the cookie.""" + _store_cookies(client, client.post('/api/token/', CREDENTIALS)) + assert client.get(f'/api/read/{book.selector}/image/1/').status_code == 200 diff --git a/frontend/src/api/index.js b/frontend/src/api/index.js index 134688a..26dee67 100644 --- a/frontend/src/api/index.js +++ b/frontend/src/api/index.js @@ -1,47 +1,15 @@ import axios from "axios"; import router from "@/router"; import store from "@/store"; -import { jwtDecode } from "jwt-decode"; -/** - * Gets a valid access token or refreshes if needed - * Uses a consistent 5-minute threshold for token expiration - */ -async function get_access_token() { - // If we don't have tokens in the store, return null - if (!store.state.jwt || !store.state.jwt.access) { - return null; - } +// Authentication is cookie based: the browser sends the HttpOnly access token +// automatically, so there is no Authorization header to set here. +const axios_jwt = axios.create({ withCredentials: true }); - try { - const access = jwtDecode(store.state.jwt.access); - const now = Date.now() / 1000; - const refreshThreshold = 300; // 5 minutes in seconds - - // If token is about to expire, refresh it - if (access.exp - now < refreshThreshold) { - try { - // Wait for the token to refresh - await store.dispatch('refreshToken'); - return store.state.jwt.access; - } catch (error) { - console.error('Failed to refresh token:', error); - return null; - } - } - - return store.state.jwt.access; - } catch (error) { - console.error('Error decoding token:', error); - return null; - } -} - -const axios_jwt = axios.create(); - -// Add CSRF token to all requests if using cookies for authentication +// Add the CSRF header if a csrftoken cookie exists. Cookie authentication is +// protected against cross-site form posts by SameSite=Lax, so this is only used +// when the backend does set the cookie. axios_jwt.interceptors.request.use(function(config) { - // Get CSRF token from cookie if it exists const csrfToken = document.cookie .split('; ') .find(row => row.startsWith('csrftoken=')) @@ -54,26 +22,32 @@ axios_jwt.interceptors.request.use(function(config) { return config; }); -// Add JWT token to all requests -axios_jwt.interceptors.request.use(async function (config) { - const access_token = await get_access_token(); +// A 401 means the access cookie is no longer valid. Try to refresh it once; if +// that fails the session is gone, so log out and send the user to the login page. +axios_jwt.interceptors.response.use( + (response) => response, + async (error) => { + const config = error.config; - if (access_token) { - config.headers.Authorization = "Bearer " + access_token; - } else if (!router.currentRoute.value.fullPath.includes('login')) { - // Only redirect if we're not already on the login page - router.push({ - name: 'login', - query: { - next: router.currentRoute.value.fullPath, - error: 'Please log in to continue' + if (error.response?.status === 401 && config && !config._retried && !config.url.includes('/token/')) { + config._retried = true; + try { + await store.dispatch('refreshToken'); + return await axios_jwt.request(config); + } catch { + store.commit('logOut'); + router.push({ + name: 'login', + query: { + next: config.url, + error: 'Your session has expired. Please log in again.' + } + }); } - }); - } + } - return config; -}, function (error) { - return Promise.reject(error); -}); + return Promise.reject(error); + } +); export default axios_jwt diff --git a/frontend/src/components/TheNavbar.vue b/frontend/src/components/TheNavbar.vue index 0ab7230..38a662a 100644 --- a/frontend/src/components/TheNavbar.vue +++ b/frontend/src/components/TheNavbar.vue @@ -51,12 +51,12 @@ export default { }, computed: { isAuthenticated() { - return !!this.$store.state.jwt; + return this.$store.getters.isAuthenticated; } }, methods: { logout () { - store.commit('logOut') + store.dispatch('logout') router.push({name: 'login'}) } } diff --git a/frontend/src/main.js b/frontend/src/main.js index 98163e8..b18b6a0 100644 --- a/frontend/src/main.js +++ b/frontend/src/main.js @@ -21,9 +21,14 @@ library.add(faBook, faBookOpen, faEdit, faTurnUp, faTimes, faCheck) import router from './router' import store from './store' -Vue.createApp(App) +const app = Vue.createApp(App) .use(ToastPlugin) .use(store) .use(router) .component('font-awesome-icon', FontAwesomeIcon) - .mount('#app') + +// Validate the cookie before the first navigation, so the router guards see the +// real session instead of whatever is left in localStorage. +store.dispatch('restoreSession').finally(() => { + app.mount('#app') +}) diff --git a/frontend/src/router/index.js b/frontend/src/router/index.js index c490466..b17de6e 100644 --- a/frontend/src/router/index.js +++ b/frontend/src/router/index.js @@ -11,7 +11,7 @@ const HistoryView = () => import('@/views/HistoryView') // Navigation guard to check if user is authenticated function requireAuth(to, from, next) { - if (!store.state.jwt) { + if (!store.getters.isAuthenticated) { next({ name: 'login', query: { next: to.fullPath, error: 'Please log in to access this page' } @@ -23,7 +23,7 @@ function requireAuth(to, from, next) { // Navigation guard to check if user is admin function requireAdmin(to, from, next) { - if (!store.state.jwt || !store.getters.is_superuser) { + if (!store.getters.isAuthenticated || !store.getters.is_superuser) { next({ name: 'login', query: { next: to.fullPath, error: 'Admin access required' } diff --git a/frontend/src/store/index.js b/frontend/src/store/index.js index f4d0351..206be89 100644 --- a/frontend/src/store/index.js +++ b/frontend/src/store/index.js @@ -1,16 +1,11 @@ import { createStore } from 'vuex' -import axios from 'axios' -import { jwtDecode } from "jwt-decode"; import {useToast} from "vue-toast-notification"; import router from "@/router"; import api from "@/api"; -// We'll no longer use localStorage for tokens -// Instead, tokens will be stored in httpOnly cookies by the backend -// and automatically included in requests -function get_jwt_from_storage(){ - return null; // Initial state will be null until login -} +// Tokens live in HttpOnly cookies set by the backend, so JavaScript cannot read +// them. Only the user profile is kept in localStorage, to avoid a blank page +// before the first request has been made. function get_user_from_storage(){ try { return JSON.parse(localStorage.getItem('u')) @@ -19,9 +14,18 @@ function get_user_from_storage(){ } } +function session_expired_query(next){ + return { + name: 'login', + query: { + next: next, + error: 'Your session has expired. Please log in again.' + } + } +} + export default createStore({ state: { - jwt: get_jwt_from_storage(), filters: {}, user: get_user_from_storage(), classifications: [ @@ -39,23 +43,14 @@ export default createStore({ } else { return state.user.is_superuser } + }, + isAuthenticated (state) { + return state.user !== null } }, mutations: { - updateToken(state, newToken){ - // No longer storing tokens in localStorage - // Tokens are stored in httpOnly cookies by the backend - state.jwt = newToken; - }, logOut(state){ - // Clear user data from localStorage localStorage.removeItem('u') - // Clear state - - // Make a request to the backend to invalidate the token - axios.post('/api/token/blacklist/', { refresh: state.jwt?.refresh }) - .catch(error => console.error('Error blacklisting token:', error)); - state.jwt = null; state.user = null }, updateUser(state, userData){ @@ -69,95 +64,77 @@ export default createStore({ username: username, password: password } - axios.post('/api/token/', payload) - .then((response)=>{ - context.commit('updateToken', response.data); - api.get('/api/account').then(response => { - context.commit('updateUser', response.data) - }) - if ('next' in router.currentRoute.value.query) { - router.push(router.currentRoute.value.query.next) - } else { - router.push('browse') - } - - }) - .catch((error)=>{ - const $toast = useToast(); - if (error.response.data.detail) { - $toast.error(error.response.data.detail, {position:'top'}); - } - if (error.response.data.username) { - $toast.error("Username: " + error.response.data.username, {position:'top'}); - } - if (error.response.data.password) { - $toast.error("Password: " + error.response.data.password, {position:'top'}); - } - - }) - }, - refreshToken(){ - // Don't attempt to refresh if we don't have a token - if (!this.state.jwt || !this.state.jwt.refresh) { - return Promise.reject(new Error('No refresh token available')); - } - - const payload = { - refresh: this.state.jwt.refresh - } - - return axios.post('/api/token/refresh/', payload) + // The response body is empty: the backend stores the tokens as HttpOnly cookies. + api.post('/api/token/', payload) + .then(() => api.get('/api/account')) .then((response) => { - this.commit('updateToken', response.data); - return response.data; + context.commit('updateUser', response.data) + if ('next' in router.currentRoute.value.query) { + router.push(router.currentRoute.value.query.next) + } else { + router.push('browse') + } }) + .catch((error) => { + const $toast = useToast(); + const data = error.response?.data ?? {}; + if (data.detail) { + $toast.error(data.detail, {position:'top'}); + } + if (data.username) { + $toast.error("Username: " + data.username, {position:'top'}); + } + if (data.password) { + $toast.error("Password: " + data.password, {position:'top'}); + } + }) + }, + refreshToken(){ + if (!this.state.user) { + return Promise.reject(new Error('Not logged in')); + } + + // The refresh token is read from the cookie by the backend. + return api.post('/api/token/refresh/', {}) + .then((response) => response.data) .catch((error) => { console.error('Token refresh failed:', error); - // If refresh fails, log the user out and redirect to login this.commit('logOut'); - router.push({ - name: 'login', - query: { - next: router.currentRoute.value.fullPath, - error: 'Your session has expired. Please log in again.' - } - }); + router.push(session_expired_query(router.currentRoute.value.fullPath)); return Promise.reject(error); }); }, - inspectToken(){ - const token = this.state.jwt; - if (!token) return; - - try { - // For access token - const decoded = jwtDecode(token.access); - const exp = decoded.exp; - const now = Date.now() / 1000; - - // Refresh when token is within 5 minutes of expiring - const refreshThreshold = 300; // 5 minutes in seconds - - if (exp - now < refreshThreshold) { - // Token is about to expire, refresh it - this.dispatch('refreshToken'); - } else if (exp < now) { - // Token is already expired, force logout - this.commit('logOut'); - router.push({ - name: 'login', - query: { - next: router.currentRoute.value.fullPath, - error: 'Your session has expired. Please log in again.' - } - }); - } - } catch (error) { - console.error('Error inspecting token:', error); - // If we can't decode the token, log the user out - this.commit('logOut'); - router.push({name: 'login'}); + // Check that the cookie still identifies the user stored in localStorage. + restoreSession(){ + if (!this.state.user) { + return Promise.resolve(null); } + + return api.get('/api/account') + .then((response) => { + this.commit('updateUser', response.data); + return response.data; + }) + .catch((error) => { + if (error.response?.status === 401) { + this.commit('logOut'); + router.push({ name: 'login' }); + } + return null; + }); + }, + logout(){ + const wasLoggedIn = this.state.user !== null; + this.commit('logOut'); + + if (!wasLoggedIn) { + return Promise.resolve(null); + } + + return api.post('/api/token/blacklist/', {}) + .catch((error) => { + console.error('Error blacklisting token:', error); + }); } }, modules: { diff --git a/frontend/webpack.dev.js b/frontend/webpack.dev.js index 79bbd3f..9802b5d 100644 --- a/frontend/webpack.dev.js +++ b/frontend/webpack.dev.js @@ -3,6 +3,11 @@ const { VueLoaderPlugin } = require('vue-loader') const BundleTracker = require('webpack-bundle-tracker'); const webpack = require('webpack') +// The dev server port. Keep this and the publicPath in sync - django-webpack-loader +// builds the script URL from the publicPath written into webpack-stats.json. +const DEV_PORT = process.env.DEV_PORT || 8001 +const DEV_PUBLIC_PATH = `http://localhost:${DEV_PORT}/` + module.exports = () => { return { @@ -11,6 +16,7 @@ module.exports = () => { entry: path.resolve(__dirname, './src/main.js'), output: { path: path.resolve(__dirname, './dist/bundles/'), + publicPath: DEV_PUBLIC_PATH, }, module: { rules: [ @@ -47,7 +53,7 @@ module.exports = () => { new BundleTracker({ filename: 'webpack-stats.json', path: path.resolve(__dirname, './'), - publicPath: 'http://localhost:8080/' + publicPath: DEV_PUBLIC_PATH }), new webpack.DefinePlugin({ 'process.env.BASE_URL': JSON.stringify(process.env.BASE_URL), @@ -55,6 +61,7 @@ module.exports = () => { new webpack.DefinePlugin({ __VUE_OPTIONS_API__: true, __VUE_PROD_DEVTOOLS__: true }), ], devServer: { + port: DEV_PORT, headers: { "Access-Control-Allow-Origin":"*" },