Skip to content
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

トークン発行API #12

Merged
merged 16 commits into from
Mar 23, 2024
Merged

トークン発行API #12

merged 16 commits into from
Mar 23, 2024

Conversation

furutahidehiko
Copy link
Owner

@furutahidehiko furutahidehiko commented Mar 11, 2024

やったこと

  • トークンAPIの作成
  • テスト用ユーザーのfixtureの追加
  • READMEの追記
  • 以下は対象外となります。
     - swagger-uiの導入
     - APIキーの生成
     - エンドポイントへの認証の適用

動作確認

ID/パスワード

  • 入力
{
  "id": "1",
  "password": "pass",
  "refresh_token": "",
  "grant_type": "password"
}
  • レスポンス
    スクリーンショット 2024-03-18 22 35 33

リフレッシュトークン

  • 入力
{
  "id": "",
  "password": "",
  "refresh_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxIiwiZXhwIjoxNzE4NTQ0ODcxfQ.vDKEURXiPgse9GTVMZLSLyglCJwJIW9PNSOu3U2Tcks",
  "grant_type": "refresh_token"
}

Copy link
Collaborator

@takuron1996 takuron1996 left a comment

Choose a reason for hiding this comment

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

@furutahidehiko
パスワード認証しかない気がします

application/config/environment.py Outdated Show resolved Hide resolved
@furutahidehiko furutahidehiko marked this pull request as ready for review March 18, 2024 14:43
Copy link
Collaborator

@takuron1996 takuron1996 left a comment

Choose a reason for hiding this comment

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

@furutahidehiko
ざっくりみてコメントしておきました。

README.md Outdated Show resolved Hide resolved
application/domain/user/check_password.py Outdated Show resolved Hide resolved
application/domain/user/check_password.py Outdated Show resolved Hide resolved
application/domain/user/check_token.py Outdated Show resolved Hide resolved
application/domain/user/check_password.py Outdated Show resolved Hide resolved
application/domain/user/token.py Outdated Show resolved Hide resolved
application/routers.py Outdated Show resolved Hide resolved
application/routers.py Outdated Show resolved Hide resolved
application/schemas/user.py Outdated Show resolved Hide resolved
@takuron1996 takuron1996 added duplicate This issue or pull request already exists enhancement New feature or request and removed duplicate This issue or pull request already exists labels Mar 18, 2024
Copy link
Collaborator

@takuron1996 takuron1996 left a comment

Choose a reason for hiding this comment

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

@furutahidehiko
コメントを消し忘れの部分と変数名の部分の対応をお願いします

application/routers.py Outdated Show resolved Hide resolved
@furutahidehiko
Copy link
Owner Author

@takuron1996
何度もレビューありがとうございます&すみません💦

3480947
で内部変数をuser_idからemailに変更しました(エンドポイントは引き続きuser_idのまま)

@furutahidehiko
Copy link
Owner Author

furutahidehiko commented Mar 22, 2024

メモ:
poetry.tomlのbcrypt = "^4.0.1"だと以下のエラーが出ますが、

fastapi  | (trapped) error reading bcrypt version
fastapi  | Traceback (most recent call last):
fastapi  |   File "/root/.cache/pypoetry/virtualenvs/fast-api-practice-hjPiGOLl-py3.11/lib/python3.11/site-packages/passlib/handlers/bcrypt.py", line 620, in _load_backend_mixin
fastapi  |     version = _bcrypt.__about__.__version__
fastapi  |               ^^^^^^^^^^^^^^^^^
fastapi  | AttributeError: module 'bcrypt' has no attribute '__about__'

今回の対応には直接影響ないかつため一旦別で対応しようと思ってます
(理由:上記エラーでててパスワードのハッシュ化は問題なくできているため)

対応方法
pyca/bcrypt#684 (comment)
で記載のとおりversion固定したら良さそうです

2024/03/23 追記
passlibはやめてbcryptでハッシュ化と検証を行う方針で修正

@takuron1996
Copy link
Collaborator

@furutahidehiko
version固定ではなく
pyca/bcrypt#684 (comment)
で書いてある通りpasslib を使用するのをやめるのはどうでしょうか?

@furutahidehiko
Copy link
Owner Author

@takuron1996

以下でpasslib でなくbcryptを使ってハッシュ化するように修正しました

519f929...cc55a84

@@ -13,7 +13,6 @@ asyncpg = "^0.28.0"
alembic = "^1.12.0"
ulid-py = "^1.1.0"
bcrypt = "^4.0.1"
passlib = { version = "^1.7.4", extras = ["bcrypt"] }
Copy link
Owner Author

Choose a reason for hiding this comment

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

[memo]
passlib使わなくなったので不要かなと思い削除しました

"""設定したパスワードと一致するかどうかを検証."""
input_password_hash = password.encode("utf-8")
hashed_password = self._password.encode("utf-8")
return bcrypt.checkpw(input_password_hash, hashed_password)
Copy link
Owner Author

Choose a reason for hiding this comment

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

[memo]
設定したパスワード(userテーブルに保存されたデータ)と入力されたパスワードが一致しているか確認するためにエンコードする。
bcrypt.checkpwでinput_password_hash(入力されたパスワード)とhashed_password (設定したパスワード)の突き合わせを実施

Copy link
Collaborator

@takuron1996 takuron1996 left a comment

Choose a reason for hiding this comment

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

@takuron1996 takuron1996 merged commit 459e7b5 into main Mar 23, 2024
@takuron1996 takuron1996 deleted the feature/auth branch March 23, 2024 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants