endpoint for status of the user #162

danielradu10 · 2025-04-30T11:33:46Z

No description provided.

sstanculeanu · 2025-05-05T08:25:35Z

api/groups/common_test.go

 					{Name: "/sign-multiple-transactions", Open: true},
 					{Name: "/set-security-mode", Open: true},
 					{Name: "/unset-security-mode", Open: true},
+					{Name: "/security-status", Open: true},


as this can be extended in the future, perhaps we rename it to something more generic like user-status

sstanculeanu · 2025-05-05T08:37:43Z

api/groups/guardianGroup.go

+		logUserStatusRequest(userIp, userAgent, &request, debugErr)
+	}()
+
+	err := json.NewDecoder(c.Request.Body).Decode(&request)


I don't think request body and UserStatusRequest is needed here.. perhaps it can be something similar to node's /address/:address endpoint instead (/guardian/user-status/erd1....)

sstanculeanu · 2025-05-05T08:40:44Z

cmd/multi-factor-auth/config/api.toml

        { Name = "/sign-multiple-transactions", Open = true, Auth = false, MaxContentLength = 1500000 },
        { Name = "/set-security-mode", Open = true, Auth = false, MaxContentLength = 200 },
        { Name = "/unset-security-mode", Open = true, Auth = false, MaxContentLength = 200 },
+        { Name = "/security-status", Open = true, Auth = false, MaxContentLength = 200 },


MaxContentLength not needed, it does not apply for get requests

sstanculeanu · 2025-05-05T08:41:12Z

core/constants.go

 // NoExpiryValue is the returned value for a persistent key expiry time
 const NoExpiryValue = -1
+
+type Status int


missing comments on new fields

sstanculeanu · 2025-05-05T08:41:38Z

core/requests/types.go

+	UserAddr string `json:"user"`
+}
+
+// UserStatusResponse - is the JSON response for the user status interrogation


Suggested change

// UserStatusResponse - is the JSON response for the user status interrogation

// UserStatusResponse is the JSON response for the user status request

sstanculeanu · 2025-05-05T08:51:38Z

resolver/serviceResolver.go

 	return resolver.verifyCodesReturningGuardian(userAddress, txs[0].GuardianAddr, userIp, code, secondCode)
 }

+func (resolver *serviceResolver) verifyUserReturningSecurityStatus(userAddr string) (core.Status, error) {


Suggested change

func (resolver *serviceResolver) verifyUserReturningSecurityStatus(userAddr string) (core.Status, error) {

func (resolver *serviceResolver) getUserStatus(userAddr string) (core.Status, error) {

no verification done here

sstanculeanu · 2025-05-05T08:52:21Z

resolver/serviceResolver.go

+func (resolver *serviceResolver) verifyUserReturningSecurityStatus(userAddr string) (core.Status, error) {
+	userAddress, err := sdkData.NewAddressFromBech32String(userAddr)
+	if err != nil {
+		return 0, err


Suggested change

return 0, err

return core.NotSet, err

not needed now

sstanculeanu · 2025-05-05T08:52:58Z

resolver/serviceResolver.go

+	addressBytes := userAddress.AddressBytes()
+	resolver.userCritSection.RLock(string(addressBytes))
+	_, err = resolver.getUserInfo(addressBytes)
+	resolver.userCritSection.RUnlock(string(addressBytes))
+	if err != nil {
+		return -1, err
+	}


I think this can be removed 🤔

removed it now

sstanculeanu · 2025-05-05T08:53:43Z

testscommon/rateLimiterStub.go

+	if r.GetSecurityStatusCalled != nil {
+		return r.GetSecurityStatusCalled(key)
+	}
+	return -1


Suggested change

return -1

return core.NotSet

sstanculeanu · 2025-05-05T08:54:00Z

testscommon/rateLimiterMock.go


+// GetSecurityStatus -
+func (r *RateLimiterMock) GetSecurityStatus(key string) core.Status {
+	return 0


Suggested change

return 0

return core.NotSet

ssd04 · 2025-05-05T08:53:31Z

api/groups/guardianGroup.go

 	signMultipleTransactionsPath  = "/sign-multiple-transactions"
 	setSecurityModeNoExpirePath   = "/set-security-mode"
 	unsetSecurityModeNoExpirePath = "/unset-security-mode"
+	getSecurityStatus             = "/security-status"


rename to getSecurityStatusPath?

ssd04 · 2025-05-05T08:54:18Z

core/constants.go

+type Status int
+
+const (
+	NotSet Status = iota
+	ManualSet
+	AutomaticallySet


add missing comments

ssd04 · 2025-05-05T08:55:22Z

go.sum

are these changes needed? did you run go mod tidy?

ssd04 · 2025-05-05T08:55:43Z

handlers/secureOtp/secureOtpHandler.go

 	return totp.rateLimiter.UnsetSecurityModeNoExpire(key)
 }

+// GetSecurityStatus - returns the status of the security mode


Suggested change

// GetSecurityStatus - returns the status of the security mode

// GetSecurityStatus returns the status of the security mode

?

ssd04 · 2025-05-05T09:02:13Z

resolver/serviceResolver.go

+	_, err = resolver.getUserInfo(addressBytes)
+	resolver.userCritSection.RUnlock(string(addressBytes))
+	if err != nil {
+		return -1, err


what -1 means here?

set it as a error value, not needed it anymore

is this -1 still needed?

no, all those lines are removed now.

sstanculeanu · 2025-05-05T14:32:02Z

cmd/multi-factor-auth/swagger/data.go


-// swagger:route GET /security-status Guardian getSecurityStatus
-// Returns the security status.
+// swagger:route GET /user-status Guardian getSecurityStatus


Suggested change

// swagger:route GET /user-status Guardian getSecurityStatus

// swagger:route GET /user-status Guardian getUserStatus

sstanculeanu · 2025-05-05T14:32:11Z

cmd/multi-factor-auth/swagger/data.go

-// swagger:route GET /security-status Guardian getSecurityStatus
-// Returns the security status.
+// swagger:route GET /user-status Guardian getSecurityStatus
+// Returns the security status of the user.


Suggested change

// Returns the security status of the user.

// Returns the status of the user.

sstanculeanu · 2025-05-05T14:33:05Z

api/groups/guardianGroup_test.go


 		facade := mockFacade.GuardianFacadeStub{
-			GetSecurityStatusCalled: func(request requests.UserStatusRequest) (*requests.UserStatusResponse, error) {
+			GetSecurityStatusCalled: func(userAddress string) (*requests.UserStatusResponse, error) {


rename this as well to GetUserStatusCalled

sstanculeanu · 2025-05-05T14:33:42Z

redis/redisLimiter.go

-	if trials < maxFailures && expTime != core.NoExpiryValue {
+	trials, err := strconv.ParseInt(dbVal, 10, 64)
+	if err != nil {
+		log.Debug("error when returning security status", "err", err)


Suggested change

log.Debug("error when returning security status", "err", err)

log.Debug("error when converting security status", "err", err)

sstanculeanu · 2025-05-05T14:34:00Z

resolver/serviceResolver.go

-// GetSecurityStatus gets the user's security status
-func (resolver *serviceResolver) GetSecurityStatus(request requests.UserStatusRequest) (*requests.UserStatusResponse, error) {
-	status, err := resolver.verifyUserReturningSecurityStatus(request.UserAddr)
+// GetUserStatus gets the user's security status


Suggested change

// GetUserStatus gets the user's security status

// GetUserStatus gets the user's status

sstanculeanu · 2025-05-05T14:34:50Z

testscommon/serviceResolverStub.go

 	SetSecurityModeNoExpireCalled   func(userIp string, request requests.SecurityModeNoExpire) (*requests.OTPCodeVerifyData, error)
 	UnsetSecurityModeNoExpireCalled func(userIp string, request requests.SecurityModeNoExpire) (*requests.OTPCodeVerifyData, error)
-	GetSecurityStatusCalled         func(request requests.UserStatusRequest) (*requests.UserStatusResponse, error)
+	GetSecurityStatusCalled         func(userAddress string) (*requests.UserStatusResponse, error)


Suggested change

GetSecurityStatusCalled func(userAddress string) (*requests.UserStatusResponse, error)

GetUserStatusCalled func(userAddress string) (*requests.UserStatusResponse, error)

ssd04 · 2025-05-05T16:06:17Z

cmd/multi-factor-auth/swagger/data.go

i think we need to also generate swagger files based on these changes

fix native auth whitelister check

ssd04 · 2025-06-30T11:00:21Z

cmd/multi-factor-auth/swagger/ui/swagger.json

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+


Suggested change

remove these empty lines?

sstanculeanu

Observations after testing:

swagger not working, could not add parameters on the request

AdoAdoAdo · 2025-07-01T07:58:48Z

cmd/multi-factor-auth/swagger/ui/swagger.json

      }
    },
+    "userStatusResponse": {
+      "description": "The status of the operation",


status of which operation?

this should be the address/user status on the TCS

AdoAdoAdo · 2025-07-01T08:00:40Z

core/constants.go

 const NoExpiryValue = -1
+
+// Status represents the status of the security mode
+type Status int


I think this is too generic, especially when used for the security mode

maybe EnhancedSecurityModeStatus

AdoAdoAdo · 2025-07-01T08:02:15Z

core/requests/types.go


+// UserStatusResponse is the JSON response for the user status interrogation
+type UserStatusResponse struct {
+	SecurityStatus int `json:"status"`


Maybe rename to SecurityModeStatus? or EnhancedSecurityModeStatus?

SecurityStatus = NotSet doesn't sound good, as the account has protection via regular 2FA

AdoAdoAdo · 2025-07-01T08:10:55Z

resolver/serviceResolver.go

+	_, err = resolver.getUserInfo(addressBytes)
+	resolver.userCritSection.RUnlock(string(addressBytes))
+	if err != nil {
+		return -1, err


is this -1 still needed?

The base branch was changed.

danielradu10 added 9 commits April 30, 2025 14:32

added redis limiter methods and unit tests

faae299

added security status endpoint

20f72fa

added unit tests

65bbe37

fix

2c4b6c6

fix go.mod

64c67ab

added getter for value of redis key and consider

4577031

linter fix

bec08bc

small fix

44790bf

fixes and swagger

27d71fe

sstanculeanu self-requested a review May 5, 2025 08:24

sstanculeanu reviewed May 5, 2025

View reviewed changes

ssd04 reviewed May 5, 2025

View reviewed changes

fixes after review

7c9c4d3

sstanculeanu reviewed May 5, 2025

View reviewed changes

fixes after reviews

a32ad2a

ssd04 reviewed May 5, 2025

View reviewed changes

ssd04 and others added 5 commits May 6, 2025 13:36

fix native auth whitelister check

e1d40c4

remove debug log

bf251d2

Merge pull request #163 from multiversx/fix-nativeauth-whitelist-check

df88e72

fix native auth whitelister check

swagger update

62a028f

Merge branch 'main' into user-status

4970e25

sstanculeanu approved these changes Jun 30, 2025

View reviewed changes

danielradu10 changed the base branch from main to rc/barnard June 30, 2025 10:48

ssd04 approved these changes Jun 30, 2025

View reviewed changes

danielradu10 added 2 commits June 30, 2025 14:01

Merge branch 'rc/barnard' into user-status

b3d0dc6

removed empty lines

18481a3

sstanculeanu approved these changes Jun 30, 2025

View reviewed changes

ssd04 approved these changes Jun 30, 2025

View reviewed changes

sstanculeanu requested changes Jun 30, 2025

View reviewed changes

fixed swagger

603a2bc

ssd04 approved these changes Jul 1, 2025

View reviewed changes

fixed swagger response for user-status

c640ac1

AdoAdoAdo reviewed Jul 1, 2025

View reviewed changes

danielradu10 added 2 commits July 1, 2025 13:04

fixes after review

6e80038

swagger fix

5b4b354

sstanculeanu previously approved these changes Jul 1, 2025

View reviewed changes

ssd04 previously approved these changes Jul 1, 2025

View reviewed changes

Base automatically changed from rc/barnard to main July 17, 2025 14:32

	// UserStatusResponse - is the JSON response for the user status interrogation
	// UserStatusResponse is the JSON response for the user status request

	func (resolver *serviceResolver) verifyUserReturningSecurityStatus(userAddr string) (core.Status, error) {
	func (resolver *serviceResolver) getUserStatus(userAddr string) (core.Status, error) {

	// GetSecurityStatus - returns the status of the security mode
	// GetSecurityStatus returns the status of the security mode

	// swagger:route GET /user-status Guardian getSecurityStatus
	// swagger:route GET /user-status Guardian getUserStatus

	// Returns the security status of the user.
	// Returns the status of the user.

	log.Debug("error when returning security status", "err", err)
	log.Debug("error when converting security status", "err", err)

	// GetUserStatus gets the user's security status
	// GetUserStatus gets the user's status

	GetSecurityStatusCalled func(userAddress string) (*requests.UserStatusResponse, error)
	GetUserStatusCalled func(userAddress string) (*requests.UserStatusResponse, error)

endpoint for status of the user #162

Are you sure you want to change the base?

endpoint for status of the user #162

Uh oh!

Conversation

danielradu10 commented Apr 30, 2025

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!

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!

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!

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!

Choose a reason for hiding this comment