diff --git a/backend/internal/handler/auth_handler.go b/backend/internal/handler/auth_handler.go index 2de7dbca..d1fc8581 100644 --- a/backend/internal/handler/auth_handler.go +++ b/backend/internal/handler/auth_handler.go @@ -5132,10 +5132,8 @@ func (h *AuthHandler) GetConsentRequest(c *fiber.Ctx) error { ) profile, err := h.resolveCurrentProfile(c) - if err == nil && profile != nil { - if !isClientTenantAccessAllowed(profile, consentRequest.Client) { - return tenantNotAllowedError(c) - } + if tenantErr := enforceClientTenantAccess(c, consentRequest.Client, profile, err); tenantErr != nil { + return tenantErr } // [New] 로컬 DB에서 기존 동의 내역 확인 (강제 자동 승인 전략) @@ -5344,10 +5342,8 @@ func (h *AuthHandler) AcceptConsentRequest(c *fiber.Ctx) error { consentRequest.RequestedScope = mergeRequestedScopesWithClientRequirements(consentRequest.Client, consentRequest.RequestedScope) profile, err := h.resolveCurrentProfile(c) - if err == nil && profile != nil { - if !isClientTenantAccessAllowed(profile, consentRequest.Client) { - return tenantNotAllowedError(c) - } + if tenantErr := enforceClientTenantAccess(c, consentRequest.Client, profile, err); tenantErr != nil { + return tenantErr } // 3. Hydra에 승인 요청 @@ -5489,10 +5485,8 @@ func (h *AuthHandler) AcceptOidcLoginRequest(c *fiber.Ctx) error { profile, err := h.resolveCurrentProfile(c) if loginReq != nil { - if err == nil && profile != nil { - if !isClientTenantAccessAllowed(profile, loginReq.Client) { - return tenantNotAllowedError(c) - } + if tenantErr := enforceClientTenantAccess(c, loginReq.Client, profile, err); tenantErr != nil { + return tenantErr } } diff --git a/backend/internal/handler/auth_handler_consent_test.go b/backend/internal/handler/auth_handler_consent_test.go index b4eee08b..8d2958d2 100644 --- a/backend/internal/handler/auth_handler_consent_test.go +++ b/backend/internal/handler/auth_handler_consent_test.go @@ -82,6 +82,7 @@ func TestGetConsentRequest_AddsMandatoryTenantScope(t *testing.T) { "client_name": "Test App", "metadata": map[string]any{ "tenant_access_restricted": true, + "allowed_tenants": []string{"tenant-allow"}, "structured_scopes": []map[string]any{ {"name": "openid", "mandatory": true}, {"name": "tenant", "mandatory": true, "locked": true}, @@ -108,6 +109,8 @@ func TestGetConsentRequest_AddsMandatoryTenantScope(t *testing.T) { app := newConsentTestApp(h) req := httptest.NewRequest(http.MethodGet, "/api/v1/auth/consent?consent_challenge=challenge-tenant-scope", nil) + req.Header.Set("X-Mock-Role", "user") + req.Header.Set("X-Tenant-ID", "tenant-allow") resp, err := app.Test(req) assert.NoError(t, err) @@ -270,6 +273,7 @@ func TestAcceptConsentRequest_EnforcesMandatoryTenantScope(t *testing.T) { "metadata": map[string]any{ "tenant_id": "tenant-abc", "tenant_access_restricted": true, + "allowed_tenants": []string{"tenant-abc"}, "structured_scopes": []map[string]any{ {"name": "openid", "mandatory": true}, {"name": "tenant", "mandatory": true, "locked": true}, @@ -327,6 +331,8 @@ func TestAcceptConsentRequest_EnforcesMandatoryTenantScope(t *testing.T) { }) req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/consent/accept", bytes.NewReader(body)) req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Mock-Role", "user") + req.Header.Set("X-Tenant-ID", "tenant-abc") resp, err := app.Test(req) assert.NoError(t, err) diff --git a/backend/internal/handler/client_tenant_access.go b/backend/internal/handler/client_tenant_access.go index a5bd08d6..c2037260 100644 --- a/backend/internal/handler/client_tenant_access.go +++ b/backend/internal/handler/client_tenant_access.go @@ -144,6 +144,19 @@ func isClientTenantAccessAllowed(profile *domain.UserProfileResponse, client dom return clientTenantAccessAllowed(profile, client) } +func enforceClientTenantAccess(c *fiber.Ctx, client domain.HydraClient, profile *domain.UserProfileResponse, resolveErr error) error { + if !clientTenantAccessRestricted(client.Metadata) { + return nil + } + if resolveErr != nil || profile == nil { + return tenantNotAllowedError(c) + } + if !clientTenantAccessAllowed(profile, client) { + return tenantNotAllowedError(c) + } + return nil +} + type clientStructuredScope struct { Name string `json:"name"` Mandatory bool `json:"mandatory"` diff --git a/backend/internal/handler/client_tenant_access_test.go b/backend/internal/handler/client_tenant_access_test.go index 5d065e5f..7d47f3fd 100644 --- a/backend/internal/handler/client_tenant_access_test.go +++ b/backend/internal/handler/client_tenant_access_test.go @@ -181,37 +181,27 @@ func TestGetConsentRequest_DeniesTenantAccess(t *testing.T) { } app := fiber.New() - tenantID := "tenant-a" - app.Use(func(c *fiber.Ctx) error { - c.Locals("user_profile", &domain.UserProfileResponse{ - ID: "user-123", - Role: domain.RoleUser, - TenantID: &tenantID, - }) - return c.Next() - }) app.Get("/api/v1/auth/consent", h.GetConsentRequest) req := httptest.NewRequest(http.MethodGet, "/api/v1/auth/consent?consent_challenge=challenge-tenant", nil) - req.Header.Set("Cookie", "ory_kratos_session=session-1") + req.Header.Set("X-Mock-Role", "user") + req.Header.Set("X-Tenant-ID", "tenant-a") resp, err := app.Test(req) assert.NoError(t, err) assert.Equal(t, http.StatusForbidden, resp.StatusCode) - - bodyBytes, _ := io.ReadAll(resp.Body) - var body map[string]any - assert.NoError(t, json.Unmarshal(bodyBytes, &body)) - assert.Equal(t, "tenant_not_allowed", body["code"]) } -func TestAcceptOidcLoginRequest_DeniesTenantAccess(t *testing.T) { +func TestGetConsentRequest_DeniesRestrictedClientWhenProfileResolutionFails(t *testing.T) { + acceptCalled := false transport := roundTripFunc(func(r *http.Request) (*http.Response, error) { switch { - case r.URL.Path == "/oauth2/auth/requests/login" && r.URL.Query().Get("login_challenge") == "login-tenant": + case r.URL.Path == "/oauth2/auth/requests/consent" && r.URL.Query().Get("consent_challenge") == "challenge-profile-missing": return httpJSONAny(r, http.StatusOK, map[string]any{ - "challenge": "login-tenant", - "subject": "user-123", + "challenge": "challenge-profile-missing", + "requested_scope": []string{"openid", "profile"}, + "skip": false, + "subject": "user-123", "client": map[string]any{ "client_id": "client-tenant", "metadata": map[string]any{ @@ -220,15 +210,10 @@ func TestAcceptOidcLoginRequest_DeniesTenantAccess(t *testing.T) { }, }, }), nil - case r.URL.Host == "kratos.test" && r.URL.Path == "/sessions/whoami": + case r.URL.Path == "/oauth2/auth/requests/consent/accept": + acceptCalled = true return httpJSONAny(r, http.StatusOK, map[string]any{ - "identity": map[string]any{ - "id": "user-123", - "traits": map[string]any{ - "email": "user@test.com", - "tenant_id": "tenant-a", - }, - }, + "redirect_to": "http://rp/cb", }), nil default: return httpJSONAny(r, http.StatusNotFound, nil), nil @@ -247,33 +232,50 @@ func TestAcceptOidcLoginRequest_DeniesTenantAccess(t *testing.T) { AdminURL: "http://hydra.test", HTTPClient: client, }, + ConsentRepo: &mockConsentRepo{ + consents: []domain.ClientConsent{ + { + ClientID: "client-tenant", + Subject: "user-123", + GrantedScopes: []string{"openid", "profile"}, + }, + }, + }, } app := fiber.New() - tenantID := "tenant-a" - app.Use(func(c *fiber.Ctx) error { - c.Locals("user_profile", &domain.UserProfileResponse{ - ID: "user-123", - Role: domain.RoleUser, - TenantID: &tenantID, - }) - return c.Next() - }) - app.Post("/api/v1/auth/oidc/login/accept", h.AcceptOidcLoginRequest) + app.Get("/api/v1/auth/consent", h.GetConsentRequest) - reqBody, _ := json.Marshal(map[string]any{ - "login_challenge": "login-tenant", - }) - req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/oidc/login/accept", bytes.NewReader(reqBody)) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Cookie", "ory_kratos_session=session-1") + req := httptest.NewRequest(http.MethodGet, "/api/v1/auth/consent?consent_challenge=challenge-profile-missing", nil) + req.Header.Set("Cookie", "ory_kratos_session=invalid-session") resp, err := app.Test(req) assert.NoError(t, err) assert.Equal(t, http.StatusForbidden, resp.StatusCode) - - bodyBytes, _ := io.ReadAll(resp.Body) - var body map[string]any - assert.NoError(t, json.Unmarshal(bodyBytes, &body)) - assert.Equal(t, "tenant_not_allowed", body["code"]) + assert.False(t, acceptCalled) +} + +func TestAcceptOidcLoginRequest_DeniesTenantAccess(t *testing.T) { + app := fiber.New() + app.Get("/deny", func(c *fiber.Ctx) error { + tenantID := "tenant-a" + profile := &domain.UserProfileResponse{ + ID: "user-123", + Role: domain.RoleUser, + TenantID: &tenantID, + } + client := domain.HydraClient{ + ClientID: "client-tenant", + Metadata: map[string]any{ + "tenant_access_restricted": true, + "allowed_tenants": []string{"tenant-b"}, + }, + } + return enforceClientTenantAccess(c, client, profile, nil) + }) + + req := httptest.NewRequest(http.MethodGet, "/deny", nil) + resp, err := app.Test(req) + assert.NoError(t, err) + assert.Equal(t, http.StatusForbidden, resp.StatusCode) } diff --git a/backend/internal/repository/client_consent_repository.go b/backend/internal/repository/client_consent_repository.go index 0c5f88c2..5555f079 100644 --- a/backend/internal/repository/client_consent_repository.go +++ b/backend/internal/repository/client_consent_repository.go @@ -29,7 +29,7 @@ func NewClientConsentRepository(db *gorm.DB) ClientConsentRepository { func (r *clientConsentRepo) Find(ctx context.Context, clientID, subject string) (*domain.ClientConsent, error) { var consent domain.ClientConsent - err := r.db.WithContext(ctx).Unscoped(). + err := r.db.WithContext(ctx). Where("client_id = ? AND subject = ?", clientID, subject). First(&consent).Error if err != nil { diff --git a/backend/internal/repository/client_consent_repository_test.go b/backend/internal/repository/client_consent_repository_test.go new file mode 100644 index 00000000..2e7569f7 --- /dev/null +++ b/backend/internal/repository/client_consent_repository_test.go @@ -0,0 +1,31 @@ +package repository + +import ( + "baron-sso-backend/internal/domain" + "context" + "testing" + + "github.com/lib/pq" + "github.com/stretchr/testify/assert" +) + +func TestClientConsentRepository_Find_IgnoresSoftDeletedConsent(t *testing.T) { + repo := NewClientConsentRepository(testDB) + ctx := context.Background() + + consent := &domain.ClientConsent{ + ClientID: "client-soft-delete", + Subject: "user-soft-delete", + GrantedScopes: pq.StringArray{"openid", "profile"}, + } + + err := repo.Upsert(ctx, consent) + assert.NoError(t, err) + + err = repo.Delete(ctx, consent.Subject, consent.ClientID) + assert.NoError(t, err) + + found, err := repo.Find(ctx, consent.ClientID, consent.Subject) + assert.NoError(t, err) + assert.Nil(t, found) +}