SMQ-2627 - Align PATs with new architecture #3295

nyagamunene · 2025-12-18T16:42:54Z

What type of PR is this?

What does this do?

Which issue(s) does this PR fix/relate to?

Related Issue Feature: PAT align with new architecture #2627
Resolves Feature: PAT align with new architecture #2627

Have you included tests for your changes?

Did you document any new/modified feature?

Notes

codecov · 2025-12-18T16:54:23Z

Codecov Report

❌ Patch coverage is 46.68770% with 169 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.09%. Comparing base (5b913dd) to head (a4a22a5).

Files with missing lines	Patch %	Lines
auth/pat.go	35.46%	97 Missing and 14 partials ⚠️
auth/postgres/repo.go	0.00%	31 Missing ⚠️
auth/postgres/pat.go	0.00%	12 Missing ⚠️
auth/service.go	0.00%	6 Missing and 1 partial ⚠️
auth/cache/pat.go	0.00%	6 Missing ⚠️
auth/api/grpc/auth/server.go	92.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3295      +/-   ##
==========================================
- Coverage   79.16%   74.09%   -5.07%     
==========================================
  Files         109      179      +70     
  Lines       13203    19953    +6750     
==========================================
+ Hits        10452    14784    +4332     
- Misses       2133     4277    +2144     
- Partials      618      892     +274

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dborovcanin · 2025-12-23T10:15:36Z

pkg/authn/authsvc/authn.go

 	}

-	if strings.HasPrefix(token, patPrefix) {
+	tokenType := authn.TokenType(res.GetTokenType())


This creates possible misconfiguration issue. I.e. what if token prefix here and tokenType returned from the service are not the same? While we are not able to validate token - we are able to parse it locally (even if only for prefix) and we can do this verification to prevent mismatch.

dborovcanin · 2025-12-23T10:18:32Z

channels/middleware/authorization.go

-			} else {
-				req.Operation = auth.ListOp
-			}
+			req.Operation = auth.ChannelDisconnectClientOp


Let's stay consistent in naming, compare this to ClientConnectToChannelOp.

arvindh123 · 2025-12-23T12:07:36Z

auth/pat.go

-	UnshareOp
-	PublishOp
-	SubscribeOp
+	ClientCreateOp Operation = iota + 100


Is it possible to reuse the existing service Operations ?

we will have cyclic imports

felixgateru · 2026-01-12T10:16:42Z

auth/pat.go

 	return err
 }

+var ValidOperationsForEntity = map[EntityType][]Operation{


I noticed here that we have not included domains as an entity, does this mean that pats can not be used for actions on domains?

From past discussion with @arvindh123, PATs are being used for entities like channels, clients, groups, dashboards and messages. It is not used to create users and domains. @dborovcanin can you confirm this?

This is fine; we can limit PATS to this at the moment. I.e. PATS are used for users to manage entities within the domain.

felixgateru · 2026-01-12T11:21:37Z

auth/pat.go

 	}

+	if !IsValidOperationForEntity(s.EntityType, s.Operation) {
+		return errors.Wrap(apiutil.ErrInvalidQueryParams, errors.New("operation not valid for entity type"))


You can define this error, instead of calling new each time

internal/proto/auth/v1/auth.proto

dborovcanin · 2026-01-12T12:48:04Z

internal/proto/auth/v1/auth.proto

+  string user_id = 2;             // User id
+  string pat_id = 3;              // Pat id
+  uint32 entity_type = 4;         // Entity type
+  string optional_domain_id = 5;  // Optional domain id


Why not optional string domain_id = 5?

Since users was remove I think it should be optional

pkg/roles/rolemanager/middleware/authorization.go

internal/proto/auth/v1/auth.proto

auth/postgres/init.go

auth/pat.go

auth/scope_test.go

auth/service.go

arvindh123

I have idea now, no need to implement in this PR.

We can have the permission map in auth service only
And in policy request we can send only the operations

Since PAT also sends the operation , we can make the users authz to send the operation instead of permission and
In auth service we can convert operations to Permissions

What do you think?

internal/proto/auth/v1/auth.proto

nyagamunene · 2026-01-20T17:08:50Z

I have idea now, no need to implement in this PR.

We can have the permission map in auth service only And in policy request we can send only the operations

Since PAT also sends the operation , we can make the users authz to send the operation instead of permission and In auth service we can convert operations to Permissions

What do you think?

This is a good idea. It would simplify the authorization process and have no need of having the permissions file for all the entities.

arvindh123 · 2026-01-21T08:00:44Z

pkg/authn/authsvc/authn.go

+	if isPAT && tokenType != authn.PersonalAccessToken {
+		return authn.Session{}, errTokenTypeMismatch
+	}
+	if !isPAT && tokenType == authn.PersonalAccessToken {
+		return authn.Session{}, errTokenTypeMismatch
 	}



This logic can be we merged into single case

Or something like this

if isPAT != (tokenType == authn.PersonalAccessToken) { return authn.Session{}, errTokenTypeMismatch }

Since this is library, i think we don't want to do this check here
We can do this check in Auth Service itself and believe response that auth service has returned

arvindh123 · 2026-01-21T08:05:32Z

pkg/authz/authsvc/authz.go

+	if pr.PatID != "" && pr.TokenType == authn.PersonalAccessToken {
+		patReq := grpcAuthV1.AuthZReq{
+			TokenType: uint32(pr.TokenType),
 			Policy: &grpcAuthV1.PolicyReq{
-				Domain:          pr.Domain,
-				SubjectType:     pr.SubjectType,
-				SubjectKind:     pr.SubjectKind,
-				SubjectRelation: pr.SubjectRelation,
-				Subject:         pr.Subject,
-				Relation:        pr.Relation,
-				Permission:      pr.Permission,
-				Object:          pr.Object,
-				ObjectType:      pr.ObjectType,
+				Subject:    pr.UserID,
+				PatId:      pr.PatID,
+				ObjectType: pr.EntityType.String(),
+				Domain:     pr.DomainID,
+				Operation:  uint32(pr.Operation),
+				Object:     pr.EntityID,
 			},
+		}
+		patRes, err := a.authSvcClient.Authorize(ctx, &patReq)
+		if err != nil {
+			return errors.Wrap(errors.ErrAuthorization, err)
+		}
+		if !patRes.GetAuthorized() {
+			return errors.ErrAuthorization
+		}
+	}


@nyagamunene I have question, Does this TokenType is needed?
How this TokenType is helpfull for us

arvindh123 · 2026-01-21T08:45:16Z

pkg/permissions/config.go

-		if err != nil {
-			return nil, nil, fmt.Errorf("failed to get permissions for %s: %w", entityType, err)
-		}
+func (pc *PermissionConfig) GetAuthOperations(entityType string) (map[string]string, map[string]string, error) {


Is it possible to reuse the existing GetPermission(et string, op K) , instead of GetAuthOperations

Or

Could please explain the reason for choosing having the new function GetAuthOperations

PATs requires us to extract auth_operation from the permission yaml file.

arvindh123 · 2026-01-21T12:26:38Z

internal/proto/auth/v1/auth.proto

 message AuthZReq {
-  oneof auth_type {
-    PolicyReq policy = 1;         // Policy-based authorization
-    PATReq pat = 2;               // PAT authorization
-  }
+  PolicyReq policy = 1;
 }



I think we can convert PolicyReq to AuthZReq, Since AuthZReq have only one field

arvindh123 · 2026-01-21T12:35:03Z

cmd/domains/main.go


+	authOps := make(map[string]auth.Operation)
+	for opName, opInfo := range domainRoleOps {
+		if opInfo.AuthOperation != "" {
+			if authOp, err := auth.ParseOperation(opInfo.AuthOperation); err == nil {
+				authOps[opName] = authOp
+			}
+		}
+	}


authOps is needed now, Since we have remove GetAuthOperations?

Yes. Its the operations used by the PATs

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

nyagamunene marked this pull request as ready for review December 18, 2025 21:06

nyagamunene requested a review from a team as a code owner December 18, 2025 21:06

nyagamunene force-pushed the SMQ-2627 branch from 7aa84c3 to 8976860 Compare December 22, 2025 18:55

dborovcanin requested changes Dec 23, 2025

View reviewed changes

arvindh123 requested changes Dec 23, 2025

View reviewed changes

nyagamunene force-pushed the SMQ-2627 branch from d2f93f8 to 9632e74 Compare December 23, 2025 12:47

dborovcanin force-pushed the SMQ-2627 branch from 81bc751 to 30bd274 Compare December 23, 2025 15:05

nyagamunene force-pushed the SMQ-2627 branch from 30bd274 to 55ad6f9 Compare January 12, 2026 08:53

felixgateru reviewed Jan 12, 2026

View reviewed changes

felixgateru requested changes Jan 12, 2026

View reviewed changes

felixgateru previously approved these changes Jan 12, 2026

View reviewed changes

dborovcanin requested changes Jan 12, 2026

View reviewed changes

nyagamunene dismissed felixgateru’s stale review via d284ac7 January 12, 2026 13:10

arvindh123 requested changes Jan 14, 2026

View reviewed changes

pkg/roles/rolemanager/middleware/authorization.go Show resolved Hide resolved

internal/proto/auth/v1/auth.proto Outdated Show resolved Hide resolved

nyagamunene force-pushed the SMQ-2627 branch from bb5595c to cdb0867 Compare January 15, 2026 08:14

felixgateru requested changes Jan 15, 2026

View reviewed changes

auth/postgres/init.go Show resolved Hide resolved

auth/pat.go Outdated Show resolved Hide resolved

auth/scope_test.go Show resolved Hide resolved

auth/service.go Outdated Show resolved Hide resolved

nyagamunene force-pushed the SMQ-2627 branch 3 times, most recently from 7a0353a to 7937796 Compare January 19, 2026 11:22

felixgateru previously approved these changes Jan 20, 2026

View reviewed changes

arvindh123 requested changes Jan 20, 2026

View reviewed changes

internal/proto/auth/v1/auth.proto Outdated Show resolved Hide resolved

internal/proto/auth/v1/auth.proto Outdated Show resolved Hide resolved

internal/proto/auth/v1/auth.proto Outdated Show resolved Hide resolved

nyagamunene dismissed felixgateru’s stale review via a04e9ea January 20, 2026 16:06

arvindh123 requested changes Jan 21, 2026

View reviewed changes

nyagamunene force-pushed the SMQ-2627 branch from df6ff16 to 573264f Compare January 21, 2026 10:12

arvindh123 requested changes Jan 21, 2026

View reviewed changes

nyagamunene force-pushed the SMQ-2627 branch from 3569e38 to 11300b0 Compare January 21, 2026 17:04

nyagamunene added 2 commits January 22, 2026 13:35

align pats with entity operations

13e9e6d

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

fix authorization

84a5959

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

nyagamunene added 24 commits January 22, 2026 13:35

fix tests

2aa65ef

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

fix failing linter

4e9d779

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

update proto

80519c3

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

update proto

a56ce21

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

address comments

4697d2e

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

address comments

0a9c4d3

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

update error

dc24097

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

fix failing linter

86621e3

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

fix pats tests

5c1da3b

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

define error as a variable

6554a8e

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

address comments

ec97da5

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

fix failing linter

7145786

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

fix failing linter

1cd5a62

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

address comments

9be80e7

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

refactor permissions.yaml

5f8835c

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

fix roles permissions

70eb59d

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

address comments

d768e35

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

address comments

fa61e20

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

refactor proto file

0f66797

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

address comments

94a8116

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

fix failing linter

5f122ef

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

fix failing linter

52313bf

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

address comments

8f86de7

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

address comments

f623aca

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

dborovcanin force-pushed the SMQ-2627 branch from 11300b0 to f623aca Compare January 22, 2026 12:35

nyagamunene added 4 commits January 23, 2026 13:56

refactor permissions file

e8cd084

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

add constants

d26b8f6

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

fix failing linter

9a18ff9

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

fix tests

a4a22a5

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

SMQ-2627 - Align PATs with new architecture #3295

Are you sure you want to change the base?

SMQ-2627 - Align PATs with new architecture #3295

Conversation

nyagamunene commented Dec 18, 2025

What type of PR is this?

What does this do?

Which issue(s) does this PR fix/relate to?

Have you included tests for your changes?

Did you document any new/modified feature?

Notes

Uh oh!

codecov bot commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arvindh123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nyagamunene commented Jan 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Dec 18, 2025 •

edited

Loading