From ec1bcfe0b8417d159fd19d8b876f80b03470732b Mon Sep 17 00:00:00 2001
From: ale <ale@incal.net>
Date: Mon, 29 Jan 2018 09:54:57 +0000
Subject: [PATCH] Do not return an error when a user has no encryption keys on
Open
And make HTTP result codes slightly more meaningful.
---
README.md | 7 +++++--
server/keystore.go | 22 ++++++++++++++--------
server/server.go | 12 ++++++++++--
3 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/README.md b/README.md
index 27201a0e..cc2e3907 100644
--- a/README.md
+++ b/README.md
@@ -27,14 +27,17 @@ similarly JSON-encoded.
Retrieve the encrypted key for a user, decrypt it with the provided
password, and store it in memory.
-OpenRequest is an object with the
-following attributes:
+OpenRequest is an object with the following attributes:
* `username`
* `password` to decrypt the user's key with
* `ttl` (seconds) time after which the credentials are automatically
forgotten
+If the user has no encrypted keys in the database, the request will
+still return successfully: no action will be performed, and no errors
+will be returned.
+
`/api/get` (*GetRequest*) -> *GetResponse*
Retrieve the key for a user. GetRequest must contain the following
diff --git a/server/keystore.go b/server/keystore.go
index af4f9fe7..19a1d1a3 100644
--- a/server/keystore.go
+++ b/server/keystore.go
@@ -4,6 +4,7 @@ import (
"context"
"errors"
"io/ioutil"
+ "log"
"strings"
"sync"
"time"
@@ -15,9 +16,10 @@ import (
)
var (
- ErrNoKeys = errors.New("no keys available")
- ErrBadUser = errors.New("username does not match authentication token")
- ErrInvalidTTL = errors.New("invalid ttl")
+ errNoKeys = errors.New("no keys available")
+ errBadUser = errors.New("username does not match authentication token")
+ errUnauthorized = errors.New("unauthorized")
+ errInvalidTTL = errors.New("invalid ttl")
)
// Database represents the interface to the underlying backend for
@@ -130,7 +132,7 @@ func (s *KeyStore) expire() {
// A Context is needed because this method might issue an RPC.
func (s *KeyStore) Open(ctx context.Context, username, password string, ttlSeconds int) error {
if ttlSeconds == 0 {
- return ErrInvalidTTL
+ return errInvalidTTL
}
encKeys, err := s.db.GetPrivateKeys(ctx, username)
@@ -138,7 +140,8 @@ func (s *KeyStore) Open(ctx context.Context, username, password string, ttlSecon
return err
}
if len(encKeys) == 0 {
- return ErrNoKeys
+ // No keys found. Not an error.
+ return nil
}
// Naive and inefficient way of decrypting multiple keys: it
@@ -163,17 +166,20 @@ func (s *KeyStore) Get(username, ssoTicket string) ([]byte, error) {
// Validate the SSO ticket.
tkt, err := s.validator.Validate(ssoTicket, "", s.service, nil)
if err != nil {
- return nil, err
+ // Log authentication failures for debugging purposes.
+ log.Printf("Validate(%s) error: %v", username, err)
+ return nil, errUnauthorized
}
if tkt.User != username {
- return nil, ErrBadUser
+ log.Printf("Validate(%s) user mismatch: sso=%s", username, tkt.User)
+ return nil, errBadUser
}
s.mx.Lock()
defer s.mx.Unlock()
u, ok := s.userKeys[username]
if !ok {
- return nil, ErrNoKeys
+ return nil, errNoKeys
}
return u.pkey, nil
}
diff --git a/server/server.go b/server/server.go
index fc97436f..a7de20a9 100644
--- a/server/server.go
+++ b/server/server.go
@@ -24,7 +24,7 @@ func (s *keyStoreServer) handleOpen(w http.ResponseWriter, r *http.Request) {
err := s.KeyStore.Open(r.Context(), req.Username, req.Password, req.TTL)
if err != nil {
log.Printf("Open(%s) error: %v", req.Username, err)
- http.Error(w, err.Error(), http.StatusUnauthorized)
+ http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
@@ -40,7 +40,15 @@ func (s *keyStoreServer) handleGet(w http.ResponseWriter, r *http.Request) {
key, err := s.KeyStore.Get(req.Username, req.SSOTicket)
if err != nil {
log.Printf("Get(%s) error: %v", req.Username, err)
- http.Error(w, err.Error(), http.StatusUnauthorized)
+ // Return an appropriate error code.
+ switch err {
+ case errUnauthorized, errBadUser:
+ http.Error(w, err.Error(), http.StatusUnauthorized)
+ case errNoKeys:
+ http.NotFound(w, r)
+ default:
+ http.Error(w, err.Error(), http.StatusInternalServerError)
+ }
return
}
--
GitLab