Client の CRUD API の作成 by anko9801 · Pull Request #52 · traPtitech/portal-oidc

anko9801 · 2026-01-25T15:51:27Z

OAuth2/OIDC における Client の CRUD を /api/v1/admin/client/{clientId} のパスで実装しました。

saitenntaisei

LGTM

Pugma

確認しました！　実装ありがとうございます
また、レビュー遅くなってすみません 🙇
テストでもう少しエラーに関するケースも追加したほうがいいのかなという印象はありましたが、概ね実装は問題なさそうです 👀

一点だけ、軽く確認してもらえればいいかなという感じです

Pugma · 2026-02-07T08:10:47Z

internal/domain/client.go

+type ClientWithSecret struct {
+	Client
+	ClientSecret string
+}


直前の Client 構造体はアプリで管理する情報 (domain) として用意されていて、現に他のところでも使われていると思います

ただ ClientWithSecret は、現状は Client 登録時の response としてしか使われておらず、また domain として ClientSecret 要素がそのまま string で平文として保管されていそうな印象もある気がします

個人的には

この構造体を router 層に移動する

ClientSecret を domain として持つときは CryptedClientSecret とか、暗号化がされていることを明示できる命名にする

ほうが、読んだときに意図がわかりやすい気がします！

Copilot

Pull request overview

OAuth2/OIDC の Client 管理（CRUD + シークレット再生成）を admin API として追加し、OpenAPI 定義・ルーティング・永続化・テストまで一通り実装する PR です。

Changes:

/api/v1/admin/clients 配下に Client の CRUD とシークレット再生成 API を追加（OpenAPI/生成コード/ハンドラ）
Client のドメイン・リポジトリ・ユースケースを追加し、MariaDB(sqlc) 永続化を実装
ユースケース単体テストと DB を使った統合テストを追加

Reviewed changes

Copilot reviewed 18 out of 19 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
internal/usecase/client.go	Client CRUD/secret 再生成のユースケースを追加
internal/usecase/client_test.go	ユースケースの単体テスト（インメモリ mock repo）を追加
internal/router/v1/handler.go	v1 ハンドラの DI コンテナを追加
internal/router/v1/client.go	Client CRUD/secret API の Echo ハンドラ実装を追加
internal/router/v1/client_test.go	MariaDB を使った admin client API の統合テストを追加
internal/router/v1/gen/server.go	OpenAPI から生成された server ラッパ/ルーティング/strict ハンドラを追加
internal/router/v1/gen/models.go	OpenAPI から生成された Client モデル群を追加
internal/domain/client.go	Client/ClientType のドメインモデルを追加
internal/repository/client.go	clients テーブル向けリポジトリ実装を追加
internal/repository/oidc/querier.go	sqlc Querier に DeleteAllClients を追加
internal/repository/oidc/oidc.sql.go	DeleteAllClients の生成コードを追加
internal/repository/oidc/db.go	DeleteAllClients の prepared statement を追加
internal/testutil/root.go	go.mod を辿ってプロジェクトルートを探すテストユーティリティを追加
api/openapi.yaml	Client 管理 API とスキーマ定義を追加
db/schema.sql	テーブル作成を IF NOT EXISTS に変更（テスト/初期化向け）
db/query/oidc.sql	DeleteAllClients を追加（sqlc 入力）
cmd/serve.go	Client 用の repo/usecase/handler を組み立ててルート登録
go.mod	uuid/x-crypto 依存追加
go.sum	依存追加に伴う更新

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-08T00:49:15Z

internal/router/v1/client.go

+func (h *Handler) CreateClient(ctx echo.Context) error {
+	var req gen.ClientCreate
+	if err := ctx.Bind(&req); err != nil {
+		return echo.NewHTTPError(http.StatusBadRequest, err.Error())
+	}


OpenAPI では redirect_uris の minItems=1 や client_type の enum、URI format 等が定義されていますが、現状は Bind のみで不正入力を受け入れます。name/redirect_uris/client_type のバリデーションを追加して 400 を返すようにしてください（Update も同様）。

Copilot · 2026-02-08T00:49:15Z

internal/router/v1/client.go

+	clients, err := h.clientUseCase.List(ctx.Request().Context())
+	if err != nil {
+		return echo.NewHTTPError(http.StatusInternalServerError, err.Error())
+	}


内部エラー時に err.Error() をそのままレスポンスに載せると、SQL エラー等の内部情報が外部に露出します。ログには詳細を残しつつ、クライアントへのメッセージは一般化（例: "internal server error"）するなど情報漏えいを避けてください。

Copilot · 2026-02-08T00:49:16Z

internal/usecase/client_test.go

+	uc := NewClientUseCase(repo)
+	ctx := context.Background()
+
+	created, _ := uc.Create(ctx, "original", domain.ClientTypeConfidential, []string{"http://localhost:3000/callback"})


テスト内で uc.Create のエラーを捨てています。失敗時に created が nil になり後続がパニックする可能性があるため、ここも if err != nil { t.Fatalf(...) } の形で検証してください。

Suggested change

created, _ := uc.Create(ctx, "original", domain.ClientTypeConfidential, []string{"http://localhost:3000/callback"})

created, err := uc.Create(ctx, "original", domain.ClientTypeConfidential, []string{"http://localhost:3000/callback"})

if err != nil {

t.Fatalf("Create failed: %v", err)

}

Copilot · 2026-02-08T00:49:16Z

internal/usecase/client_test.go

+	created, _ := uc.Create(ctx, "test-client", domain.ClientTypeConfidential, []string{"http://localhost:3000/callback"})
+	originalHash := repo.getSecretHash(created.ClientID)
+
+	newSecret, err := uc.RegenerateSecret(ctx, created.ClientID)
+	if err != nil {


テスト内で uc.Create のエラーを捨てています。失敗時に created が nil になり後続がパニックする可能性があるため、ここも if err != nil { t.Fatalf(...) } の形で検証してください。

Copilot · 2026-02-08T00:49:16Z

api/openapi.yaml

+paths:
+  /api/v1/admin/clients:
+    get:


PR 説明では /api/v1/admin/client/{clientId}（client が単数）となっていますが、OpenAPI/実装は /api/v1/admin/clients/{clientId}（複数形）です。どちらが正かを揃えて、PR 説明またはパス定義を修正してください。

Copilot · 2026-02-08T00:49:17Z

internal/usecase/client_test.go

+	uc := NewClientUseCase(repo)
+	ctx := context.Background()
+
+	created, _ := uc.Create(ctx, "test-client", domain.ClientTypeConfidential, []string{"http://localhost:3000/callback"})


テスト内で uc.Create のエラーを捨てています。失敗時に created が nil になり後続がパニックする可能性があるため、ここも if err != nil { t.Fatalf(...) } の形で検証してください。

Suggested change

created, _ := uc.Create(ctx, "test-client", domain.ClientTypeConfidential, []string{"http://localhost:3000/callback"})

created, err := uc.Create(ctx, "test-client", domain.ClientTypeConfidential, []string{"http://localhost:3000/callback"})

if err != nil {

t.Fatalf("Create failed: %v", err)

}

Copilot · 2026-02-08T00:49:17Z

internal/usecase/client_test.go

+	created, _ := uc.Create(ctx, "test-client", domain.ClientTypeConfidential, []string{"http://localhost:3000/callback"})
+
+	err := uc.Delete(ctx, created.ClientID)
+	if err != nil {


テスト内で uc.Create のエラーを捨てています。失敗時に created が nil になり後続がパニックする可能性があるため、ここも if err != nil { t.Fatalf(...) } の形で検証してください。

Copilot · 2026-02-08T00:49:17Z

internal/usecase/client.go

+	secret, err := generateSecret()
+	if err != nil {
+		return nil, err
+	}
+
+	secretHash, err := hashSecret(secret)
+	if err != nil {
+		return nil, err


client_type が public の場合でも無条件にシークレットを生成しています。public client はシークレットを保持しない前提とするなら、confidential のみ生成・保存し、public は client_secret_hash を NULL にする／そもそも public を受け付けない等、型ごとの扱いを明確にしてください。

Suggested change

secret, err := generateSecret()

if err != nil {

return nil, err

}

secretHash, err := hashSecret(secret)

if err != nil {

return nil, err

var (

secret string

secretHash string

)

switch clientType {

case domain.ClientTypeConfidential:

s, err := generateSecret()

if err != nil {

return nil, err

}

h, err := hashSecret(s)

if err != nil {

return nil, err

}

secret = s

secretHash = h

case domain.ClientTypePublic:

// Public clients do not have a client secret.

// Leave secret and secretHash as empty values so that

// the repository can persist a NULL/empty secret hash.

default:

return nil, errors.New("invalid client type")

これは確実に対応しておいていただきたいです
#53 で対応されている fosite の実装の都合や、また secret が public client でも必ず返ってしまうため、仕様の範囲外になります

Copilot · 2026-02-08T00:49:17Z

internal/usecase/client.go

+func (u *clientUseCase) RegenerateSecret(ctx context.Context, clientID uuid.UUID) (string, error) {
+	_, err := u.repo.Get(ctx, clientID)
+	if err != nil {
+		if errors.Is(err, repository.ErrClientNotFound) {
+			return "", ErrClientNotFound


RegenerateSecret は clientType を確認せず再生成しています。public client を許可する設計なら、public では拒否する（400/409 等）か、secret を保持しない方針に合わせて挙動を定義してください。

Copilot · 2026-02-08T00:49:17Z

cmd/serve.go

+	clientRepo := repository.NewClientRepository(queries)
+	clientUC := usecase.NewClientUseCase(clientRepo)
+	handler := v1.NewHandler(clientUC)



ここで admin 系の CRUD ハンドラを組み立てていますが、ルーティング前段の認証・認可ミドルウェアが見当たりません（/api/v1/admin/* が無条件で公開されます）。admin 専用の認証と権限チェックを追加してください。

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

anko9801 added 4 commits January 26, 2026 00:47

feat: add client table schema and queries

d10ac39

feat: add client CRUD API definition and generated code

25cdca1

feat: implement client CRUD domain, repository, usecase, and router

bde5d01

feat: integrate client usecase into server

fc37018

anko9801 requested a review from saitenntaisei as a code owner January 25, 2026 15:51

anko9801 requested a review from Pugma January 25, 2026 15:57

anko9801 changed the title ~~feat: Client CRUD API~~ Client の CRUD API の作成 Jan 25, 2026

test: add client integration tests

b4dbbc4

saitenntaisei approved these changes Jan 26, 2026

View reviewed changes

Pugma requested changes Feb 7, 2026

View reviewed changes

Pugma requested review from Copilot February 8, 2026 00:41

Copilot started reviewing on behalf of Pugma February 8, 2026 00:41 View session

Copilot AI reviewed Feb 8, 2026

View reviewed changes

-	secret, err := generateSecret()
-	if err != nil {
-		return nil, err
-	}
-	secretHash, err := hashSecret(secret)
-	if err != nil {
-		return nil, err
+	var (
+		secret     string
+		secretHash string
+	)
+	switch clientType {
+	case domain.ClientTypeConfidential:
+		s, err := generateSecret()
+		if err != nil {
+			return nil, err
+		}
+		h, err := hashSecret(s)
+		if err != nil {
+			return nil, err
+		}
+		secret = s
+		secretHash = h
+	case domain.ClientTypePublic:
+		// Public clients do not have a client secret.
+		// Leave secret and secretHash as empty values so that
+		// the repository can persist a NULL/empty secret hash.
+	default:
+		return nil, errors.New("invalid client type")

Conversation

anko9801 commented Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

saitenntaisei left a comment

Choose a reason for hiding this comment

Uh oh!

Pugma left a comment

Choose a reason for hiding this comment

Uh oh!

Pugma Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Pugma Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

anko9801 commented Jan 25, 2026 •

edited

Loading