SMQ-2609 - Enable superadmin to perform actions over entities #2688

felixgateru · 2025-02-07T14:23:15Z

What type of PR is this?

This is a refactor as it remove DomainUserID encoding.

What does this do?

This pr removes DomainUserID encodinh and makes all authorization on entities to be performed with userID.

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

Related Issue #
Resolves Bug: SuperAdmin could not perform actions over entities #2609

Have you included tests for your changes?

Yes, I have updated tests.

Did you document any new/modified feature?

No,

Notes

None

codecov · 2025-02-10T14:59:19Z

Codecov Report

Attention: Patch coverage is 47.27273% with 58 lines in your changes missing coverage. Please review.

Project coverage is 27.58%. Comparing base (a01d257) to head (98a06c0).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
auth/service.go	36.95%	27 Missing and 2 partials ⚠️
domains/middleware/authorization.go	0.00%	16 Missing ⚠️
auth/jwt/tokenizer.go	42.10%	7 Missing and 4 partials ⚠️
auth/keys.go	83.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2688      +/-   ##
==========================================
+ Coverage   27.53%   27.58%   +0.04%     
==========================================
  Files         351      219     -132     
  Lines       55379    44701   -10678     
==========================================
- Hits        15251    12332    -2919     
+ Misses      39372    31775    -7597     
+ Partials      756      594     -162

☔ 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.

pkg/authz/authsvc/authz.go

groups/service.go

dborovcanin · 2025-02-26T13:34:28Z

@arvindh123 @felixgateru What's the status of this PR?

arvindh123 · 2025-03-07T11:21:04Z

domains/middleware/authorization.go

@@ -141,8 +150,7 @@ func (am *authorizationMiddleware) ListDomains(ctx context.Context, session auth
 }

 func (am *authorizationMiddleware) SendInvitation(ctx context.Context, session authn.Session, invitation domains.Invitation) (err error) {
-	domainUserId := auth.EncodeDomainUserID(invitation.DomainID, invitation.InviteeUserID)


Why Invitations related authz middleware functions are removed ?

arvindh123 · 2025-03-07T13:52:47Z

In this PR, we are changing the format from <domain_id>_<user_id> to just <user_id> in Spice DB for subjects.

One side effect/bug that occurs due to this PR is as follows:
I created domain_1 with ID 88724ac6-a569-43c2-a895-d8d3e93ad3df and domain_2 with ID 716a571e-fbe7-4cd2-afdc-fa0780a24531.

Then, I created a client, client_1_of_domain_2, with ID 1729828a-3c3d-4917-8678-b76abd28576a, in domain_2 (ID 716a571e-fbe7-4cd2-afdc-fa0780a24531).

However, when I try to view client_1_of_domain_2 with domain_1 in the URL (e.g., http://localhost/domain_1/clients/client_1_of_domain_2),
it shows the details of client_1_of_domain_2 from domain_2. Ideally, I should see a "not found" message instead. (edited)

arvindh123 · 2025-03-07T13:55:02Z

In this PR, we are changing the format from <domain_id>_<user_id> to just <user_id> in Spice DB for subjects.

One side effect/bug that occurs due to this PR is as follows: I created domain_1 with ID 88724ac6-a569-43c2-a895-d8d3e93ad3df and domain_2 with ID 716a571e-fbe7-4cd2-afdc-fa0780a24531.

Then, I created a client, client_1_of_domain_2, with ID 1729828a-3c3d-4917-8678-b76abd28576a, in domain_2 (ID 716a571e-fbe7-4cd2-afdc-fa0780a24531).

However, when I try to view client_1_of_domain_2 with domain_1 in the URL (e.g., http://localhost/domain_1/clients/client_1_of_domain_2), it shows the details of client_1_of_domain_2 from domain_2. Ideally, I should see a "not found" message instead. (edited)

I think we should find another way to solve the issue.

internal/proto/token/v1/token.proto

arvindh123 · 2025-03-14T11:17:16Z

journal/middleware/authorization.go

@@ -39,7 +39,7 @@ func (am *authorizationMiddleware) RetrieveAll(ctx context.Context, session smqa
 	permission := readPermission
 	objectType := page.EntityType.String()
 	object := page.EntityID
-	subject := session.DomainUserID
+	subject := session.Subject


Why this was changed to Subject

I changed this to Subject as it can be just the userID or the domainUserId. Should I revert this?

DomainUserID means id of user within the domain, which is different from actual user id for platform ,
In each domain, user should have unquie id .
So just we are adding prefix domain_id to user id to get the unqiue user id for user per domain , which is called domain user id

auth/service.go

auth/jwt/tokenizer.go

domains/middleware/authorization.go

auth/keys.go

pkg/authn/authn.go

api/http/authn.go

internal/proto/auth/v1/auth.proto

domains/middleware/authorization.go

dborovcanin

@arvindh123 Please re-review.

arvindh123

LGTM

Signed-off-by: Felix Gateru <felix.gateru@gmail.com>

…ct in auth key Signed-off-by: Felix Gateru <felix.gateru@gmail.com>

Signed-off-by: Felix Gateru <felix.gateru@gmail.com>

felixgateru force-pushed the smq2609 branch from 99d8100 to a06c394 Compare February 10, 2025 14:44

felixgateru marked this pull request as ready for review February 10, 2025 14:44

felixgateru requested a review from a team as a code owner February 10, 2025 14:44

felixgateru force-pushed the smq2609 branch from a06c394 to a536135 Compare February 10, 2025 14:52

arvindh123 requested changes Feb 11, 2025

View reviewed changes

pkg/authz/authsvc/authz.go Outdated Show resolved Hide resolved

groups/service.go Outdated Show resolved Hide resolved

felixgateru mentioned this pull request Feb 11, 2025

Update listing of groups by userID #2704

Closed

felixgateru force-pushed the smq2609 branch 2 times, most recently from 7512024 to 47452d5 Compare February 17, 2025 08:48

dborovcanin force-pushed the smq2609 branch from 47452d5 to 3c8457b Compare February 26, 2025 13:34

arvindh123 requested changes Mar 7, 2025

View reviewed changes

felixgateru force-pushed the smq2609 branch from 3c8457b to e169839 Compare March 7, 2025 11:31

felixgateru force-pushed the smq2609 branch from e169839 to 37c6e22 Compare March 13, 2025 13:54

felixgateru changed the title ~~SMQ-2609 - Remove DomainUserID encoding from session and authn~~ SMQ-2609 - Enable superadmin to perform actions over entities Mar 13, 2025

felixgateru force-pushed the smq2609 branch 2 times, most recently from 0f04178 to f9e1685 Compare March 13, 2025 14:55

arvindh123 requested changes Mar 14, 2025

View reviewed changes

auth/service.go Show resolved Hide resolved

auth/service.go Show resolved Hide resolved

felixgateru force-pushed the smq2609 branch 2 times, most recently from b0ae01d to ec39e29 Compare March 20, 2025 08:06

arvindh123 requested changes Mar 20, 2025

View reviewed changes

auth/jwt/tokenizer.go Outdated Show resolved Hide resolved

domains/middleware/authorization.go Outdated Show resolved Hide resolved

auth/keys.go Show resolved Hide resolved

pkg/authn/authn.go Show resolved Hide resolved

felixgateru force-pushed the smq2609 branch 2 times, most recently from d97fe33 to 6c72b27 Compare March 26, 2025 11:38

arvindh123 reviewed Mar 28, 2025

View reviewed changes

api/http/authn.go Show resolved Hide resolved

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

domains/middleware/authorization.go Show resolved Hide resolved

felixgateru force-pushed the smq2609 branch 2 times, most recently from c48025d to c9423f0 Compare April 7, 2025 11:04

dborovcanin force-pushed the smq2609 branch from c9423f0 to c5aaa01 Compare April 8, 2025 20:25

dborovcanin approved these changes Apr 9, 2025

View reviewed changes

arvindh123 force-pushed the smq2609 branch from c5aaa01 to c224d98 Compare April 10, 2025 11:22

arvindh123 approved these changes Apr 10, 2025

View reviewed changes

felixgateru added 10 commits April 10, 2025 16:47

refactor: use subject for authorization

bfbe5d4

Signed-off-by: Felix Gateru <felix.gateru@gmail.com>

ci: fix linter warnings

f5ef9e0

Signed-off-by: Felix Gateru <felix.gateru@gmail.com>

tests: fix auth tests

7c8a6aa

Signed-off-by: Felix Gateru <felix.gateru@gmail.com>

tests: fix keys api tests

39f737a

Signed-off-by: Felix Gateru <felix.gateru@gmail.com>

tests: fix users tests

a0b380d

Signed-off-by: Felix Gateru <felix.gateru@gmail.com>

refactor: revert domainuserid name change

905ff60

Signed-off-by: Felix Gateru <felix.gateru@gmail.com>

feat: add role check before issuing token

3a76f4e

Signed-off-by: Felix Gateru <felix.gateru@gmail.com>

tests: fix keys tests

f939204

Signed-off-by: Felix Gateru <felix.gateru@gmail.com>

refactor: remove domain and user from auth key, store userid as subje…

775d66e

…ct in auth key Signed-off-by: Felix Gateru <felix.gateru@gmail.com>

refactor: refactor authenticate middleware, improve comment

98a06c0

Signed-off-by: Felix Gateru <felix.gateru@gmail.com>

dborovcanin force-pushed the smq2609 branch from c224d98 to 98a06c0 Compare April 10, 2025 14:47

dborovcanin merged commit 299cee7 into absmach:main Apr 10, 2025
8 of 9 checks passed

felixgateru deleted the smq2609 branch May 29, 2025 09:17

arvindh123 mentioned this pull request Jul 23, 2025

Feature: Adding Super admin role in authentication response #2393

Open

SMQ-2609 - Enable superadmin to perform actions over entities #2688

SMQ-2609 - Enable superadmin to perform actions over entities #2688

Uh oh!

Conversation

felixgateru commented Feb 7, 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 Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

dborovcanin commented Feb 26, 2025

Uh oh!

arvindh123 Mar 7, 2025

Choose a reason for hiding this comment

Uh oh!

arvindh123 commented Mar 7, 2025

Uh oh!

arvindh123 commented Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

arvindh123 Mar 14, 2025

Choose a reason for hiding this comment

Uh oh!

felixgateru Mar 14, 2025

Choose a reason for hiding this comment

Uh oh!

arvindh123 Mar 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dborovcanin left a comment

Choose a reason for hiding this comment

Uh oh!

arvindh123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Feb 10, 2025 •

edited

Loading

arvindh123 commented Mar 7, 2025 •

edited

Loading