Merge branch 'master' into discussions

Author: dopry

AUTHORS

@@ -102,6 +102,7 @@ Rodney Richardson
 Rustem Saiargaliev
 Rustem Saiargaliev
 Sandro Rodrigues
+Sean 'Shaleh' Perry
 Shaheed Haque
 Shaun Stanworth
 Sayyid Hamid Mahdavi

CHANGELOG.md

@@ -23,7 +23,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * Update token to TextField from CharField with 255 character limit and SHA-256 checksum in AbstractAccessToken model. Removing the 255 character limit enables supporting JWT tokens with additional claims
 * Update middleware, validators, and views to use token checksums instead of token for token retrieval and validation.
 * #1446 use generic models pk instead of id.
-* Bump oauthlib version to 3.2.0 and above
+* Transactions wrapping writes of the Tokens now rely on Django's database routers to determine the correct
+  database to use instead of assuming that 'default' is the correct one.
+* Bump oauthlib version to 3.2.2 and above
 * Update the OAuth2Validator's invalidate_authorization_code method to return an InvalidGrantError if the associated grant does not exist.
 
 ### Deprecated

README.rst

@@ -45,7 +45,7 @@ Requirements
 
 * Python 3.8+
 * Django 4.2, 5.0 or 5.1
-* oauthlib 3.2+
+* oauthlib 3.2.2+
 
 Installation
 ------------

docs/advanced_topics.rst

@@ -65,6 +65,17 @@ That's all, now Django OAuth Toolkit will use your model wherever an Application
     is because of the way Django currently implements swappable models.
     See `issue #90 <https://github.com/jazzband/django-oauth-toolkit/issues/90>`_ for details.
 
+Configuring multiple databases
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+There is no requirement that the tokens are stored in the default database or that there is a
+default database provided the database routers can determine the correct Token locations. Because the
+Tokens have foreign keys to the ``User`` model, you likely want to keep the tokens in the same database
+as your User model. It is also important that all of the tokens are stored in the same database.
+This could happen for instance if one of the Tokens is locally overridden and stored in a separate database.
+The reason for this is transactions will only be made for the database where AccessToken is stored
+even when writing to RefreshToken or other tokens.
+
 Multiple Grants
 ~~~~~~~~~~~~~~~
 

docs/contributing.rst

@@ -252,6 +252,26 @@ Open :file:`mycoverage/index.html` in your browser and you can see a coverage su
 
 There's no need to wait for Codecov to complain after you submit your PR.
 
+The tests are generic and written to work with both single database and multiple database configurations. tox will run
+tests both ways. You can see the configurations used in tests/settings.py and tests/multi_db_settings.py.
+
+When there are multiple databases defined, Django tests will not work unless they are told which database(s) to work with.
+For test writers this means any test must either:
+- instead of Django's TestCase or TransactionTestCase use the versions of those
+  classes defined in tests/common_testing.py
+- when using pytest's `django_db` mark, define it like this:
+  `@pytest.mark.django_db(databases=retrieve_current_databases())`
+
+In test code, anywhere the database is referenced the Django router needs to be used exactly like the package's code.
+
+.. code-block:: python
+
+    token_database = router.db_for_write(AccessToken)
+    with self.assertNumQueries(1, using=token_database):
+        # call something using the database
+
+Without the 'using' option, this test fails in the multiple database scenario because 'default' will be used instead.
+
 Code conventions matter
 -----------------------
 

docs/index.rst

@@ -23,7 +23,7 @@ Requirements
 
 * Python 3.8+
 * Django 4.2, 5.0 or 5.1
-* oauthlib 3.2+
+* oauthlib 3.2.2+
 
 Index
 =====

docs/requirements.txt

@@ -1,5 +1,5 @@
 Django
-oauthlib>=3.2.0
+oauthlib>=3.2.2
 m2r>=0.2.1
 mistune<2
 sphinx==7.2.6

oauth2_provider/apps.py

@@ -4,3 +4,7 @@
 class DOTConfig(AppConfig):
     name = "oauth2_provider"
     verbose_name = "Django OAuth Toolkit"
+
+    def ready(self):
+        # Import checks to ensure they run.
+        from . import checks  # noqa: F401

oauth2_provider/checks.py

@@ -0,0 +1,28 @@
+from django.apps import apps
+from django.core import checks
+from django.db import router
+
+from .settings import oauth2_settings
+
+
+@checks.register(checks.Tags.database)
+def validate_token_configuration(app_configs, **kwargs):
+    databases = set(
+        router.db_for_write(apps.get_model(model))
+        for model in (
+            oauth2_settings.ACCESS_TOKEN_MODEL,
+            oauth2_settings.ID_TOKEN_MODEL,
+            oauth2_settings.REFRESH_TOKEN_MODEL,
+        )
+    )
+
+    # This is highly unlikely, but let's warn people just in case it does.
+    # If the tokens were allowed to be in different databases this would require all
+    # writes to have a transaction around each database. Instead, let's enforce that
+    # they all live together in one database.
+    # The tokens are not required to live in the default database provided the Django
+    # routers know the correct database for them.
+    if len(databases) > 1:
+        return [checks.Error("The token models are expected to be stored in the same database.")]
+
+    return []

oauth2_provider/models.py

@@ -2,14 +2,15 @@
 import logging
 import time
 import uuid
+from contextlib import suppress
 from datetime import timedelta
 from urllib.parse import parse_qsl, urlparse
 
 from django.apps import apps
 from django.conf import settings
 from django.contrib.auth.hashers import identify_hasher, make_password
 from django.core.exceptions import ImproperlyConfigured
-from django.db import models, transaction
+from django.db import models, router, transaction
 from django.urls import reverse
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
@@ -512,17 +513,19 @@ def revoke(self):
         Mark this refresh token revoked and revoke related access token
         """
         access_token_model = get_access_token_model()
+        access_token_database = router.db_for_write(access_token_model)
         refresh_token_model = get_refresh_token_model()
-        with transaction.atomic():
+
+        # Use the access_token_database instead of making the assumption it is in 'default'.
+        with transaction.atomic(using=access_token_database):
             token = refresh_token_model.objects.select_for_update().filter(pk=self.pk, revoked__isnull=True)
             if not token:
                 return
             self = list(token)[0]
 
-            try:
-                access_token_model.objects.get(pk=self.access_token_id).revoke()
-            except access_token_model.DoesNotExist:
-                pass
+            with suppress(access_token_model.DoesNotExist):
+                access_token_model.objects.get(id=self.access_token_id).revoke()
+
             self.access_token = None
             self.revoked = timezone.now()
             self.save()
@@ -655,7 +658,7 @@ def get_access_token_model():
 
 
 def get_id_token_model():
-    """Return the AccessToken model that is active in this project."""
+    """Return the IDToken model that is active in this project."""
     return apps.get_model(oauth2_settings.ID_TOKEN_MODEL)