From 1b39d71921be20df43e87e97a3f55c718789f49e Mon Sep 17 00:00:00 2001
From: Livio Spring <livio.a@gmail.com>
Date: Tue, 17 Sep 2024 14:18:29 +0200
Subject: [PATCH] fix: user grants deactivation (#8634)

# Which Problems Are Solved

ZITADEL's user grants deactivation mechanism did not work correctly.
Deactivated user grants were still provided in token, which could lead
to unauthorized access to applications and resources.
Additionally, the management and auth API always returned the state as
active or did not provide any information about the state.

# How the Problems Are Solved

- Correctly check the user grant state on active for tokens and user
information (userinfo, introspection, saml attributes)
- Map state in API and display in Console

(cherry picked from commit ca1914e235df8eb62189cec07eb0de2cdad29629)
---
 .../user-grants/user-grants.component.html      | 17 ++++++++++++++++-
 .../user-grants/user-grants.component.ts        | 10 +++++++++-
 .../src/app/pages/grants/grants.component.html  |  1 +
 .../granted-project-detail.component.html       | 11 ++++++++++-
 .../owned-project-detail.component.html         |  2 +-
 .../auth-user-detail.component.html             | 11 ++++++++++-
 .../user-detail/user-detail.component.html      |  2 +-
 internal/api/grpc/auth/user_grant.go            |  1 +
 internal/api/grpc/user/user_grant.go            | 17 ++++++++++++++++-
 internal/api/oidc/client.go                     | 13 +++++++++----
 internal/api/saml/storage.go                    |  5 +++++
 .../auth/repository/eventsourcing/repository.go |  7 ++++++-
 internal/query/user_grant.go                    |  4 ++++
 internal/query/userinfo_by_id.sql               |  1 +
 proto/zitadel/auth.proto                        |  5 +++++
 15 files changed, 95 insertions(+), 12 deletions(-)

diff --git a/console/src/app/modules/user-grants/user-grants.component.html b/console/src/app/modules/user-grants/user-grants.component.html
index efca9f74ec..82de67c8bb 100644
--- a/console/src/app/modules/user-grants/user-grants.component.html
+++ b/console/src/app/modules/user-grants/user-grants.component.html
@@ -154,10 +154,25 @@
         </td>
       </ng-container>
 
+      <ng-container matColumnDef="state">
+        <th mat-header-cell *matHeaderCellDef>{{ 'PROJECT.GRANT.STATE' | translate }}</th>
+        <td mat-cell *matCellDef="let grant">
+          <span
+            class="state"
+            [ngClass]="{
+              active: grant.state === UserGrantState.USER_GRANT_STATE_ACTIVE,
+              inactive: grant.state === UserGrantState.USER_GRANT_STATE_INACTIVE,
+            }"
+          >
+            {{ 'USER.DATA.STATE' + grant.state | translate }}
+          </span>
+        </td>
+      </ng-container>
+
       <ng-container matColumnDef="actions" stickyEnd>
         <th mat-header-cell *matHeaderCellDef class="user-tr-actions"></th>
         <td mat-cell class="user-tr-actions" *matCellDef="let grant; let i = index">
-          <cnsl-table-actions [hasActions]="true">
+          <cnsl-table-actions [hasActions]="!context.includes('user')">
             <button
               actions
               matTooltip="{{ 'ACTIONS.REMOVE' | translate }}"
diff --git a/console/src/app/modules/user-grants/user-grants.component.ts b/console/src/app/modules/user-grants/user-grants.component.ts
index 4b65eb99bf..63f9b8f882 100644
--- a/console/src/app/modules/user-grants/user-grants.component.ts
+++ b/console/src/app/modules/user-grants/user-grants.component.ts
@@ -8,7 +8,13 @@ import { tap } from 'rxjs/operators';
 import { enterAnimations } from 'src/app/animations';
 import { UserGrant as AuthUserGrant } from 'src/app/proto/generated/zitadel/auth_pb';
 import { Role } from 'src/app/proto/generated/zitadel/project_pb';
-import { Type, UserGrant as MgmtUserGrant, UserGrantQuery, UserGrant } from 'src/app/proto/generated/zitadel/user_pb';
+import {
+  Type,
+  UserGrant as MgmtUserGrant,
+  UserGrant,
+  UserGrantQuery,
+  UserGrantState,
+} from 'src/app/proto/generated/zitadel/user_pb';
 import { GrpcAuthService } from 'src/app/services/grpc-auth.service';
 import { ManagementService } from 'src/app/services/mgmt.service';
 import { ToastService } from 'src/app/services/toast.service';
@@ -66,6 +72,7 @@ export class UserGrantsComponent implements OnInit, AfterViewInit {
   public UserGrantContext: any = UserGrantContext;
   public Type: any = Type;
   public ActionKeysType: any = ActionKeysType;
+  public UserGrantState: any = UserGrantState;
   @Input() public type: Type | undefined = undefined;
 
   public filterOpen: boolean = false;
@@ -86,6 +93,7 @@ export class UserGrantsComponent implements OnInit, AfterViewInit {
     'type',
     'creationDate',
     'changeDate',
+    'state',
     'roleNamesList',
     'actions',
   ];
diff --git a/console/src/app/pages/grants/grants.component.html b/console/src/app/pages/grants/grants.component.html
index 03a5c2ca90..85d6a4df90 100644
--- a/console/src/app/pages/grants/grants.component.html
+++ b/console/src/app/pages/grants/grants.component.html
@@ -17,6 +17,7 @@
         'type',
         'creationDate',
         'changeDate',
+        'state',
         'roleNamesList',
         'actions'
       ]"
diff --git a/console/src/app/pages/projects/granted-projects/granted-project-detail/granted-project-detail.component.html b/console/src/app/pages/projects/granted-projects/granted-project-detail/granted-project-detail.component.html
index 17a41cce91..67fbdfddc7 100644
--- a/console/src/app/pages/projects/granted-projects/granted-project-detail/granted-project-detail.component.html
+++ b/console/src/app/pages/projects/granted-projects/granted-project-detail/granted-project-detail.component.html
@@ -42,7 +42,16 @@
           [context]="UserGrantContext.GRANTED_PROJECT"
           [projectId]="projectId"
           [grantId]="grantId"
-          [displayedColumns]="['select', 'user', 'projectId', 'creationDate', 'changeDate', 'roleNamesList', 'actions']"
+          [displayedColumns]="[
+            'select',
+            'user',
+            'projectId',
+            'creationDate',
+            'changeDate',
+            'state',
+            'roleNamesList',
+            'actions',
+          ]"
           [disableWrite]="(['user.grant.write$', 'user.grant.write:' + grantId] | hasRole | async) === false"
           [disableDelete]="(['user.grant.delete$', 'user.grant.delete:' + grantId] | hasRole | async) === false"
           [refreshOnPreviousRoutes]="['/grant-create/project/{{projectId}}/grant/{{grantId}}']"
diff --git a/console/src/app/pages/projects/owned-projects/owned-project-detail/owned-project-detail.component.html b/console/src/app/pages/projects/owned-projects/owned-project-detail/owned-project-detail.component.html
index 740c9532d7..a6941d62c4 100644
--- a/console/src/app/pages/projects/owned-projects/owned-project-detail/owned-project-detail.component.html
+++ b/console/src/app/pages/projects/owned-projects/owned-project-detail/owned-project-detail.component.html
@@ -187,7 +187,7 @@
         <cnsl-user-grants
           [context]="UserGrantContext.OWNED_PROJECT"
           [projectId]="projectId"
-          [displayedColumns]="['select', 'user', 'creationDate', 'changeDate', 'roleNamesList', 'actions']"
+          [displayedColumns]="['select', 'user', 'creationDate', 'changeDate', 'state', 'roleNamesList', 'actions']"
           [refreshOnPreviousRoutes]="['/grant-create/project/' + projectId]"
           [disableWrite]="(['user.grant.write$', 'user.grant.write:' + projectId] | hasRole | async) === false"
           [disableDelete]="(['user.grant.delete$', 'user.grant.delete:' + projectId] | hasRole | async) === false"
diff --git a/console/src/app/pages/users/user-detail/auth-user-detail/auth-user-detail.component.html b/console/src/app/pages/users/user-detail/auth-user-detail/auth-user-detail.component.html
index a5d58cc06f..ce18e55e4a 100644
--- a/console/src/app/pages/users/user-detail/auth-user-detail/auth-user-detail.component.html
+++ b/console/src/app/pages/users/user-detail/auth-user-detail/auth-user-detail.component.html
@@ -136,7 +136,16 @@
           <cnsl-user-grants
             [userId]="user.id"
             [context]="USERGRANTCONTEXT"
-            [displayedColumns]="['org', 'projectId', 'type', 'creationDate', 'changeDate', 'roleNamesList', 'actions']"
+            [displayedColumns]="[
+              'org',
+              'projectId',
+              'type',
+              'creationDate',
+              'changeDate',
+              'state',
+              'roleNamesList',
+              'actions',
+            ]"
             [disableWrite]="(['user.grant.write$'] | hasRole | async) === false"
             [disableDelete]="(['user.grant.delete$'] | hasRole | async) === false"
           >
diff --git a/console/src/app/pages/users/user-detail/user-detail/user-detail.component.html b/console/src/app/pages/users/user-detail/user-detail/user-detail.component.html
index 45b6821b4b..9e319b7bc6 100644
--- a/console/src/app/pages/users/user-detail/user-detail/user-detail.component.html
+++ b/console/src/app/pages/users/user-detail/user-detail/user-detail.component.html
@@ -222,7 +222,7 @@
             <cnsl-user-grants
               [userId]="user.id"
               [context]="USERGRANTCONTEXT"
-              [displayedColumns]="['select', 'projectId', 'creationDate', 'changeDate', 'roleNamesList', 'actions']"
+              [displayedColumns]="['select', 'projectId', 'creationDate', 'changeDate', 'state', 'roleNamesList', 'actions']"
               [disableWrite]="(['user.grant.write$'] | hasRole | async) === false"
               [disableDelete]="(['user.grant.delete$'] | hasRole | async) === false"
             >
diff --git a/internal/api/grpc/auth/user_grant.go b/internal/api/grpc/auth/user_grant.go
index 242a54b044..9a1842f9a9 100644
--- a/internal/api/grpc/auth/user_grant.go
+++ b/internal/api/grpc/auth/user_grant.go
@@ -55,5 +55,6 @@ func UserGrantToPb(grant *query.UserGrant) *auth_pb.UserGrant {
 		ProjectGrantId: grant.GrantID,
 		RoleKeys:       grant.Roles,
 		UserType:       user.TypeToPb(grant.UserType),
+		State:          user.UserGrantStateToPb(grant.State),
 	}
 }
diff --git a/internal/api/grpc/user/user_grant.go b/internal/api/grpc/user/user_grant.go
index f4dd099409..eb7f2be205 100644
--- a/internal/api/grpc/user/user_grant.go
+++ b/internal/api/grpc/user/user_grant.go
@@ -23,7 +23,7 @@ func UserGrantToPb(assetPrefix string, grant *query.UserGrant) *user_pb.UserGran
 	return &user_pb.UserGrant{
 		Id:                 grant.ID,
 		UserId:             grant.UserID,
-		State:              user_pb.UserGrantState_USER_GRANT_STATE_ACTIVE,
+		State:              UserGrantStateToPb(grant.State),
 		RoleKeys:           grant.Roles,
 		ProjectId:          grant.ProjectID,
 		OrgId:              grant.ResourceOwner,
@@ -51,6 +51,21 @@ func UserGrantToPb(assetPrefix string, grant *query.UserGrant) *user_pb.UserGran
 	}
 }
 
+func UserGrantStateToPb(state domain.UserGrantState) user_pb.UserGrantState {
+	switch state {
+	case domain.UserGrantStateActive:
+		return user_pb.UserGrantState_USER_GRANT_STATE_ACTIVE
+	case domain.UserGrantStateInactive:
+		return user_pb.UserGrantState_USER_GRANT_STATE_INACTIVE
+	case domain.UserGrantStateRemoved,
+		domain.UserGrantStateUnspecified:
+		// these states should never occur here and are mainly listed for linting purposes
+		fallthrough
+	default:
+		return user_pb.UserGrantState_USER_GRANT_STATE_UNSPECIFIED
+	}
+}
+
 func UserGrantQueriesToQuery(ctx context.Context, queries []*user_pb.UserGrantQuery) (q []query.SearchQuery, err error) {
 	q = make([]query.SearchQuery, len(queries))
 	for i, query := range queries {
diff --git a/internal/api/oidc/client.go b/internal/api/oidc/client.go
index 77fe7451b2..80a61a189a 100644
--- a/internal/api/oidc/client.go
+++ b/internal/api/oidc/client.go
@@ -799,19 +799,24 @@ func (o *OPStorage) assertRoles(ctx context.Context, userID, applicationID strin
 	if projectID != "" {
 		roleAudience = append(roleAudience, projectID)
 	}
-	queries := make([]query.SearchQuery, 0, 2)
 	projectQuery, err := query.NewUserGrantProjectIDsSearchQuery(roleAudience)
 	if err != nil {
 		return nil, nil, err
 	}
-	queries = append(queries, projectQuery)
 	userIDQuery, err := query.NewUserGrantUserIDSearchQuery(userID)
 	if err != nil {
 		return nil, nil, err
 	}
-	queries = append(queries, userIDQuery)
+	activeQuery, err := query.NewUserGrantStateQuery(domain.UserGrantStateActive)
+	if err != nil {
+		return nil, nil, err
+	}
 	grants, err := o.query.UserGrants(ctx, &query.UserGrantsQueries{
-		Queries: queries,
+		Queries: []query.SearchQuery{
+			projectQuery,
+			userIDQuery,
+			activeQuery,
+		},
 	}, true)
 	if err != nil {
 		return nil, nil, err
diff --git a/internal/api/saml/storage.go b/internal/api/saml/storage.go
index 45b9af1b94..1fd83294d8 100644
--- a/internal/api/saml/storage.go
+++ b/internal/api/saml/storage.go
@@ -324,10 +324,15 @@ func (p *Storage) getGrants(ctx context.Context, userID, applicationID string) (
 	if err != nil {
 		return nil, err
 	}
+	activeQuery, err := query.NewUserGrantStateQuery(domain.UserGrantStateActive)
+	if err != nil {
+		return nil, err
+	}
 	return p.query.UserGrants(ctx, &query.UserGrantsQueries{
 		Queries: []query.SearchQuery{
 			projectQuery,
 			userIDQuery,
+			activeQuery,
 		},
 	}, true)
 }
diff --git a/internal/auth/repository/eventsourcing/repository.go b/internal/auth/repository/eventsourcing/repository.go
index 0d5e3609cc..79e8c42779 100644
--- a/internal/auth/repository/eventsourcing/repository.go
+++ b/internal/auth/repository/eventsourcing/repository.go
@@ -11,6 +11,7 @@ import (
 	sd "github.com/zitadel/zitadel/internal/config/systemdefaults"
 	"github.com/zitadel/zitadel/internal/crypto"
 	"github.com/zitadel/zitadel/internal/database"
+	"github.com/zitadel/zitadel/internal/domain"
 	eventstore2 "github.com/zitadel/zitadel/internal/eventstore"
 	"github.com/zitadel/zitadel/internal/id"
 	"github.com/zitadel/zitadel/internal/query"
@@ -118,7 +119,11 @@ func (q queryViewWrapper) UserGrantsByProjectAndUserID(ctx context.Context, proj
 	if err != nil {
 		return nil, err
 	}
-	queries := &query.UserGrantsQueries{Queries: []query.SearchQuery{userGrantUserID, userGrantProjectID}}
+	activeQuery, err := query.NewUserGrantStateQuery(domain.UserGrantStateActive)
+	if err != nil {
+		return nil, err
+	}
+	queries := &query.UserGrantsQueries{Queries: []query.SearchQuery{userGrantUserID, userGrantProjectID, activeQuery}}
 	grants, err := q.Queries.UserGrants(ctx, queries, true)
 	if err != nil {
 		return nil, err
diff --git a/internal/query/user_grant.go b/internal/query/user_grant.go
index 71031b5136..265d8eaae1 100644
--- a/internal/query/user_grant.go
+++ b/internal/query/user_grant.go
@@ -143,6 +143,10 @@ func NewUserGrantRoleQuery(value string) (SearchQuery, error) {
 	return NewTextQuery(UserGrantRoles, value, TextListContains)
 }
 
+func NewUserGrantStateQuery(value domain.UserGrantState) (SearchQuery, error) {
+	return NewNumberQuery(UserGrantState, value, NumberEquals)
+}
+
 func NewUserGrantWithGrantedQuery(owner string) (SearchQuery, error) {
 	orgQuery, err := NewUserGrantResourceOwnerSearchQuery(owner)
 	if err != nil {
diff --git a/internal/query/userinfo_by_id.sql b/internal/query/userinfo_by_id.sql
index 2c09215a69..93db8ab1b4 100644
--- a/internal/query/userinfo_by_id.sql
+++ b/internal/query/userinfo_by_id.sql
@@ -38,6 +38,7 @@ user_grants as (
 	where user_id = $1
 	and instance_id = $2
 	and project_id = any($3)
+    and state = 1
 	{{ if . -}}
 	and resource_owner = any($4)
 	{{- end }}
diff --git a/proto/zitadel/auth.proto b/proto/zitadel/auth.proto
index 0eae6d24e5..ed58d70569 100644
--- a/proto/zitadel/auth.proto
+++ b/proto/zitadel/auth.proto
@@ -1565,6 +1565,11 @@ message UserGrant {
             description: "type of the user (human / machine)"
         }
     ];
+    zitadel.user.v1.UserGrantState state = 13 [
+        (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {
+            description: "current state of the user grant";
+        }
+    ];
 }
 
 message ListMyProjectOrgsRequest {