Skip to content

New flow changes #100

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

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions api-collection/Auth/CreateAccount/Valid.bru
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ meta {
}

post {
url: {{baseUrl}}/accounts/register
url: http://localhost:8000/accounts/register
Copy link
Contributor

Choose a reason for hiding this comment

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

needs to use {{baseUrl}}

Copy link
Member Author

Choose a reason for hiding this comment

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

It doeesn't seem to pull it automatically though, It's better just to use the One that is going to be most often used with in development?

body: json
auth: none
}

body:json {
{
"unique_identifier": "myname",
"username": "My Name",
"username": "Name",
"password": "qweasd123",
"email": "[email protected]"
}
Expand Down
6 changes: 3 additions & 3 deletions api-collection/Auth/LoginWithCreds/success.bru
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ meta {
}

post {
url: {{baseUrl}}/accounts/login-credentials
url: http://127.0.0.1:8000/accounts/login-credentials
body: json
auth: none
}

body:json {
{
"email": "[email protected]",
"password": "admin"
"email": "[email protected]",
"password": "bob"
}
}
18 changes: 18 additions & 0 deletions api-collection/Auth/SHAChecks/check token.bru
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
meta {
name: check token
type: http
seq: 2
}

post {
url: http://127.0.0.1:8000/accounts/check-SHA512-for-account/
Copy link
Contributor

Choose a reason for hiding this comment

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

baseUrl

body: json
auth: none
}

body:json {
{
"sha512_token" : "AA",
"unique_identifier" : "bob"
}
}
3 changes: 3 additions & 0 deletions api-collection/Auth/SHAChecks/folder.bru
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
meta {
name: SHAChecks
Copy link
Member

Choose a reason for hiding this comment

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

don't commit bruno files. It was good before we had any auto documentation on the endpoints but now it will just confuse other developers. I'd say you can even remove the whole api-collection folder

}
21 changes: 21 additions & 0 deletions api-collection/Auth/SHAChecks/register token.bru
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
meta {
name: register token
type: http
seq: 1
}

post {
url: http://127.0.0.1:8000/accounts/register-SHA512-for-account/
Copy link
Contributor

Choose a reason for hiding this comment

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

baseUrl

body: json
auth: inherit
}

headers {
Authorization: Token 894bf0bfd6ef6c05feb6a3447dfc2f3a2fb0147cd7da498d2843021794297cf0
}

body:json {
{
"sha512_token" : "AA"
}
}
21 changes: 21 additions & 0 deletions api-collection/Characters/Get charracter token.bru
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
meta {
name: Get charracter token
type: http
seq: 7
}

post {
url: http://127.0.0.1:8000/persistence/characters/GenForkToken
Copy link
Contributor

Choose a reason for hiding this comment

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

baseUrl

body: json
auth: none
}

headers {
Authorization: Token 7cac756254c5574dbdd69e2129394337158b7929446576ede3fb2a43e179540c
}

body:json {
{
"fork_compatibility": "UnityStationDevelop"
}
}
4 changes: 2 additions & 2 deletions api-collection/Characters/GetAll.bru
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ meta {
}

get {
url: {{baseUrl}}/persistence/characters
url: http://127.0.0.1:8000/persistence/characters
Copy link
Contributor

Choose a reason for hiding this comment

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

baseUrl

body: none
auth: none
}

headers {
Authorization: Token 4963885504960842e61c6cadb4a9df05647e2c7bf1cfe08ea8cff57ab37058ac
Authorization: Token a74030290fa0dbc6d85f2e8dd885bbb76d76d9dedfe7c82bc60d72e8bd09210c
}
19 changes: 19 additions & 0 deletions api-collection/Characters/GetCompatible Token.bru
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
meta {
name: GetCompatible Token
type: http
seq: 8
}

get {
url: http://localhost:8000/persistence/characters/compatibleToken?character_sheet_version=1.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

baseUrl

body: none
auth: none
}

params:query {
character_sheet_version: 1.0.0
}

headers {
X-Character-Token: eyJzZXJ2ZXJfaWQiOiJVbml0eVN0YXRpb25EZXZlbG9wIiwidXVpZCI6ImJvYiIsIm5vbmNlIjoiMTFkMTI3NzdhNTZlNWViZCJ9:1ukm1R:-l14SCkeDVJHIx9_J8fAUeQDIoHCcDavviBmqtpqcBo
}
3 changes: 3 additions & 0 deletions api-collection/admin/folder.bru
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
meta {
name: admin
}
4 changes: 4 additions & 0 deletions src/accounts/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
from knox import views as knox_views

from .views import (
CheckSHA512ForAccountView,
ConfirmAccountView,
LoginWithCredentialsView,
LoginWithTokenView,
RegisterAccountView,
RegisterSHA512ForAccount,
Copy link
Member

Choose a reason for hiding this comment

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

I think Snep said SHA512 wasn't good enough for us. I would wait for his input here

RequestPasswordResetView,
RequestVerificationTokenView,
ResendAccountConfirmationView,
Expand Down Expand Up @@ -45,4 +47,6 @@
name="reset-password-token",
),
path("reset-password/", RequestPasswordResetView.as_view(), name="reset-password"),
path("register-SHA512-for-account/", RegisterSHA512ForAccount.as_view(), name="register-SHA512-for-account"),
path("check-SHA512-for-account/", CheckSHA512ForAccountView.as_view(), name="check-SHA512-for-account"),
]
87 changes: 85 additions & 2 deletions src/accounts/api/views.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
import logging
import secrets

from datetime import timedelta
from urllib.parse import urljoin
from uuid import uuid4

from django.conf import settings
from django.contrib.auth import authenticate
from django.core.exceptions import ObjectDoesNotExist, PermissionDenied
from django.core.management import BaseCommand
from django.utils import timezone
from drf_spectacular.utils import extend_schema
from knox.models import AuthToken
from knox.views import LoginView as KnoxLoginView
from rest_framework import status
from rest_framework import serializers, status
from rest_framework.generics import GenericAPIView
from rest_framework.permissions import AllowAny
from rest_framework.response import Response
Expand All @@ -20,7 +23,7 @@
from commons.error_response import ErrorResponse
from commons.mail_wrapper import send_email_with_template

from ..models import Account, AccountConfirmation, PasswordResetRequestModel
from ..models import Account, AccountConfirmation, PasswordResetRequestModel, SHA512Token
from .serializers import (
ConfirmAccountSerializer,
EmailSerializer,
Expand Down Expand Up @@ -369,3 +372,83 @@ def post(self, request, *args, **kwargs):
return Response(status=status.HTTP_200_OK)
else:
return ErrorResponse(serializer.errors, status.HTTP_400_BAD_REQUEST)


class RegisterSHA512ForAccount(APIView):
class InputSerializer(serializers.Serializer):
Copy link
Member

Choose a reason for hiding this comment

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

don't create serializers inside the view class. There is a serializers file for this purpose. Everything on their own file

Copy link
Member

Choose a reason for hiding this comment

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

also the name for this serializer sucks. Follow the convention of the other serializers in the project

sha512_token = serializers.CharField(max_length=128)

def post(self, request, *args, **kwargs):
user: Account = request.user
if not request.auth:
Copy link
Member

Choose a reason for hiding this comment

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

since this view doesn't override the permission_classes field, it will use the default one which is IsAuthenticatedand handles the case where not valid auth token is passed, so this would not be needed.

return ErrorResponse("Invalid or missing token.", status.HTTP_401_UNAUTHORIZED)

if not user.is_confirmed:
return ErrorResponse(
"You must confirm your email before performing this action.",
status.HTTP_403_FORBIDDEN,
)

serializer = self.InputSerializer(data=request.data)
if not serializer.is_valid():
return ErrorResponse(serializer.errors, status.HTTP_400_BAD_REQUEST)

SHA512Token.objects.create(account=user, token=serializer.validated_data["sha512_token"])
Copy link
Member

Choose a reason for hiding this comment

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

wait, who creates the token? We can receive whatever here.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeha, Token is made by server and hub, so theoretically it can be anything, As long as they both agree


return Response(
{"detail": "SHA512 token registered successfully."},
status=status.HTTP_200_OK,
)


class CheckSHA512ForAccountView(APIView):
"""
Given an account unique_identifier and a SHA512 token,
checks if the token is associated with that account.
Deletes the token after checking.
**Public endpoint**
"""

permission_classes = (AllowAny,)

class InputSerializer(serializers.Serializer):
unique_identifier = serializers.CharField(max_length=28)
sha512_token = serializers.CharField(max_length=128)

def post(self, request, *args, **kwargs):
serializer = self.InputSerializer(data=request.data)
if not serializer.is_valid():
return ErrorResponse(serializer.errors, status.HTTP_400_BAD_REQUEST)

unique_id = serializer.validated_data["unique_identifier"]
token = serializer.validated_data["sha512_token"]

try:
account = Account.objects.get(unique_identifier=unique_id)
except Account.DoesNotExist:
return Response({"exists": False}, status=status.HTTP_200_OK)
Copy link
Member

Choose a reason for hiding this comment

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

even tho a 404 here would explain everything already, I think it is more sensible to respond with 401 unaunthenticated and the message Token has expired or is invalid


valid_cutoff = timezone.now() - timedelta(minutes=3)
matching_token = SHA512Token.objects.filter(
account=account,
token=token,
created_at__gte=valid_cutoff,
).first()

if matching_token:
matching_token.delete()
return Response(
{"exists": True, "account": PublicAccountDataSerializer(account, context={"request": request}).data},
Copy link
Member

Choose a reason for hiding this comment

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

just return 200 here. If you got a 200 it means everything is fine, no need for the existsfield

status=status.HTTP_200_OK,
)
else:
return Response({"exists": False}, status=status.HTTP_200_OK)
Copy link
Member

Choose a reason for hiding this comment

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

return 401 here, unauthenticated. You can add the message Token has expired or is invalid



class Command(BaseCommand):
Copy link
Member

Choose a reason for hiding this comment

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

remove, this doesn't go here

help = "Delete expired SHA512 tokens (older than 3 minutes)"

def handle(self, *args, **kwargs):
cutoff = timezone.now() - timedelta(minutes=3)
deleted, _ = SHA512Token.objects.filter(created_at__lt=cutoff).delete()
self.stdout.write(f"Deleted {deleted} expired SHA512 tokens.")
24 changes: 24 additions & 0 deletions src/accounts/migrations/0005_sha512token.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Generated by Django 3.2.25 on 2025-08-03 16:41

from django.conf import settings
from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
('accounts', '0004_alter_account_username'),
]

operations = [
migrations.CreateModel(
name='SHA512Token',
fields=[
Copy link
Contributor

Choose a reason for hiding this comment

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

are these all the fields that we migrate? or are there more?

Copy link
Member Author

Choose a reason for hiding this comment

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

idk,This is automatically generated code

('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('token', models.CharField(max_length=128)),
('created_at', models.DateTimeField(auto_now_add=True)),
('account', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='sha512_tokens', to=settings.AUTH_USER_MODEL)),
],
),
]
15 changes: 13 additions & 2 deletions src/accounts/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ class Account(AbstractUser):
unique=False,
validators=[MinLengthValidator(3), UnicodeUsernameValidator()],
help_text=(
"Public username is used to identify your account publicly and shows in "
"OOC. This can be changed at any time"
"Public username is used to identify your account publicly and shows in OOC. This can be changed at any time"
),
)

Expand Down Expand Up @@ -133,3 +132,15 @@ def is_token_valid(self):
if self.created_at is None:
return False
return (self.created_at + timedelta(minutes=settings.PASS_RESET_TOKEN_TTL)) > timezone.now()


class SHA512Token(models.Model):
token = models.CharField(max_length=128)
account = models.ForeignKey(Account, related_name="sha512_tokens", on_delete=models.CASCADE)
created_at = models.DateTimeField(auto_now_add=True)

def __str__(self):
return f"SHA512 token for {self.account} created at {self.created_at}"

def is_valid(self):
return (self.created_at + timedelta(minutes=3)) > timezone.now()
18 changes: 18 additions & 0 deletions src/persistence/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,16 @@

from .views import (
CreateCharacterView,
CreateCharacterViewToken,
DeleteCharacterView,
DeleteCharacterViewToken,
GenerateForkTokenView,
GetAllCharactersByAccountView,
GetCharacterByIdView,
GetCompatibleCharacters,
GetCompatibleCharactersToken,
UpdateCharacterView,
UpdateCharacterViewToken,
)

app_name = "persistence"
Expand All @@ -18,4 +23,17 @@
path("characters/compatible", GetCompatibleCharacters.as_view(), name="characters-compatible"),
path("characters/<int:pk>/update", UpdateCharacterView.as_view(), name="characters-patch"),
path("characters/<int:pk>/delete", DeleteCharacterView.as_view(), name="characters-delete"),
path(
"characters/<int:pk>/updateToken", UpdateCharacterViewToken.as_view(), name="characters-patch-token"
), # PutAccountsCharacterByIDByCharactersToken
path(
"characters/createToken", CreateCharacterViewToken.as_view(), name="characters-create-token"
), # PostMakeAccountsCharacterByCharactersToken
path(
"characters/compatibleToken", GetCompatibleCharactersToken.as_view(), name="characters-compatible-token"
), # GetCharactersByCharacterSheetToken
path(
"characters/<int:pk>/deleteToken", DeleteCharacterViewToken.as_view(), name="characters-delete-token"
), # DeleteAccountsCharacterByIDByCharactersToken
path("characters/GenForkToken", GenerateForkTokenView.as_view(), name="Gen-Fork-token"),
]
Loading
Loading