Skip to content

Fix refresh token max length (occurs with Microsoft OAuth) #1601

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.

Sign up for GitHub

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

Open
richardARPANET wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix refresh token max length (occurs with Microsoft OAuth) #1601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 17 additions & 0 deletions oauth2_provider/migrations/0013_alter_refreshtoken_token.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Generated by Django 5.1.2 on 2025-10-12 00:00

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("oauth2_provider", "0012_add_token_checksum"),
]

operations = [
migrations.AlterField(
model_name="refreshtoken",
name="token",
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

This migration locks in the new length, but there’s no contextual hint for future maintainers about why 8000 is the chosen bound (vs. using an unbounded text-like field). Consider adding a brief comment in the model (not the migration) explaining the rationale (e.g., known provider token sizes / why not TextField) so the next schema change doesn’t require re-discovering the same provider constraints.

Suggested change
name="token",
name="token",
# Use a bounded CharField instead of TextField so the token remains indexable
# in common databases; 8000 chars comfortably covers known provider token sizes.

Copilot uses AI. Check for mistakes.
field=models.CharField(max_length=8000),
),
]
2 changes: 1 addition & 1 deletion oauth2_provider/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ class AbstractRefreshToken(models.Model):
user = models.ForeignKey(
settings.AUTH_USER_MODEL, on_delete=models.CASCADE, related_name="%(app_label)s_%(class)s"
)
token = models.CharField(max_length=255)
token = models.CharField(max_length=8000)
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

Using a very large CharField(max_length=8000) can have cross-database operational downsides (e.g., index-length limitations on some MySQL/MariaDB setups if this column is or becomes indexed/unique; also potentially larger on-row storage and slower comparisons). If lookups are supported via the existing checksum mechanism (note the 0012_add_token_checksum dependency in this PR), consider switching token to a TextField for storage and ensuring queries/indexing rely on the checksum field instead. If you keep CharField, it would help to document why 8000 was chosen (provider observation/spec reference) to avoid arbitrary magic-number drift.

Suggested change
token = models.CharField(max_length=8000)
token = models.TextField()

Copilot uses AI. Check for mistakes.
application = models.ForeignKey(oauth2_settings.APPLICATION_MODEL, on_delete=models.CASCADE)
access_token = models.OneToOneField(
oauth2_settings.ACCESS_TOKEN_MODEL,
Expand Down
Loading