Skip to content

Commit

Permalink
exp/services/recoverysigner: add basic logs (#2567)
Browse files Browse the repository at this point in the history
### What

Add basic logs for each action the service can perform that describe what's happening to help with identifying issues. Also fixes logs so that all existing logs carry the request ID and account ID for the current request.

### Why

This is a young service and we're likely to run into issues that we will need logs to help us debug.
  • Loading branch information
leighmcculloch authored May 8, 2020
1 parent ebd7214 commit 40f71ac
Show file tree
Hide file tree
Showing 8 changed files with 150 additions and 46 deletions.
19 changes: 17 additions & 2 deletions exp/services/recoverysigner/internal/serve/account_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,18 @@ func (h accountDeleteHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
return
}

l := h.Logger.Ctx(ctx).
WithField("account", req.Address.Address())

l.Info("Request to delete account.")

acc, err := h.AccountStore.Get(req.Address.Address())
if err == account.ErrNotFound {
l.Info("Account not found.")
notFound.Render(w)
return
} else if err != nil {
h.Logger.Error(err)
l.Error(err)
serverError.Render(w)
return
}
Expand All @@ -54,6 +60,7 @@ func (h accountDeleteHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)

// Authorized if authenticated as the account.
authorized := claims.Address == req.Address.Address()
l.Infof("Authorized with self: %v.", authorized)

// Authorized if authenticated as an identity registered with the account.
for _, i := range acc.Identities {
Expand All @@ -66,27 +73,35 @@ func (h accountDeleteHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
(m.Type == account.AuthMethodTypeEmail && m.Value == claims.Email)) {
respIdentity.Authenticated = true
authorized = true
l.Infof("Authorized with %s.", m.Type)
break
}
}

resp.Identities = append(resp.Identities, respIdentity)
}

l.Infof("Authorized: %v.", authorized)
if !authorized {
notFound.Render(w)
return
}

l.Info("Deleting account.")

err = h.AccountStore.Delete(req.Address.Address())
if err == account.ErrNotFound {
// It can happen if two authorized users are trying to delete the account at the same time.
l.Info("Account not found.")
notFound.Render(w)
return
} else if err != nil {
h.Logger.Error(err)
l.Error("Error deleting account:", err)
serverError.Render(w)
return
}

l.Info("Deleted account.")

httpjson.Render(w, resp, httpjson.JSON)
}
12 changes: 11 additions & 1 deletion exp/services/recoverysigner/internal/serve/account_get.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,18 @@ func (h accountGetHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}

l := h.Logger.Ctx(ctx).
WithField("account", req.Address.Address())

l.Info("Request to get account.")

acc, err := h.AccountStore.Get(req.Address.Address())
if err == account.ErrNotFound {
l.Info("Account not found.")
notFound.Render(w)
return
} else if err != nil {
h.Logger.Error(err)
l.Error(err)
serverError.Render(w)
return
}
Expand All @@ -54,6 +60,7 @@ func (h accountGetHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {

// Authorized if authenticated as the account.
authorized := claims.Address == req.Address.Address()
l.Infof("Authorized with self: %v.", authorized)

// Authorized if authenticated as an identity registered with the account.
for _, i := range acc.Identities {
Expand All @@ -66,12 +73,15 @@ func (h accountGetHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
(m.Type == account.AuthMethodTypeEmail && m.Value == claims.Email)) {
respIdentity.Authenticated = true
authorized = true
l.Infof("Authorized with %s.", m.Type)
break
}
}

resp.Identities = append(resp.Identities, respIdentity)
}

l.Infof("Authorized: %v.", authorized)
if !authorized {
notFound.Render(w)
return
Expand Down
20 changes: 18 additions & 2 deletions exp/services/recoverysigner/internal/serve/account_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ func (h accountListHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}

l := h.Logger.Ctx(ctx)

l.Info("Request to get accounts.")

resp := accountListResponse{
Accounts: []accountResponse{},
}
Expand All @@ -40,7 +44,7 @@ func (h accountListHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
if err == account.ErrNotFound {
// Do nothing.
} else if err != nil {
h.Logger.Error(err)
l.Error(err)
serverError.Render(w)
return
} else {
Expand All @@ -55,12 +59,15 @@ func (h accountListHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
accResp.Identities = append(accResp.Identities, accRespIdentity)
}
resp.Accounts = append(resp.Accounts, accResp)
l.WithField("account", acc.Address).
WithField("auth_method_type", account.AuthMethodTypeAddress).
Info("Found account with auth method type as self.")
}

// Find accounts that have the address listed as an owner or other identity.
accs, err := h.AccountStore.FindWithIdentityAddress(claims.Address)
if err != nil {
h.Logger.Error(err)
l.Error(err)
serverError.Render(w)
return
}
Expand All @@ -82,6 +89,9 @@ func (h accountListHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
accResp.Identities = append(accResp.Identities, accRespIdentity)
}
resp.Accounts = append(resp.Accounts, accResp)
l.WithField("account", acc.Address).
WithField("auth_method_type", account.AuthMethodTypeAddress).
Info("Found account with auth method type as identity.")
}
}

Expand Down Expand Up @@ -111,6 +121,9 @@ func (h accountListHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
accResp.Identities = append(accResp.Identities, accRespIdentity)
}
resp.Accounts = append(resp.Accounts, accResp)
l.WithField("account", acc.Address).
WithField("auth_method_type", account.AuthMethodTypePhoneNumber).
Info("Found account with auth method type as identity.")
}
}

Expand Down Expand Up @@ -140,6 +153,9 @@ func (h accountListHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
accResp.Identities = append(accResp.Identities, accRespIdentity)
}
resp.Accounts = append(resp.Accounts, accResp)
l.WithField("account", acc.Address).
WithField("auth_method_type", account.AuthMethodTypeEmail).
Info("Found account with auth method type as identity.")
}
}

Expand Down
18 changes: 17 additions & 1 deletion exp/services/recoverysigner/internal/serve/account_post.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,16 +86,25 @@ func (h accountPostHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}

l := h.Logger.Ctx(ctx).
WithField("account", req.Address.Address())

l.Info("Request to register account.")

if req.Address.Address() != claims.Address {
l.WithField("address", claims.Address).
Info("Not authorized as self, authorized as other address.")
unauthorized.Render(w)
return
}

if req.Validate() != nil {
l.Info("Request validation failed.")
badRequest.Render(w)
return
}

authMethodCount := 0
acc := account.Account{
Address: req.Address.Address(),
Identities: []account.Identity{},
Expand All @@ -109,20 +118,27 @@ func (h accountPostHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
Type: account.AuthMethodType(m.Type),
Value: m.Value,
})
authMethodCount++
}
acc.Identities = append(acc.Identities, accIdentity)
}
l = l.
WithField("identities_count", len(acc.Identities)).
WithField("auth_methods_count", authMethodCount)

err = h.AccountStore.Add(acc)
if err == account.ErrAlreadyExists {
l.Info("Account already registered.")
conflict.Render(w)
return
} else if err != nil {
h.Logger.Error(err)
l.Error(err)
serverError.Render(w)
return
}

l.Info("Account registered.")

resp := accountResponse{
Address: acc.Address,
Signer: h.SigningAddress.Address(),
Expand Down
26 changes: 24 additions & 2 deletions exp/services/recoverysigner/internal/serve/account_put.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,23 +81,36 @@ func (h accountPutHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {

req := accountPutRequest{}
err := httpdecode.Decode(r, &req)
if err != nil || req.Address == nil || req.Validate() != nil {
if err != nil || req.Address == nil {
badRequest.Render(w)
return
}

l := h.Logger.Ctx(ctx).
WithField("account", req.Address.Address())

l.Info("Request to update account.")

if req.Validate() != nil {
l.Info("Request validation failed.")
badRequest.Render(w)
return
}

acc, err := h.AccountStore.Get(req.Address.Address())
if err == account.ErrNotFound {
l.Info("Account not found.")
notFound.Render(w)
return
} else if err != nil {
h.Logger.Error(err)
l.Error(err)
serverError.Render(w)
return
}

// Authorized if authenticated as the account.
authorized := claims.Address == req.Address.Address()
l.Infof("Authorized with self: %v.", authorized)

// Authorized if authenticated as an identity registered with the account.
for _, i := range acc.Identities {
Expand All @@ -106,6 +119,7 @@ func (h accountPutHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
(m.Type == account.AuthMethodTypePhoneNumber && m.Value == claims.PhoneNumber) ||
(m.Type == account.AuthMethodTypeEmail && m.Value == claims.Email)) {
authorized = true
l.Infof("Authorized with %s.", m.Type)
break
}
}
Expand All @@ -115,6 +129,7 @@ func (h accountPutHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}

authMethodCount := 0
accWithNewIdentiies := account.Account{
Address: req.Address.Address(),
Identities: []account.Identity{},
Expand All @@ -128,13 +143,18 @@ func (h accountPutHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
Type: account.AuthMethodType(m.Type),
Value: m.Value,
})
authMethodCount++
}
accWithNewIdentiies.Identities = append(accWithNewIdentiies.Identities, accIdentity)
}
l = l.
WithField("identities_count", len(accWithNewIdentiies.Identities)).
WithField("auth_methods_count", authMethodCount)

err = h.AccountStore.Update(accWithNewIdentiies)
if err == account.ErrNotFound {
// It can happen if another authorized user is trying to delete the account at the same time.
l.Info("Account not found.")
notFound.Render(w)
return
} else if err != nil {
Expand All @@ -143,6 +163,8 @@ func (h accountPutHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}

l.Info("Account updated.")

resp := accountResponse{
Address: accWithNewIdentiies.Address,
Signer: h.SigningAddress.Address(),
Expand Down
Loading

0 comments on commit 40f71ac

Please sign in to comment.