endpoint for status of the user by danielradu10 · Pull Request #162 · multiversx/mx-multi-factor-auth-go-service

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

No description provided.

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

 					{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

+		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

        { 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

 // 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

+	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

 	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

+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

+	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

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


Suggested change

return -1

return core.NotSet

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


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


Suggested change

return 0

return core.NotSet

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

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


rename to getSecurityStatusPath?

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

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


add missing comments

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

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

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

 	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

+	_, 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


-// 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

-// 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


 		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

-	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

-// 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

 	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

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

fix native auth whitelister check

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

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


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

      }
    },
+    "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

 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


+// 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

+	_, 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)

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!