Skip to content

models pk instead of models id #1446

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

Merged
merged 7 commits into from
Aug 15, 2024
Merged

models pk instead of models id #1446

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1412fb3
use user.pk instead of user.id
sahama Jul 30, 2024
e50c58b
update AUTHORS and CHANGELOG
sahama Jul 31, 2024
548e566
use pk instead of id of models
sahama Jul 31, 2024
d31fd66
update CHANGELOG
sahama Jul 31, 2024
16cc0d8
pk instead of id in admin
sahama Jul 31, 2024
8f7a9bd
Merge branch 'master' into BUGFIX/models-pk-instead-of-models-id
n2ygk Aug 15, 2024
6f2dbd4
decide not to deprecate
n2ygk Aug 15, 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
1 change: 1 addition & 0 deletions AUTHORS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ Rustem Saiargaliev
Sandro Rodrigues
Shaheed Haque
Shaun Stanworth
Sayyid Hamid Mahdavi
Silvano Cerza
Sora Yanai
Spencer Carroll
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* #1425 Remove deprecated `RedirectURIValidator`, `WildcardSet` per #1345; `validate_logout_request` per #1274

### Fixed
* now all part of code use pk instead of id for models.
### Security

## [2.4.0] - 2024-05-13
Expand Down
2 changes: 1 addition & 1 deletion oauth2_provider/admin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@


class ApplicationAdmin(admin.ModelAdmin):
list_display = ("id", "name", "user", "client_type", "authorization_grant_type")
list_display = ("pk", "name", "user", "client_type", "authorization_grant_type")
list_filter = ("client_type", "authorization_grant_type", "skip_authorization")
radio_fields = {
"client_type": admin.HORIZONTAL,
Expand Down
5 changes: 3 additions & 2 deletions oauth2_provider/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,9 @@ def clean(self):
):
raise ValidationError(_("You cannot use HS256 with public grants or clients"))

# TODO: I removed usage of this in templates. so it can label as deprecated.`
def get_absolute_url(self):
return reverse("oauth2_provider:detail", args=[str(self.id)])
return reverse("oauth2_provider:detail", args=[str(self.pk)])

def get_allowed_schemes(self):
"""
Expand Down Expand Up @@ -508,7 +509,7 @@ def revoke(self):
self = list(token)[0]

try:
access_token_model.objects.get(id=self.access_token_id).revoke()
access_token_model.objects.get(pk=self.access_token_id).revoke()
except access_token_model.DoesNotExist:
pass
self.access_token = None
Expand Down
8 changes: 4 additions & 4 deletions oauth2_provider/oauth2_validators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ def save_bearer_token(self, token, request, *args, **kwargs):
# from the db while acquiring a lock on it
# We also put it in the "request cache"
refresh_token_instance = RefreshToken.objects.select_for_update().get(
id=refresh_token_instance.id
pk=refresh_token_instance.pk
)
request.refresh_token_instance = refresh_token_instance

Expand Down Expand Up @@ -741,7 +741,7 @@ def get_original_scopes(self, refresh_token, request, *args, **kwargs):
rt = request.refresh_token_instance
if not rt.access_token_id:
try:
return AccessToken.objects.get(source_refresh_token_id=rt.id).scope
return AccessToken.objects.get(source_refresh_token_id=rt.pk).scope
except AccessToken.DoesNotExist:
return []
return rt.access_token.scope
Expand Down Expand Up @@ -792,9 +792,9 @@ def get_jwt_bearer_token(self, token, token_handler, request):

def get_claim_dict(self, request):
if self._get_additional_claims_is_request_agnostic():
claims = {"sub": lambda r: str(r.user.id)}
claims = {"sub": lambda r: str(r.user.pk)}
else:
claims = {"sub": str(request.user.id)}
claims = {"sub": str(request.user.pk)}

# https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims
if self._get_additional_claims_is_request_agnostic():
Expand Down
4 changes: 2 additions & 2 deletions oauth2_provider/templates/oauth2_provider/application_detail.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ <h3 class="block-center-heading">{{ application.name }}</h3>

<div class="btn-toolbar">
<a class="btn" href="{% url "oauth2_provider:list" %}">{% trans "Go Back" %}</a>
<a class="btn btn-primary" href="{% url "oauth2_provider:update" application.id %}">{% trans "Edit" %}</a>
<a class="btn btn-danger" href="{% url "oauth2_provider:delete" application.id %}">{% trans "Delete" %}</a>
<a class="btn btn-primary" href="{% url "oauth2_provider:update" application.pk %}">{% trans "Edit" %}</a>
<a class="btn btn-danger" href="{% url "oauth2_provider:delete" application.pk %}">{% trans "Delete" %}</a>
</div>
</div>
{% endblock content %}
4 changes: 2 additions & 2 deletions oauth2_provider/templates/oauth2_provider/application_form.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
{% load i18n %}
{% block content %}
<div class="block-center">
<form class="form-horizontal" method="post" action="{% block app-form-action-url %}{% url 'oauth2_provider:update' application.id %}{% endblock app-form-action-url %}">
<form class="form-horizontal" method="post" action="{% block app-form-action-url %}{% url 'oauth2_provider:update' application.pk %}{% endblock app-form-action-url %}">
<h3 class="block-center-heading">
{% block app-form-title %}
{% trans "Edit application" %} {{ application.name }}
Expand Down Expand Up @@ -31,7 +31,7 @@ <h3 class="block-center-heading">

<div class="control-group">
<div class="controls">
<a class="btn" href="{% block app-form-back-url %}{% url "oauth2_provider:detail" application.id %}{% endblock app-form-back-url %}">
<a class="btn" href="{% block app-form-back-url %}{% url "oauth2_provider:detail" application.pk %}{% endblock app-form-back-url %}">
{% trans "Go Back" %}
</a>
<button type="submit" class="btn btn-primary">{% trans "Save" %}</button>
Expand Down
2 changes: 1 addition & 1 deletion oauth2_provider/templates/oauth2_provider/application_list.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ <h3 class="block-center-heading">{% trans "Your applications" %}</h3>
{% if applications %}
<ul>
{% for application in applications %}
<li><a href="{{ application.get_absolute_url }}">{{ application.name }}</a></li>
<li><a href="{% url "oauth2_provider:detail" application.pk %}">{{ application.name }}</a></li>
{% endfor %}
</ul>

Expand Down
16 changes: 8 additions & 8 deletions tests/test_token_revocation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def test_revoke_access_token(self):
response = self.client.post(url, data=data)
self.assertEqual(response.status_code, 200)
self.assertEqual(response.content, b"")
self.assertFalse(AccessToken.objects.filter(id=tok.id).exists())
self.assertFalse(AccessToken.objects.filter(pk=tok.pk).exists())

def test_revoke_access_token_public(self):
public_app = Application(
Expand Down Expand Up @@ -101,7 +101,7 @@ def test_revoke_access_token_with_hint(self):
url = reverse("oauth2_provider:revoke-token")
response = self.client.post(url, data=data)
self.assertEqual(response.status_code, 200)
self.assertFalse(AccessToken.objects.filter(id=tok.id).exists())
self.assertFalse(AccessToken.objects.filter(pk=tok.pk).exists())

def test_revoke_access_token_with_invalid_hint(self):
tok = AccessToken.objects.create(
Expand All @@ -123,7 +123,7 @@ def test_revoke_access_token_with_invalid_hint(self):
url = reverse("oauth2_provider:revoke-token")
response = self.client.post(url, data=data)
self.assertEqual(response.status_code, 200)
self.assertFalse(AccessToken.objects.filter(id=tok.id).exists())
self.assertFalse(AccessToken.objects.filter(pk=tok.pk).exists())

def test_revoke_refresh_token(self):
tok = AccessToken.objects.create(
Expand All @@ -146,9 +146,9 @@ def test_revoke_refresh_token(self):
url = reverse("oauth2_provider:revoke-token")
response = self.client.post(url, data=data)
self.assertEqual(response.status_code, 200)
refresh_token = RefreshToken.objects.filter(id=rtok.id).first()
refresh_token = RefreshToken.objects.filter(pk=rtok.pk).first()
self.assertIsNotNone(refresh_token.revoked)
self.assertFalse(AccessToken.objects.filter(id=rtok.access_token.id).exists())
self.assertFalse(AccessToken.objects.filter(pk=rtok.access_token.pk).exists())

def test_revoke_refresh_token_with_revoked_access_token(self):
tok = AccessToken.objects.create(
Expand All @@ -172,8 +172,8 @@ def test_revoke_refresh_token_with_revoked_access_token(self):
response = self.client.post(url, data=data)
self.assertEqual(response.status_code, 200)

self.assertFalse(AccessToken.objects.filter(id=tok.id).exists())
refresh_token = RefreshToken.objects.filter(id=rtok.id).first()
self.assertFalse(AccessToken.objects.filter(pk=tok.pk).exists())
refresh_token = RefreshToken.objects.filter(pk=rtok.pk).first()
self.assertIsNotNone(refresh_token.revoked)

def test_revoke_token_with_wrong_hint(self):
Expand Down Expand Up @@ -202,4 +202,4 @@ def test_revoke_token_with_wrong_hint(self):
url = reverse("oauth2_provider:revoke-token")
response = self.client.post(url, data=data)
self.assertEqual(response.status_code, 200)
self.assertFalse(AccessToken.objects.filter(id=tok.id).exists())
self.assertFalse(AccessToken.objects.filter(pk=tok.pk).exists())
Loading