-
Notifications
You must be signed in to change notification settings - Fork 0
feat(auth): migrate user identities from integer to UUIDv4 #43
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,73 @@ | ||||||||||||||||||||||||||
| """Change User.id to UUID | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Revision ID: 7f066a8213a8 | ||||||||||||||||||||||||||
| Revises: e1a664780dc6 | ||||||||||||||||||||||||||
| Create Date: 2026-03-27 22:35:06.005762 | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| from typing import Sequence, Union | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| from alembic import op | ||||||||||||||||||||||||||
| import sqlalchemy as sa | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # revision identifiers, used by Alembic. | ||||||||||||||||||||||||||
| revision: str = "7f066a8213a8" | ||||||||||||||||||||||||||
| down_revision: Union[str, Sequence[str], None] = "e1a664780dc6" | ||||||||||||||||||||||||||
Check noticeCode scanning / CodeQL Unused global variable Note
The global variable 'down_revision' is not used.
Copilot AutofixAI 28 days ago Copilot could not generate an autofix suggestion Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support. |
||||||||||||||||||||||||||
| branch_labels: Union[str, Sequence[str], None] = None | ||||||||||||||||||||||||||
Check noticeCode scanning / CodeQL Unused global variable Note
The global variable 'branch_labels' is not used.
Copilot AutofixAI 28 days ago In general, to fix an unused global variable you either remove the assignment if it serves no purpose, or, if it is intentionally unused, rename it to follow an “unused” naming convention (e.g. prefix with Concretely, in branch_labels: Union[str, Sequence[str], None] = NoneNo new imports, functions, or other definitions are needed.
Suggested changeset
1
alembic/versions/7f066a8213a8_change_user_id_to_uuid.py
Copilot is powered by AI and may make mistakes. Always verify output.
Refresh and try again.
|
||||||||||||||||||||||||||
| depends_on: Union[str, Sequence[str], None] = None | ||||||||||||||||||||||||||
Check noticeCode scanning / CodeQL Unused global variable Note
The global variable 'depends_on' is not used.
Copilot AutofixAI 28 days ago To fix an unused global variable, either remove its assignment if it has no side effects and is not needed, or rename it to a clearly “unused” style if it must remain. Here, Concretely, in
Suggested changeset
1
alembic/versions/7f066a8213a8_change_user_id_to_uuid.py
Copilot is powered by AI and may make mistakes. Always verify output.
Refresh and try again.
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def upgrade() -> None: | ||||||||||||||||||||||||||
| """Upgrade schema.""" | ||||||||||||||||||||||||||
| # ### commands auto generated by Alembic - please adjust! ### | ||||||||||||||||||||||||||
| # Setup extension for UUID generation if needed | ||||||||||||||||||||||||||
| op.execute('CREATE EXTENSION IF NOT EXISTS "uuid-ossp"') | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # We truncate because casting int -> uuid with a random function would break existing foreign key relations. | ||||||||||||||||||||||||||
| op.execute("TRUNCATE TABLE users CASCADE") | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Drop foreign key constraint on verification_tokens | ||||||||||||||||||||||||||
| op.drop_constraint( | ||||||||||||||||||||||||||
| "verification_tokens_user_id_fkey", "verification_tokens", type_="foreignkey" | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Alter the columns | ||||||||||||||||||||||||||
| # Alter the columns. First drop the integer default from the id column to allow cast. | ||||||||||||||||||||||||||
| op.execute("ALTER TABLE users ALTER COLUMN id DROP DEFAULT") | ||||||||||||||||||||||||||
| op.execute( | ||||||||||||||||||||||||||
| "ALTER TABLE users ALTER COLUMN id SET DATA TYPE UUID USING (uuid_generate_v4())" | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| op.execute( | ||||||||||||||||||||||||||
| "ALTER TABLE verification_tokens ALTER COLUMN user_id SET DATA TYPE UUID USING (uuid_generate_v4())" | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Re-add foreign key constraint | ||||||||||||||||||||||||||
| op.create_foreign_key( | ||||||||||||||||||||||||||
| "verification_tokens_user_id_fkey", | ||||||||||||||||||||||||||
| "verification_tokens", | ||||||||||||||||||||||||||
| "users", | ||||||||||||||||||||||||||
| ["user_id"], | ||||||||||||||||||||||||||
| ["id"], | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| # ### end Alembic commands ### | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def downgrade() -> None: | ||||||||||||||||||||||||||
| """Downgrade schema.""" | ||||||||||||||||||||||||||
| # ### commands auto generated by Alembic - please adjust! ### | ||||||||||||||||||||||||||
| op.alter_column( | ||||||||||||||||||||||||||
| "verification_tokens", | ||||||||||||||||||||||||||
| "user_id", | ||||||||||||||||||||||||||
| existing_type=sa.Uuid(), | ||||||||||||||||||||||||||
| type_=sa.INTEGER(), | ||||||||||||||||||||||||||
| existing_nullable=False, | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| op.alter_column( | ||||||||||||||||||||||||||
| "users", | ||||||||||||||||||||||||||
| "id", | ||||||||||||||||||||||||||
| existing_type=sa.Uuid(), | ||||||||||||||||||||||||||
| type_=sa.INTEGER(), | ||||||||||||||||||||||||||
| existing_nullable=False, | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| # ### end Alembic commands ### | ||||||||||||||||||||||||||
|
Comment on lines
+56
to
+73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incomplete The
If downgrade support is required, it should mirror upgrade: ♻️ Suggested fix for downgrade def downgrade() -> None:
"""Downgrade schema."""
- # ### commands auto generated by Alembic - please adjust! ###
- op.alter_column('verification_tokens', 'user_id',
- existing_type=sa.Uuid(),
- type_=sa.INTEGER(),
- existing_nullable=False)
- op.alter_column('users', 'id',
- existing_type=sa.Uuid(),
- type_=sa.INTEGER(),
- existing_nullable=False)
- # ### end Alembic commands ###
+ # WARNING: Downgrade will lose all user data
+ op.drop_constraint('verification_tokens_user_id_fkey', 'verification_tokens', type_='foreignkey')
+ op.execute('TRUNCATE TABLE users CASCADE')
+ op.execute('ALTER TABLE verification_tokens ALTER COLUMN user_id SET DATA TYPE INTEGER USING 1')
+ op.execute('ALTER TABLE users ALTER COLUMN id SET DATA TYPE INTEGER USING 1')
+ op.execute('ALTER TABLE users ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY')
+ op.create_foreign_key('verification_tokens_user_id_fkey', 'verification_tokens', 'users', ['user_id'], ['id'])Alternatively, if downgrade isn't supported, raise def downgrade() -> None:
raise NotImplementedError("Downgrade from UUID to INTEGER is not supported")🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,32 @@ | ||||||||||||||||||||||||||
| """Add Meeting model | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Revision ID: e1a664780dc6 | ||||||||||||||||||||||||||
| Revises: 19dc9714d9ea | ||||||||||||||||||||||||||
| Create Date: 2026-03-27 22:18:35.070453 | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| from typing import Sequence, Union | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| from alembic import op | ||||||||||||||||||||||||||
| import sqlalchemy as sa | ||||||||||||||||||||||||||
Check noticeCode scanning / CodeQL Unused import Note
Import of 'sa' is not used.
Copilot AutofixAI 28 days ago To fix the problem, remove the unused import so that every imported name is actually referenced in the file. This keeps the code cleaner and avoids unnecessary dependencies. Concretely, in
Suggested changeset
1
alembic/versions/e1a664780dc6_add_meeting_model.py
Copilot is powered by AI and may make mistakes. Always verify output.
Refresh and try again.
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # revision identifiers, used by Alembic. | ||||||||||||||||||||||||||
| revision: str = "e1a664780dc6" | ||||||||||||||||||||||||||
| down_revision: Union[str, Sequence[str], None] = "19dc9714d9ea" | ||||||||||||||||||||||||||
| branch_labels: Union[str, Sequence[str], None] = None | ||||||||||||||||||||||||||
| depends_on: Union[str, Sequence[str], None] = None | ||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def upgrade() -> None: | ||||||||||||||||||||||||||
| """Upgrade schema.""" | ||||||||||||||||||||||||||
| # ### commands auto generated by Alembic - please adjust! ### | ||||||||||||||||||||||||||
| pass | ||||||||||||||||||||||||||
| # ### end Alembic commands ### | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def downgrade() -> None: | ||||||||||||||||||||||||||
| """Downgrade schema.""" | ||||||||||||||||||||||||||
| # ### commands auto generated by Alembic - please adjust! ### | ||||||||||||||||||||||||||
| pass | ||||||||||||||||||||||||||
|
Comment on lines
+21
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Empty migration with misleading title. The migration is titled "Add Meeting model" but contains no operations. If this is a placeholder for future work, consider:
If the Meeting model operations were intended but omitted, please add them. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| # ### end Alembic commands ### | ||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| INFO [alembic.runtime.migration] Context impl PostgresqlImpl. | ||
| INFO [alembic.runtime.migration] Will assume transactional DDL. | ||
| INFO [alembic.runtime.migration] Running upgrade 19dc9714d9ea -> e1a664780dc6, Add Meeting model | ||
|
Comment on lines
+1
to
+3
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove migration log from repository. Similar to the test output files, Alembic execution logs are development artifacts. Consider adding 🧰 Tools🪛 LanguageTool[style] ~2-~2: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE) 🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,10 +20,15 @@ class Settings(BaseSettings): | |||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| # Security | ||||||||||||||||||||||||||||||||||||
| SECRET_KEY: str = "placeholder_secret_key" | ||||||||||||||||||||||||||||||||||||
| ACCESS_TOKEN_EXPIRE_MINUTES: int = 15 | ||||||||||||||||||||||||||||||||||||
| ALGORITHM: str = "HS256" | ||||||||||||||||||||||||||||||||||||
| ACCESS_TOKEN_EXPIRE_MINUTES: int = 60 | ||||||||||||||||||||||||||||||||||||
| REFRESH_TOKEN_EXPIRE_DAYS: int = 7 | ||||||||||||||||||||||||||||||||||||
| VERIFICATION_TOKEN_EXPIRE_HOURS: int = 24 | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| # Account Lockout | ||||||||||||||||||||||||||||||||||||
| MAX_FAILED_LOGIN_ATTEMPTS: int = 5 | ||||||||||||||||||||||||||||||||||||
| ACCOUNT_LOCKOUT_DAYS: int = 5 | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+24
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
FILES="$(fd -i 'config.py$' app) $(fd -i 'security.py$' app) $(fd -i 'account_lockout.py$' app)"
for f in $FILES; do
echo "=== $f ==="
rg -n -C2 'ACCESS_TOKEN_EXPIRE_MINUTES|MAX_FAILED_LOGIN_ATTEMPTS|ACCOUNT_LOCKOUT_DAYS|REFRESH_TOKEN_EXPIRE_DAYS' "$f"
doneRepository: Brints/FluentMeet Length of output: 1567 🏁 Script executed: # Read the full config.py file to understand BaseSettings setup and inheritance
wc -l app/core/config.py && head -50 app/core/config.pyRepository: Brints/FluentMeet Length of output: 1500 🏁 Script executed: # Read the account_lockout.py file to see how these config values are used
wc -l app/services/account_lockout.py && cat app/services/account_lockout.pyRepository: Brints/FluentMeet Length of output: 3574 🏁 Script executed: # Read the relevant parts of security.py to understand token creation logic
wc -l app/core/security.py && sed -n '60,120p' app/core/security.pyRepository: Brints/FluentMeet Length of output: 1765 🏁 Script executed: # Check if there's any validation or clamping of these values anywhere in the codebase
rg -n 'max\(.*MAX_FAILED_LOGIN_ATTEMPTS\)|min\(.*ACCOUNT_LOCKOUT_DAYS\)|clamp|assert.*EXPIRE' app/Repository: Brints/FluentMeet Length of output: 43 Add bounds validation for security-critical config values. Lines 24, 25, 29, and 30 accept any integer; Suggested fix+from pydantic import Field
from pydantic_settings import BaseSettings, SettingsConfigDict
@@
- ACCESS_TOKEN_EXPIRE_MINUTES: int = 60
+ ACCESS_TOKEN_EXPIRE_MINUTES: int = Field(default=60, gt=0)
- REFRESH_TOKEN_EXPIRE_DAYS: int = 7
+ REFRESH_TOKEN_EXPIRE_DAYS: int = Field(default=7, gt=0)
- VERIFICATION_TOKEN_EXPIRE_HOURS: int = 24
+ VERIFICATION_TOKEN_EXPIRE_HOURS: int = Field(default=24, gt=0)
@@
- MAX_FAILED_LOGIN_ATTEMPTS: int = 5
+ MAX_FAILED_LOGIN_ATTEMPTS: int = Field(default=5, gt=0)
- ACCOUNT_LOCKOUT_DAYS: int = 5
+ ACCOUNT_LOCKOUT_DAYS: int = Field(default=5, gt=0)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| # Database | ||||||||||||||||||||||||||||||||||||
| POSTGRES_SERVER: str = "localhost" | ||||||||||||||||||||||||||||||||||||
| POSTGRES_USER: str = "postgres" | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
Check notice
Code scanning / CodeQL
Unused global variable Note
Copilot Autofix
AI 28 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.