Skip to content
Open
Show file tree
Hide file tree
Changes from 17 commits
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
1 change: 1 addition & 0 deletions api/groups/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +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: "/user-status/:address", Open: true},
{Name: "/debug", Open: true},
{Name: "/verify-code", Open: true},
{Name: "/registered-users", Open: true},
Expand Down
47 changes: 47 additions & 0 deletions api/groups/guardianGroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const (
signMultipleTransactionsPath = "/sign-multiple-transactions"
setSecurityModeNoExpirePath = "/set-security-mode"
unsetSecurityModeNoExpirePath = "/unset-security-mode"
getUserStatusPath = "/user-status/:address"
registerPath = "/register"
verifyCodePath = "/verify-code"
registeredUsersPath = "/registered-users"
Expand Down Expand Up @@ -103,6 +104,11 @@ func NewGuardianGroup(facade shared.FacadeHandler) (*guardianGroup, error) {
Method: http.MethodGet,
Handler: gg.config,
},
{
Path: getUserStatusPath,
Method: http.MethodGet,
Handler: gg.getUserStatus,
},
}
gg.endpoints = endpoints

Expand Down Expand Up @@ -237,6 +243,47 @@ func (gg *guardianGroup) unsetSecurityModeNoExpire(c *gin.Context) {
returnStatus(c, nil, http.StatusOK, "", chainApiShared.ReturnCodeSuccess)
}

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, userAddr, debugErr)
}()

status, err := gg.facade.GetUserStatus(userAddr)
if err != nil {
debugErr = fmt.Errorf("%w while interrogating security status", err)
handleErrorAndReturn(c, status, err.Error())
return

}

returnStatus(c, status, http.StatusOK, "", chainApiShared.ReturnCodeSuccess)
}

func logUserStatusRequest(userIp string, userAgent string, userAddr string, debugErr error) {
logArgs := []interface{}{
"route", getUserStatusPath,
"ip", userIp,
"user agent", userAgent,
"userAddr", userAddr,
}
defer func() {
guardianLog.Info("Request info", logArgs...)
}()

if debugErr == nil {
logArgs = append(logArgs, "result", "success")
return
}

logArgs = append(logArgs, "error", debugErr.Error())
}

// signTransaction returns the transaction signed by the guardian if the verification passed
func (gg *guardianGroup) signTransaction(c *gin.Context) {
var request requests.SignTransaction
Expand Down
64 changes: 64 additions & 0 deletions api/groups/guardianGroup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,70 @@ func TestGuardianGroup_UnsetSecurityModeNoExpire(t *testing.T) {
})
}

func TestGuardianGroup_getUserStatus(t *testing.T) {
t.Parallel()

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

facade := mockFacade.GuardianFacadeStub{
GetUserStatusCalled: func(userAddress string) (*requests.UserStatusResponse, error) {
return &requests.UserStatusResponse{SecurityStatus: -1}, expectedError
},
}

gg, _ := groups.NewGuardianGroup(&facade)

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

req, _ := http.NewRequest("GET", "/guardian/user-status/"+providedAddr, nil)
resp := httptest.NewRecorder()
ws.ServeHTTP(resp, req)

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

expectedGenResponse := createExpectedGeneralResponse(&requests.UserStatusResponse{
SecurityStatus: -1,
}, "")

assert.Equal(t, expectedGenResponse.Data, statusRsp.Data)
assert.True(t, strings.Contains(statusRsp.Error, expectedError.Error()))
require.Equal(t, http.StatusInternalServerError, resp.Code)
})

t.Run("should work", func(t *testing.T) {
t.Parallel()

facade := mockFacade.GuardianFacadeStub{
GetUserStatusCalled: func(userAddress string) (*requests.UserStatusResponse, error) {
return &requests.UserStatusResponse{
SecurityStatus: 1,
}, nil
},
}

gg, _ := groups.NewGuardianGroup(&facade)

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

req, _ := http.NewRequest("GET", "/guardian/user-status/"+providedAddr, nil)
resp := httptest.NewRecorder()
ws.ServeHTTP(resp, req)

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

expectedGenResponse := createExpectedGeneralResponse(&requests.UserStatusResponse{
SecurityStatus: 1,
}, "")

assert.Equal(t, expectedGenResponse.Data, statusRsp.Data)
assert.Equal(t, "", statusRsp.Error)
require.Equal(t, http.StatusOK, resp.Code)
})
}

func TestGuardianGroup_signMultipleTransaction(t *testing.T) {
t.Parallel()

Expand Down
26 changes: 24 additions & 2 deletions api/middleware/nativeAuthWhitelistHandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package middleware

import (
"fmt"
"strings"

"github.com/multiversx/mx-multi-factor-auth-go-service/config"
)
Expand All @@ -20,7 +21,8 @@ func NewNativeAuthWhitelistHandler(apiPackages map[string]config.APIPackageConfi
for _, route := range groupCfg.Routes {
if !route.Auth {
fullPath := fmt.Sprintf("%s%s", groupPath, route.Name)
whitelistedRoutes[fullPath] = struct{}{}
basePath := trimPathPlaceholder(fullPath)
whitelistedRoutes[basePath] = struct{}{}
}
}
}
Expand All @@ -31,9 +33,29 @@ func NewNativeAuthWhitelistHandler(apiPackages map[string]config.APIPackageConfi
}
}

func trimPathPlaceholder(path string) string {
parts := strings.Split(path, ":")
if len(parts) > 0 {
return "/" + strings.Trim(parts[0], "/")
}

return path
}

func extractBaseRoutePath(path string) string {
parts := strings.Split(path, "/")

if len(parts) > 2 {
return "/" + parts[1] + "/" + parts[2] // group and base path
}

return path
}

// IsWhitelisted returns true if the provided route is whitelisted for native authentication
func (handler *nativeAuthWhitelistHandler) IsWhitelisted(route string) bool {
_, found := handler.whitelistedRoutesMap[route]
baseRoute := extractBaseRoutePath(route)
_, found := handler.whitelistedRoutesMap[baseRoute]
return found
}

Expand Down
9 changes: 9 additions & 0 deletions api/middleware/nativeAuthWhitelistHandler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ func TestNativeAuthWhitelistHandler(t *testing.T) {
Open: true,
Auth: false,
},
{
Name: "/user-status/:address",
Open: true,
Auth: false,
},
},
},
"status": {
Expand All @@ -43,9 +48,13 @@ func TestNativeAuthWhitelistHandler(t *testing.T) {
require.True(t, handler.IsWhitelisted("/guardian"))
require.True(t, handler.IsWhitelisted("/status"))
require.True(t, handler.IsWhitelisted("/log"))
require.True(t, handler.IsWhitelisted("/guardian/user-status/erd1"))
require.True(t, handler.IsWhitelisted("/guardian/user-status"))
require.True(t, handler.IsWhitelisted("/guardian/user-status/"))
require.False(t, handler.IsWhitelisted("/guardian/register"))
require.False(t, handler.IsWhitelisted("guardian/sign-transaction"))
require.False(t, handler.IsWhitelisted("/sign-transaction"))
require.False(t, handler.IsWhitelisted("guardian/user-status/erd1"))
require.False(t, handler.IsWhitelisted("guardian"))
require.False(t, handler.IsWhitelisted(""))
}
Expand Down
1 change: 1 addition & 0 deletions api/shared/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +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)
GetUserStatus(userAddress string) (*requests.UserStatusResponse, error)
RegisteredUsers() (uint32, error)
TcsConfig() *tcsCore.TcsConfig
GetMetrics() map[string]*requests.EndpointMetricsResponse
Expand Down
1 change: 1 addition & 0 deletions cmd/multi-factor-auth/config/api.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +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 = "/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
20 changes: 20 additions & 0 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 @@ -289,3 +289,23 @@ type _ struct {
// required:true
Payload requests.SecurityModeNoExpire
}

// swagger:route GET /user-status Guardian getUsersStatus
// Returns the status of the user.
// This request does not need the Authorization header
//
// responses:
// 200: userStatusResponse

// The status of the operation
// swagger:response userStatusResponse
type _ struct {
// in:body
Body struct {
// UserStatusResponse
// HTTP status code
Status string `json:"status"`
// Internal error
Error string `json:"error"`
}
}
61 changes: 61 additions & 0 deletions cmd/multi-factor-auth/swagger/ui/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,41 @@
}
}
},




















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

remove these empty lines?

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

"/user-status": {
"get": {
"description": "This request does not need the Authorization header",
"tags": [
"Guardian"
],
"summary": "Returns the status of the user.",
"operationId": "getUsersStatus",
"responses": {
"200": {
"$ref": "#/responses/userStatusResponse"
}
}
}
},
"/verify-code": {
"post": {
"security": [
Expand Down Expand Up @@ -321,6 +356,14 @@
},
"x-go-name": "ReceiverUsername"
},
"relayer": {
"type": "string",
"x-go-name": "RelayerAddr"
},
"relayerSignature": {
"type": "string",
"x-go-name": "RelayerSignature"
},
"sender": {
"type": "string",
"x-go-name": "Sender"
Expand Down Expand Up @@ -782,6 +825,24 @@
}
}
},
"userStatusResponse": {
"description": "The status of the operation",
Copy link
Contributor

Choose a reason for hiding this comment

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

status of which operation?

this should be the address/user status on the TCS

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

"schema": {
"type": "object",
"properties": {
"error": {
"description": "Internal error",
"type": "string",
"x-go-name": "Error"
},
"status": {
"description": "UserStatusResponse\nHTTP status code",
"type": "string",
"x-go-name": "Status"
}
}
}
},
"verifyCodeResponse": {
"description": "Verification result",
"schema": {
Expand Down
12 changes: 12 additions & 0 deletions core/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,15 @@ 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
// ManuallySet means that security mode was activated because of failures
ManuallySet
// AutomaticallySet means that security mode was activated by user with SetSecurityModeNoExpire
AutomaticallySet
)
1 change: 1 addition & 0 deletions core/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +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)
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
5 changes: 5 additions & 0 deletions core/requests/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ type SecurityModeNoExpire struct {
UserAddr string `json:"user"`
}

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

}

// SignMessageResponse is the service response to the sign message request
type SignMessageResponse struct {
Message string `json:"message"`
Expand Down
5 changes: 5 additions & 0 deletions facade/guardianFacade.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ func (gf *guardianFacade) UnsetSecurityModeNoExpire(userIp string, request reque
return gf.serviceResolver.UnsetSecurityModeNoExpire(userIp, 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
func (gf *guardianFacade) SignMultipleTransactions(userIp string, request requests.SignMultipleTransactions) ([][]byte, *requests.OTPCodeVerifyData, error) {
return gf.serviceResolver.SignMultipleTransactions(userIp, request)
Expand Down
Loading