MG-2048 - Authorize things and users with PATs #2499

nyagamunene · 2024-10-31T09:45:38Z

What type of PR is this?

This is a feature because it adds the following functionality: It adds authorization to things and users using PATs.

What does this do?

It adds PATs authorization to things and users middleware

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

Related Issue Implement Personal Access Tokens (PATs) #2048
Resolves #

Have you included tests for your changes?

Yes

Did you document any new/modified feature?

No

Notes

arvindh123 · 2024-11-12T09:04:59Z

clients/middleware/authorization.go

@@ -229,7 +384,7 @@ func (am *authorizationMiddleware) RemoveParentGroup(ctx context.Context, sessio
 	}

 	if th.ParentGroup != "" {
-		if err := am.extAuthorize(ctx, clients.GroupOpSetChildClient, authz.PolicyReq{
+		if err := am.extAuthorize(ctx, clients.GroupOpSetChildThing, authz.PolicyReq{
 			Domain:      session.DomainID,


Suggested change

Domain: session.DomainID,

if err := am.extAuthorize(ctx, clients.GroupOpSetChildClient, authz.PolicyReq{

arvindh123 · 2024-11-12T09:05:08Z

clients/middleware/authorization.go

@@ -200,7 +341,7 @@ func (am *authorizationMiddleware) SetParentGroup(ctx context.Context, session a
 		return errors.Wrap(err, errSetParentGroup)
 	}

-	if err := am.extAuthorize(ctx, clients.GroupOpSetChildClient, authz.PolicyReq{
+	if err := am.extAuthorize(ctx, clients.GroupOpSetChildThing, authz.PolicyReq{
 		Domain:      session.DomainID,


Suggested change

Domain: session.DomainID,

if err := am.extAuthorize(ctx, clients.GroupOpSetChildClient, authz.PolicyReq{

Domain: session.DomainID,

arvindh123 · 2024-11-12T09:11:24Z

internal/api/auth.go

+			resp.Type = mgauthn.AccessToken
+			if strings.HasPrefix(token, "pat"+seperator) {
+				resp.Type = mgauthn.PersonalAccessToken
+			}


This can be done in auth service, During authentication process.
Is there any reason behind this ?

arvindh123 · 2024-11-12T09:31:17Z

internal/proto/auth/v1/auth.proto

@@ -10,6 +10,7 @@ option go_package = "github.com/absmach/magistrala/internal/grpc/auth/v1";
 // functionalities for magistrala services.
 service AuthService {
  rpc Authorize(AuthZReq) returns (AuthZRes) {}
+  rpc AuthorizePAT(AuthZpatReq) returns (AuthZRes) {}
  rpc Authenticate(AuthNReq) returns (AuthNRes) {}


Lets have seperate RPC for PAT Authentication like PAT,

arvindh123 · 2024-11-12T09:37:23Z

auth/api/http/pats/transport.go

+		r.Post("/authorize", kithttp.NewServer(
+			(authorizePATEndpoint(svc)),
+			decodeAuthorizePATRequest,
+			api.EncodeResponse,
+			opts...,
+		).ServeHTTP)


Please remove this endpoint, For now we can provide authoirzePAT via gRPC only.

arvindh123 · 2024-11-12T09:39:40Z

auth/service.go

+	if strings.HasPrefix(token, patPrefix+patSecretSeparator) {
+		pat, err := svc.IdentifyPAT(ctx, token)
+		if err != nil {
+			return Key{}, err
+		}
+		return Key{
+			ID:        pat.ID,
+			Type:      PersonalAccessToken,
+			Subject:   pat.User,
+			User:      pat.User,
+			IssuedAt:  pat.IssuedAt,
+			ExpiresAt: pat.ExpiresAt,
+		}, nil
+	}


Move this logic to Authenticate function in file pkg/authn/authsvc/authn.go

Lets have seperate RPC for IdentifyPAT.

The Authenticate function in file pkg/authn/authsvc/authn.go will call IdentifyPAT base on token prefix

arvindh123 · 2024-11-12T09:41:44Z

pkg/authn/authsvc/authn.go

 	}
-	return authn.Session{DomainUserID: res.GetId(), UserID: res.GetUserId(), DomainID: res.GetDomainId()}, nil
+	return authn.Session{ID: res.GetId(), UserID: res.GetUserId(), DomainID: res.GetDomainId()}, nil
 }


Lets have seperate RPC call IdentifyPAT, move the logic to here

Something like below code

Suggested change

}

return authn.Session{DomainUserID: res.GetId(), UserID: res.GetUserId(), DomainID: res.GetDomainId()}, nil

return authn.Session{ID: res.GetId(), UserID: res.GetUserId(), DomainID: res.GetDomainId()}, nil

}

switch {

case strings.HasPrefix(token, patPrefix+patSecretSeparator):

res, err := a.authSvcClient.AuthenticatePAT(ctx, token)

if err != nil {

return authn.Session{}, errors.Wrap(errors.ErrAuthentication, err)

}

return authn.Session{ID: res.GetId(), UserID: res.GetUserId(), DomainID: res.GetDomainId()}, nil

default:

res, err := a.authSvcClient.Authenticate(ctx, &grpcAuthV1.AuthNReq{Token: token})

if err != nil {

return authn.Session{}, errors.Wrap(errors.ErrAuthentication, err)

}

return authn.Session{ID: res.GetId(), UserID: res.GetUserId(), DomainID: res.GetDomainId()}, nil

}

arvindh123 · 2024-11-12T12:19:40Z

users/middleware/authorization.go

@@ -179,6 +390,19 @@ func (am *authorizationMiddleware) Delete(ctx context.Context, session authn.Ses
 		session.SuperAdmin = true
 	}



Do PAT Authorization before checkAdmin or UserAuthz

Signed-off-by: nyagamunene <[email protected]>

nyagamunene self-assigned this Oct 31, 2024

nyagamunene force-pushed the AuthorizeUsersThings branch from 001bcaa to d9f739e Compare October 31, 2024 12:35

nyagamunene changed the title ~~MG-2048 - Authorize things with PATs~~ MG-2048 - Authorize things and users with PATs Nov 3, 2024

nyagamunene force-pushed the AuthorizeUsersThings branch 2 times, most recently from 56daa42 to 923af86 Compare November 5, 2024 12:52

nyagamunene changed the base branch from main to auth-refactor November 7, 2024 10:57

nyagamunene force-pushed the AuthorizeUsersThings branch from f204e8c to 00cb9c6 Compare November 11, 2024 14:49

nyagamunene marked this pull request as ready for review November 11, 2024 14:50

arvindh123 requested changes Nov 12, 2024

View reviewed changes

arvindh123 force-pushed the auth-refactor branch 2 times, most recently from 3f43476 to 4323d34 Compare November 12, 2024 18:21

dborovcanin force-pushed the auth-refactor branch from 4323d34 to c04c035 Compare November 12, 2024 18:49

nyagamunene force-pushed the AuthorizeUsersThings branch from 00cb9c6 to 2e489f4 Compare November 13, 2024 13:15

nyagamunene added 16 commits November 13, 2024 16:46

First implementation

c790c88

Signed-off-by: nyagamunene <[email protected]>

Update middleware, service and api

a585fe7

Signed-off-by: nyagamunene <[email protected]>

Add grpc authorizePAT

18ba5d6

Signed-off-by: nyagamunene <[email protected]>

Move pat to auth

dd780d5

Signed-off-by: nyagamunene <[email protected]>

Update grpc api

b0ca4cd

Signed-off-by: nyagamunene <[email protected]>

Update cmd file

96a0228

Signed-off-by: nyagamunene <[email protected]>

Update pat service

822a938

Signed-off-by: nyagamunene <[email protected]>

Update nginx variables

a4d9990

Signed-off-by: nyagamunene <[email protected]>

Update scope documentation

0fa018a

Signed-off-by: nyagamunene <[email protected]>

Increase scope coverage

b237399

Signed-off-by: nyagamunene <[email protected]>

fix failing linter

a23c0d6

Signed-off-by: nyagamunene <[email protected]>

Update protoc version

735e496

Signed-off-by: nyagamunene <[email protected]>

Add authenticate PAT grpc endpoint

a9fbefa

Signed-off-by: nyagamunene <[email protected]>

Revert Authenticate method

1702e92

Signed-off-by: nyagamunene <[email protected]>

Add grpc authorizePAT

fb2775a

Signed-off-by: nyagamunene <[email protected]>

Update grpc api

af519d5

Signed-off-by: nyagamunene <[email protected]>

nyagamunene added 23 commits November 13, 2024 16:52

Add grpc authorizePAT

71c6641

Signed-off-by: nyagamunene <[email protected]>

Initial implementation of Authorize

9c0cfb5

Signed-off-by: nyagamunene <[email protected]>

Remove pat folder

3baa1a4

Signed-off-by: nyagamunene <[email protected]>

Update user and things middleware

81bb98b

Signed-off-by: nyagamunene <[email protected]>

Authorize and Authenticate method

8cccd26

Signed-off-by: nyagamunene <[email protected]>

Update returned errors

04f6098

Signed-off-by: nyagamunene <[email protected]>

Update variable assignment

bcd7d13

Signed-off-by: nyagamunene <[email protected]>

Update any ids method

3158019

Signed-off-by: nyagamunene <[email protected]>

Update users middleware

547deaf

Signed-off-by: nyagamunene <[email protected]>

Update middleware, service and api

1f13e88

Signed-off-by: nyagamunene <[email protected]>

Update protoc version

dccf332

Signed-off-by: nyagamunene <[email protected]>

Update middleware, service and api

cd65cda

Signed-off-by: nyagamunene <[email protected]>

Add grpc authorizePAT

a4b4101

Signed-off-by: nyagamunene <[email protected]>

Initial implementation of Authorize

3037fd0

Signed-off-by: nyagamunene <[email protected]>

Remove pat folder

76797a7

Signed-off-by: nyagamunene <[email protected]>

Update user and things middleware

baf2466

Signed-off-by: nyagamunene <[email protected]>

Authorize and Authenticate method

68742b2

Signed-off-by: nyagamunene <[email protected]>

Update any ids method

5c4944f

Signed-off-by: nyagamunene <[email protected]>

Update users middleware

e75c2b3

Signed-off-by: nyagamunene <[email protected]>

Resolve conflicts

a83b000

Signed-off-by: nyagamunene <[email protected]>

Update protoc and mocks

816a51c

Signed-off-by: nyagamunene <[email protected]>

Address comments

f02670b

Signed-off-by: nyagamunene <[email protected]>

Resolve conflicts

511e918

Signed-off-by: nyagamunene <[email protected]>

nyagamunene force-pushed the AuthorizeUsersThings branch from 2e489f4 to 511e918 Compare November 13, 2024 14:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MG-2048 - Authorize things and users with PATs #2499

MG-2048 - Authorize things and users with PATs #2499

nyagamunene commented Oct 31, 2024

arvindh123 Nov 12, 2024

arvindh123 Nov 12, 2024

arvindh123 Nov 12, 2024

arvindh123 Nov 12, 2024

arvindh123 Nov 12, 2024

arvindh123 Nov 12, 2024

arvindh123 Nov 12, 2024

arvindh123 Nov 12, 2024

	Domain: session.DomainID,
	if err := am.extAuthorize(ctx, clients.GroupOpSetChildClient, authz.PolicyReq{

-	}
-	return authn.Session{DomainUserID: res.GetId(), UserID: res.GetUserId(), DomainID: res.GetDomainId()}, nil
-	return authn.Session{ID: res.GetId(), UserID: res.GetUserId(), DomainID: res.GetDomainId()}, nil
-}
+	switch {
+	case strings.HasPrefix(token, patPrefix+patSecretSeparator):
+		res, err := a.authSvcClient.AuthenticatePAT(ctx, token)
+		if err != nil {
+			return authn.Session{}, errors.Wrap(errors.ErrAuthentication, err)
+		}
+		return authn.Session{ID: res.GetId(), UserID: res.GetUserId(), DomainID: res.GetDomainId()}, nil
+	default:
+		res, err := a.authSvcClient.Authenticate(ctx, &grpcAuthV1.AuthNReq{Token: token})
+		if err != nil {
+			return authn.Session{}, errors.Wrap(errors.ErrAuthentication, err)
+		}
+		return authn.Session{ID: res.GetId(), UserID: res.GetUserId(), DomainID: res.GetDomainId()}, nil
+	}

		@@ -179,6 +390,19 @@ func (am *authorizationMiddleware) Delete(ctx context.Context, session authn.Ses
		session.SuperAdmin = true
		}

MG-2048 - Authorize things and users with PATs #2499

Are you sure you want to change the base?

MG-2048 - Authorize things and users with PATs #2499

Conversation

nyagamunene commented Oct 31, 2024

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment