Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/groups/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func getServiceRoutesConfig() config.ApiRoutesConfig {
{Name: "/sign-multiple-transactions", Open: true},
{Name: "/set-security-mode", Open: true},
{Name: "/unset-security-mode", Open: true},
{Name: "/security-status", Open: true},
{Name: "/user-status/:address", Open: true},
{Name: "/debug", Open: true},
{Name: "/verify-code", Open: true},
{Name: "/registered-users", Open: true},
Expand Down
27 changes: 11 additions & 16 deletions api/groups/guardianGroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const (
signMultipleTransactionsPath = "/sign-multiple-transactions"
setSecurityModeNoExpirePath = "/set-security-mode"
unsetSecurityModeNoExpirePath = "/unset-security-mode"
getSecurityStatus = "/security-status"
getUserStatusPath = "/user-status/:address"
registerPath = "/register"
verifyCodePath = "/verify-code"
registeredUsersPath = "/registered-users"
Expand Down Expand Up @@ -105,9 +105,9 @@ func NewGuardianGroup(facade shared.FacadeHandler) (*guardianGroup, error) {
Handler: gg.config,
},
{
Path: getSecurityStatus,
Path: getUserStatusPath,
Method: http.MethodGet,
Handler: gg.getSecurityStatus,
Handler: gg.getUserStatus,
},
}
gg.endpoints = endpoints
Expand Down Expand Up @@ -243,23 +243,18 @@ func (gg *guardianGroup) unsetSecurityModeNoExpire(c *gin.Context) {
returnStatus(c, nil, http.StatusOK, "", chainApiShared.ReturnCodeSuccess)
}

func (gg *guardianGroup) getSecurityStatus(c *gin.Context) {
var request requests.UserStatusRequest
func (gg *guardianGroup) getUserStatus(c *gin.Context) {
var debugErr error

userIp := c.GetString(mfaMiddleware.UserIpKey)
userAgent := c.GetString(mfaMiddleware.UserAgentKey)
userAddr := c.Param("address")

defer func() {
logUserStatusRequest(userIp, userAgent, &request, debugErr)
logUserStatusRequest(userIp, userAgent, userAddr, debugErr)
}()

err := json.NewDecoder(c.Request.Body).Decode(&request)
if err != nil {
debugErr = fmt.Errorf("%w while decoding request", err)
returnStatus(c, nil, http.StatusBadRequest, err.Error(), chainApiShared.ReturnCodeRequestError)
return
}
status, err := gg.facade.GetSecurityStatus(request)
status, err := gg.facade.GetUserStatus(userAddr)
if err != nil {
debugErr = fmt.Errorf("%w while interrogating security status", err)
handleErrorAndReturn(c, status, err.Error())
Expand All @@ -270,12 +265,12 @@ func (gg *guardianGroup) getSecurityStatus(c *gin.Context) {
returnStatus(c, status, http.StatusOK, "", chainApiShared.ReturnCodeSuccess)
}

func logUserStatusRequest(userIp string, userAgent string, request *requests.UserStatusRequest, debugErr error) {
func logUserStatusRequest(userIp string, userAgent string, userAddr string, debugErr error) {
logArgs := []interface{}{
"route", getSecurityStatus,
"route", getUserStatusPath,
"ip", userIp,
"user agent", userAgent,
"userAddr", getPrintableData(request.UserAddr),
"userAddr", userAddr,
}
defer func() {
guardianLog.Info("Request info", logArgs...)
Expand Down
33 changes: 4 additions & 29 deletions api/groups/guardianGroup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -519,30 +519,11 @@ func TestGuardianGroup_UnsetSecurityModeNoExpire(t *testing.T) {
func TestGuardianGroup_getSecurityStatus(t *testing.T) {
t.Parallel()

t.Run("empty body", func(t *testing.T) {
t.Parallel()

gg, _ := groups.NewGuardianGroup(&mockFacade.GuardianFacadeStub{})

ws := startWebServer(gg, "guardian", getServiceRoutesConfig(), providedAddr)

req, _ := http.NewRequest("GET", "/guardian/security-status", strings.NewReader(""))
resp := httptest.NewRecorder()
ws.ServeHTTP(resp, req)

statusRsp := generalResponse{}
loadResponse(resp.Body, &statusRsp)

assert.Nil(t, statusRsp.Data)
assert.True(t, strings.Contains(statusRsp.Error, "EOF"))
require.Equal(t, http.StatusBadRequest, resp.Code)
})

t.Run("facade returns error", func(t *testing.T) {
t.Parallel()

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename this as well to GetUserStatusCalled

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed

return &requests.UserStatusResponse{SecurityStatus: -1}, expectedError
},
}
Expand All @@ -551,10 +532,7 @@ func TestGuardianGroup_getSecurityStatus(t *testing.T) {

ws := startWebServer(gg, "guardian", getServiceRoutesConfig(), providedAddr)

request := requests.UserStatusRequest{
UserAddr: providedAddr,
}
req, _ := http.NewRequest("GET", "/guardian/security-status", requestToReader(request))
req, _ := http.NewRequest("GET", "/guardian/user-status/"+providedAddr, nil)
resp := httptest.NewRecorder()
ws.ServeHTTP(resp, req)

Expand All @@ -574,7 +552,7 @@ func TestGuardianGroup_getSecurityStatus(t *testing.T) {
t.Parallel()

facade := mockFacade.GuardianFacadeStub{
GetSecurityStatusCalled: func(request requests.UserStatusRequest) (*requests.UserStatusResponse, error) {
GetSecurityStatusCalled: func(userAddress string) (*requests.UserStatusResponse, error) {
return &requests.UserStatusResponse{
SecurityStatus: 1,
}, nil
Expand All @@ -585,10 +563,7 @@ func TestGuardianGroup_getSecurityStatus(t *testing.T) {

ws := startWebServer(gg, "guardian", getServiceRoutesConfig(), providedAddr)

request := requests.SecurityModeNoExpire{
UserAddr: providedAddr,
}
req, _ := http.NewRequest("GET", "/guardian/security-status", requestToReader(request))
req, _ := http.NewRequest("GET", "/guardian/user-status/"+providedAddr, nil)
resp := httptest.NewRecorder()
ws.ServeHTTP(resp, req)

Expand Down
2 changes: 1 addition & 1 deletion api/shared/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type FacadeHandler interface {
SignMultipleTransactions(userIp string, request requests.SignMultipleTransactions) ([][]byte, *requests.OTPCodeVerifyData, error)
SetSecurityModeNoExpire(userIp string, request requests.SecurityModeNoExpire) (*requests.OTPCodeVerifyData, error)
UnsetSecurityModeNoExpire(userIp string, request requests.SecurityModeNoExpire) (*requests.OTPCodeVerifyData, error)
GetSecurityStatus(request requests.UserStatusRequest) (*requests.UserStatusResponse, error)
GetUserStatus(userAddress string) (*requests.UserStatusResponse, error)
RegisteredUsers() (uint32, error)
TcsConfig() *tcsCore.TcsConfig
GetMetrics() map[string]*requests.EndpointMetricsResponse
Expand Down
2 changes: 1 addition & 1 deletion cmd/multi-factor-auth/config/api.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ RestApiInterface = ":8080" # The interface `address and port` to which the REST
{ 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 },
{ Name = "/user-status/:address", Open = true, Auth = false },
{ Name = "/verify-code", Open = true, Auth = true, MaxContentLength = 200 },
{ Name = "/registered-users", Open = true, Auth = false },
{ Name = "/config", Open = true, Auth = false },
Expand Down
12 changes: 2 additions & 10 deletions cmd/multi-factor-auth/swagger/data.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,8 @@ type _ struct {
Payload requests.SecurityModeNoExpire
}

// swagger:route GET /security-status Guardian getSecurityStatus
// Returns the security status.
// swagger:route GET /user-status Guardian getSecurityStatus
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

// Returns the security status of the user.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

// This request does not need the Authorization header
//
// responses:
Expand All @@ -309,11 +309,3 @@ type _ struct {
Error string `json:"error"`
}
}

// swagger:parameters userStatusRequest
type _ struct {
// UserStatusRequest payload
// in:body
// required:true
Payload requests.UserStatusRequest
}
6 changes: 5 additions & 1 deletion core/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,14 @@ const (
// NoExpiryValue is the returned value for a persistent key expiry time
const NoExpiryValue = -1

// Status represents the status of the security mode
type Status int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing comments on new fields

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

maybe EnhancedSecurityModeStatus

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed


const (
// NotSet means that security mode is not activated
NotSet Status = iota
ManualSet
// ManuallySet means that security mode was activated because of failures
ManuallySet
// AutomaticallySet means that security mode was activated by user with SetSecurityModeNoExpire
AutomaticallySet
)
2 changes: 1 addition & 1 deletion core/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type ServiceResolver interface {
SignMessage(userIp string, request requests.SignMessage) ([]byte, *requests.OTPCodeVerifyData, error)
SetSecurityModeNoExpire(userIp string, request requests.SecurityModeNoExpire) (*requests.OTPCodeVerifyData, error)
UnsetSecurityModeNoExpire(userIp string, request requests.SecurityModeNoExpire) (*requests.OTPCodeVerifyData, error)
GetSecurityStatus(request requests.UserStatusRequest) (*requests.UserStatusResponse, error)
GetUserStatus(userAddr string) (*requests.UserStatusResponse, error)
SignTransaction(userIp string, request requests.SignTransaction) ([]byte, *requests.OTPCodeVerifyData, error)
SignMultipleTransactions(userIp string, request requests.SignMultipleTransactions) ([][]byte, *requests.OTPCodeVerifyData, error)
RegisteredUsers() (uint32, error)
Expand Down
8 changes: 1 addition & 7 deletions core/requests/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,7 @@ type SecurityModeNoExpire struct {
UserAddr string `json:"user"`
}

// UserStatusRequest is the JSON request the service is receiving
// for the user status interrogation
type UserStatusRequest struct {
UserAddr string `json:"user"`
}

// UserStatusResponse - is the JSON response for the user status interrogation
// UserStatusResponse is the JSON response for the user status interrogation
type UserStatusResponse struct {
SecurityStatus int `json:"status"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename to SecurityModeStatus? or EnhancedSecurityModeStatus?

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

}
Expand Down
6 changes: 3 additions & 3 deletions facade/guardianFacade.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ func (gf *guardianFacade) UnsetSecurityModeNoExpire(userIp string, request reque
return gf.serviceResolver.UnsetSecurityModeNoExpire(userIp, request)
}

// GetSecurityStatus returns the user's security status
func (gf *guardianFacade) GetSecurityStatus(request requests.UserStatusRequest) (*requests.UserStatusResponse, error) {
return gf.serviceResolver.GetSecurityStatus(request)
// GetUserStatus returns the user's security status
func (gf *guardianFacade) GetUserStatus(userAddress string) (*requests.UserStatusResponse, error) {
return gf.serviceResolver.GetUserStatus(userAddress)
}

// SignMultipleTransactions validates user's transactions, then adds guardian signature and returns the transaction
Expand Down
11 changes: 5 additions & 6 deletions facade/guardianFacade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,8 @@ func TestGuardianFacade_Getters(t *testing.T) {
}
wasUnsetSecurityModeNoExpireCalled := false

providedUserStatusRequest := requests.UserStatusRequest{
UserAddr: "erd1qyu5wthldzr8wx5c9ucg8kjagg0jfs53s8nr3zpz3hypefsdd8ssycr6th",
}
providedUserAddr := "erd1qyu5wthldzr8wx5c9ucg8kjagg0jfs53s8nr3zpz3hypefsdd8ssycr6th"

expectedUserStatusResponse := requests.UserStatusResponse{SecurityStatus: 1}

args.ServiceResolver = &testscommon.ServiceResolverStub{
Expand Down Expand Up @@ -147,8 +146,8 @@ func TestGuardianFacade_Getters(t *testing.T) {
wasUnsetSecurityModeNoExpireCalled = true
return nil, nil
},
GetSecurityStatusCalled: func(request requests.UserStatusRequest) (*requests.UserStatusResponse, error) {
assert.Equal(t, providedUserStatusRequest, request)
GetSecurityStatusCalled: func(userAddress string) (*requests.UserStatusResponse, error) {
assert.Equal(t, providedUserAddr, providedUserAddr)
return &expectedUserStatusResponse, nil
},
SignTransactionCalled: func(userIp string, request requests.SignTransaction) ([]byte, *requests.OTPCodeVerifyData, error) {
Expand Down Expand Up @@ -216,7 +215,7 @@ func TestGuardianFacade_Getters(t *testing.T) {
assert.Nil(t, err)
assert.True(t, wasUnsetSecurityModeNoExpireCalled)

userStatus, err := facadeInstance.GetSecurityStatus(providedUserStatusRequest)
userStatus, err := facadeInstance.GetUserStatus(providedUserAddr)
assert.Nil(t, err)
assert.Equal(t, &expectedUserStatusResponse, userStatus)

Expand Down
Loading
Loading