Fix issuing recovery key (#1007)

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

authn/api/grpc/endpoint_test.go

@@ -46,39 +46,67 @@ func startGRPCServer(svc authn.Service, port int) {
 }
 
 func TestIssue(t *testing.T) {
-	loginKey, err := svc.Issue(context.Background(), email, authn.Key{Type: authn.UserKey, IssuedAt: time.Now()})
+	userKey, err := svc.Issue(context.Background(), email, authn.Key{Type: authn.UserKey, IssuedAt: time.Now()})
-	assert.Nil(t, err, fmt.Sprintf("Issuing login key expected to succeed: %s", err))
+	assert.Nil(t, err, fmt.Sprintf("Issuing user key expected to succeed: %s", err))
 
 	authAddr := fmt.Sprintf("localhost:%d", port)
 	conn, _ := grpc.Dial(authAddr, grpc.WithInsecure())
 	client := grpcapi.NewClient(mocktracer.New(), conn, time.Second)
 
-	cases := map[string]struct {
+	cases := []struct {
-		token string
+		desc string
-		id    string
+		id   string
-		kind  uint32
+		kind uint32
-		err   error
+		err  error
 	}{
-		"issue for user with valid token":   {"", email, authn.UserKey, nil},
+		{
-		"issue for user that doesn't exist": {"", loginKey.Secret, 32, status.Error(codes.InvalidArgument, "received invalid token request")},
+			desc: "issue for user with valid token",
+			id:   email,
+			kind: authn.UserKey,
+			err:  nil,
+		},
+		{
+			desc: "issue recovery key",
+			id:   email,
+			kind: authn.RecoveryKey,
+			err:  nil,
+		},
+		{
+			desc: "issue API key",
+			id:   userKey.Secret,
+			kind: authn.APIKey,
+			err:  nil,
+		},
+		{
+			desc: "issue for invalid key type",
+			id:   email,
+			kind: 32,
+			err:  status.Error(codes.InvalidArgument, "received invalid token request"),
+		},
+		{
+			desc: "issue for user that  exist",
+			id:   "",
+			kind: authn.APIKey,
+			err:  status.Error(codes.Unauthenticated, "unauthorized access"),
+		},
 	}
 
-	for desc, tc := range cases {
+	for _, tc := range cases {
 		_, err := client.Issue(context.Background(), &mainflux.IssueReq{Issuer: tc.id, Type: tc.kind})
-		assert.Equal(t, tc.err, err, fmt.Sprintf("%s: expected %s got %s", desc, tc.err, err))
+		assert.Equal(t, tc.err, err, fmt.Sprintf("%s: expected %s got %s", tc.desc, tc.err, err))
 	}
 }
 
 func TestIdentify(t *testing.T) {
-	loginKey, err := svc.Issue(context.Background(), email, authn.Key{Type: authn.UserKey, IssuedAt: time.Now()})
+	userKey, err := svc.Issue(context.Background(), email, authn.Key{Type: authn.UserKey, IssuedAt: time.Now()})
-	assert.Nil(t, err, fmt.Sprintf("Issuing login key expected to succeed: %s", err))
-
-	resetKey, err := svc.Issue(context.Background(), loginKey.Secret, authn.Key{Type: authn.RecoveryKey, IssuedAt: time.Now()})
-	assert.Nil(t, err, fmt.Sprintf("Issuing reset key expected to succeed: %s", err))
-
-	userKey, err := svc.Issue(context.Background(), loginKey.Secret, authn.Key{Type: authn.APIKey, IssuedAt: time.Now(), ExpiresAt: time.Now().Add(time.Minute)})
 	assert.Nil(t, err, fmt.Sprintf("Issuing user key expected to succeed: %s", err))
 
+	recoveryKey, err := svc.Issue(context.Background(), email, authn.Key{Type: authn.RecoveryKey, IssuedAt: time.Now()})
+	assert.Nil(t, err, fmt.Sprintf("Issuing recovery key expected to succeed: %s", err))
+
+	apiKey, err := svc.Issue(context.Background(), userKey.Secret, authn.Key{Type: authn.APIKey, IssuedAt: time.Now(), ExpiresAt: time.Now().Add(time.Minute)})
+	assert.Nil(t, err, fmt.Sprintf("Issuing API key expected to succeed: %s", err))
+
 	authAddr := fmt.Sprintf("localhost:%d", port)
 	conn, _ := grpc.Dial(authAddr, grpc.WithInsecure())
 	client := grpcapi.NewClient(mocktracer.New(), conn, time.Second)
@@ -90,19 +118,19 @@ func TestIdentify(t *testing.T) {
 		err   error
 	}{
 		{
-			desc:  "identify user with reset token",
+			desc:  "identify user with recovery token",
-			token: resetKey.Secret,
+			token: recoveryKey.Secret,
 			id:    email,
 			err:   nil,
 		},
 		{
-			desc:  "identify user with user token",
+			desc:  "identify user with API token",
-			token: userKey.Secret,
+			token: apiKey.Secret,
 			id:    email,
 			err:   nil,
 		},
 		{
-			desc:  "identify user with invalid login token",
+			desc:  "identify user with invalid user token",
 			token: "invalid",
 			id:    "",
 			err:   status.Error(codes.Unauthenticated, "unauthorized access"),

authn/api/http/endpoint_test.go

@@ -80,16 +80,16 @@ func toJSON(data interface{}) string {
 
 func TestIssue(t *testing.T) {
 	svc := newService()
-	loginKey, err := svc.Issue(context.Background(), email, authn.Key{Type: authn.UserKey, IssuedAt: time.Now()})
+	userKey, err := svc.Issue(context.Background(), email, authn.Key{Type: authn.UserKey, IssuedAt: time.Now()})
-	assert.Nil(t, err, fmt.Sprintf("Issuing login key expected to succeed: %s", err))
+	assert.Nil(t, err, fmt.Sprintf("Issuing user key expected to succeed: %s", err))
 
 	ts := newServer(svc)
 	defer ts.Close()
 	client := ts.Client()
 
-	lk := issueRequest{Type: authn.UserKey}
+	uk := issueRequest{Type: authn.UserKey}
+	ak := issueRequest{Type: authn.APIKey, Duration: time.Hour}
 	rk := issueRequest{Type: authn.RecoveryKey}
-	uk := issueRequest{Type: authn.APIKey, Duration: time.Hour}
 
 	cases := []struct {
 		desc   string
@@ -99,48 +99,48 @@ func TestIssue(t *testing.T) {
 		status int
 	}{
 		{
-			desc:   "issue login key",
+			desc:   "issue user key",
-			req:    toJSON(lk),
+			req:    toJSON(uk),
 			ct:     contentType,
 			token:  "",
 			status: http.StatusCreated,
 		},
 		{
-			desc:   "issue user key",
+			desc:   "issue API key",
-			req:    toJSON(uk),
+			req:    toJSON(ak),
 			ct:     contentType,
-			token:  loginKey.Secret,
+			token:  userKey.Secret,
 			status: http.StatusCreated,
 		},
 		{
-			desc:   "issue reset key",
+			desc:   "issue recovery key",
 			req:    toJSON(rk),
 			ct:     contentType,
-			token:  loginKey.Secret,
+			token:  userKey.Secret,
 			status: http.StatusBadRequest,
 		},
 		{
-			desc: "issue login key wrong content type",
+			desc: "issue user key wrong content type",
-			req:  toJSON(lk),
+			req:  toJSON(uk),
-			ct:   "", token: loginKey.Secret,
+			ct:   "", token: userKey.Secret,
 			status: http.StatusUnsupportedMediaType,
 		},
 		{
 			desc:   "issue key wrong content type",
 			req:    toJSON(rk),
 			ct:     "",
-			token:  loginKey.Secret,
+			token:  userKey.Secret,
 			status: http.StatusUnsupportedMediaType,
 		},
 		{
 			desc:   "issue key unauthorized",
-			req:    toJSON(uk),
+			req:    toJSON(ak),
 			ct:     contentType,
 			token:  "wrong",
 			status: http.StatusForbidden,
 		},
 		{
-			desc:   "issue reset key with empty token",
+			desc:   "issue recovery key with empty token",
 			req:    toJSON(rk),
 			ct:     contentType,
 			token:  "",
@@ -238,12 +238,12 @@ func TestRetrieve(t *testing.T) {
 
 func TestRevoke(t *testing.T) {
 	svc := newService()
-	loginKey, err := svc.Issue(context.Background(), email, authn.Key{Type: authn.UserKey, IssuedAt: time.Now()})
+	userKey, err := svc.Issue(context.Background(), email, authn.Key{Type: authn.UserKey, IssuedAt: time.Now()})
-	assert.Nil(t, err, fmt.Sprintf("Issuing login key expected to succeed: %s", err))
+	assert.Nil(t, err, fmt.Sprintf("Issuing user key expected to succeed: %s", err))
 	key := authn.Key{Type: authn.APIKey, IssuedAt: time.Now()}
 
-	k, err := svc.Issue(context.Background(), loginKey.Secret, key)
+	k, err := svc.Issue(context.Background(), userKey.Secret, key)
-	assert.Nil(t, err, fmt.Sprintf("Issuing login key expected to succeed: %s", err))
+	assert.Nil(t, err, fmt.Sprintf("Issuing user key expected to succeed: %s", err))
 
 	ts := newServer(svc)
 	defer ts.Close()
@@ -258,13 +258,13 @@ func TestRevoke(t *testing.T) {
 		{
 			desc:   "revoke an existing key",
 			id:     k.ID,
-			token:  loginKey.Secret,
+			token:  userKey.Secret,
 			status: http.StatusNoContent,
 		},
 		{
 			desc:   "revoke a non-existing key",
 			id:     "non-existing",
-			token:  loginKey.Secret,
+			token:  userKey.Secret,
 			status: http.StatusNoContent,
 		},
 		{

authn/service.go

@@ -10,9 +10,9 @@ import (
 )
 
 const (
-	loginDuration = 10 * time.Hour
+	loginDuration    = 10 * time.Hour
-	resetDuration = 5 * time.Minute
+	recoveryDuration = 5 * time.Minute
-	issuerName    = "mainflux.authn"
+	issuerName       = "mainflux.authn"
 )
 
 var (
@@ -75,9 +75,9 @@ func (svc service) Issue(ctx context.Context, issuer string, key Key) (Key, erro
 	case APIKey:
 		return svc.userKey(ctx, issuer, key)
 	case RecoveryKey:
-		return svc.resetKey(ctx, issuer, key)
+		return svc.tmpKey(issuer, recoveryDuration, key)
 	default:
-		return svc.loginKey(issuer, key)
+		return svc.tmpKey(issuer, loginDuration, key)
 	}
 }
 
@@ -127,22 +127,8 @@ func (svc service) Identify(ctx context.Context, token string) (string, error) {
 	}
 }
 
-func (svc service) loginKey(issuer string, key Key) (Key, error) {
+func (svc service) tmpKey(issuer string, duration time.Duration, key Key) (Key, error) {
 	key.Secret = issuer
-	return svc.tempKey(loginDuration, key)
-}
-
-func (svc service) resetKey(ctx context.Context, issuer string, key Key) (Key, error) {
-	issuer, err := svc.login(issuer)
-	if err != nil {
-		return Key{}, err
-	}
-	key.Secret = issuer
-
-	return svc.tempKey(resetDuration, key)
-}
-
-func (svc service) tempKey(duration time.Duration, key Key) (Key, error) {
 	key.Issuer = issuerName
 	key.ExpiresAt = key.IssuedAt.Add(duration)
 	val, err := svc.tokenizer.Issue(key)

authn/service_test.go

@@ -29,7 +29,7 @@ func newService() authn.Service {
 
 func TestIssue(t *testing.T) {
 	svc := newService()
-	loginKey, err := svc.Issue(context.Background(), email, authn.Key{Type: authn.UserKey, IssuedAt: time.Now()})
+	userKey, err := svc.Issue(context.Background(), email, authn.Key{Type: authn.UserKey, IssuedAt: time.Now()})
 	assert.Nil(t, err, fmt.Sprintf("Issuing login key expected to succeed: %s", err))
 
 	cases := []struct {
@@ -39,7 +39,7 @@ func TestIssue(t *testing.T) {
 		err    error
 	}{
 		{
-			desc: "issue login key",
+			desc: "issue user key",
 			key: authn.Key{
 				Type:     authn.UserKey,
 				IssuedAt: time.Now(),
@@ -48,7 +48,7 @@ func TestIssue(t *testing.T) {
 			err:    nil,
 		},
 		{
-			desc: "issue login key no issue time",
+			desc: "issue user key no issue time",
 			key: authn.Key{
 				Type: authn.UserKey,
 			},
@@ -56,16 +56,16 @@ func TestIssue(t *testing.T) {
 			err:    authn.ErrInvalidKeyIssuedAt,
 		},
 		{
-			desc: "issue user key",
+			desc: "issue API key",
 			key: authn.Key{
 				Type:     authn.APIKey,
 				IssuedAt: time.Now(),
 			},
-			issuer: loginKey.Secret,
+			issuer: userKey.Secret,
 			err:    nil,
 		},
 		{
-			desc: "issue user key unauthorized",
+			desc: "issue API key unauthorized",
 			key: authn.Key{
 				Type:     authn.APIKey,
 				IssuedAt: time.Now(),
@@ -74,28 +74,28 @@ func TestIssue(t *testing.T) {
 			err:    authn.ErrUnauthorizedAccess,
 		},
 		{
-			desc: "issue user key no issue time",
+			desc: "issue API key no issue time",
 			key: authn.Key{
 				Type: authn.APIKey,
 			},
-			issuer: loginKey.Secret,
+			issuer: userKey.Secret,
 			err:    authn.ErrInvalidKeyIssuedAt,
 		},
 		{
-			desc: "issue reset key",
+			desc: "issue recovery key",
 			key: authn.Key{
 				Type:     authn.RecoveryKey,
 				IssuedAt: time.Now(),
 			},
-			issuer: loginKey.Secret,
+			issuer: userKey.Secret,
 			err:    nil,
 		},
 		{
-			desc: "issue reset key no issue time",
+			desc: "issue recovery key no issue time",
 			key: authn.Key{
 				Type: authn.RecoveryKey,
 			},
-			issuer: loginKey.Secret,
+			issuer: userKey.Secret,
 			err:    authn.ErrInvalidKeyIssuedAt,
 		},
 	}
@@ -213,7 +213,7 @@ func TestIdentify(t *testing.T) {
 	loginKey, err := svc.Issue(context.Background(), email, authn.Key{Type: authn.UserKey, IssuedAt: time.Now()})
 	assert.Nil(t, err, fmt.Sprintf("Issuing login key expected to succeed: %s", err))
 
-	resetKey, err := svc.Issue(context.Background(), loginKey.Secret, authn.Key{Type: authn.RecoveryKey, IssuedAt: time.Now()})
+	recoveryKey, err := svc.Issue(context.Background(), email, authn.Key{Type: authn.RecoveryKey, IssuedAt: time.Now()})
 	assert.Nil(t, err, fmt.Sprintf("Issuing reset key expected to succeed: %s", err))
 
 	userKey, err := svc.Issue(context.Background(), loginKey.Secret, authn.Key{Type: authn.APIKey, IssuedAt: time.Now(), ExpiresAt: time.Now().Add(time.Minute)})
@@ -239,8 +239,8 @@ func TestIdentify(t *testing.T) {
 			err:  nil,
 		},
 		{
-			desc: "identify reset key",
+			desc: "identify recovery key",
-			key:  resetKey.Secret,
+			key:  recoveryKey.Secret,
 			id:   email,
 			err:  nil,
 		},

users/api/transport.go

@@ -233,6 +233,8 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) {
 			w.WriteHeader(http.StatusBadRequest)
 		case errors.Contains(errorVal, users.ErrUserNotFound):
 			w.WriteHeader(http.StatusBadRequest)
+		case errors.Contains(errorVal, users.ErrRecoveryToken):
+			w.WriteHeader(http.StatusInternalServerError)
 		}
 		if errorVal.Msg() != "" {
 			json.NewEncoder(w).Encode(errorRes{Err: errorVal.Msg()})

users/service.go

@@ -24,27 +24,26 @@ var (
 	// when accessing a protected resource.
 	ErrUnauthorizedAccess = errors.New("missing or invalid credentials provided")
 
-	// ErrNotFound indicates a non-existent entity request
+	// ErrNotFound indicates a non-existent entity request.
 	ErrNotFound = errors.New("non-existent entity")
 
-	// ErrUserNotFound indicates a non-existent user request
+	// ErrUserNotFound indicates a non-existent user request.
 	ErrUserNotFound = errors.New("non-existent user")
 
-	// ErrScanMetadata indicates problem with metadata in db
+	// ErrScanMetadata indicates problem with metadata in db.
 	ErrScanMetadata = errors.New("Failed to scan metadata")
 
-	// ErrMissingEmail indicates missing email for password reset request
+	// ErrMissingEmail indicates missing email for password reset request.
 	ErrMissingEmail = errors.New("missing email for password reset")
 
 	// ErrMissingResetToken indicates malformed or missing reset token
-	// for reseting password
+	// for reseting password.
 	ErrMissingResetToken = errors.New("error missing reset token")
 
-	// ErrGeneratingResetToken indicates error in generating password recovery
+	// ErrRecoveryToken indicates error in generating password recovery token.
-	// token
+	ErrRecoveryToken = errors.New("error generating password recovery token")
-	ErrGeneratingResetToken = errors.New("error missing reset token")
 
-	// ErrGetToken indicates error in getting signed token
+	// ErrGetToken indicates error in getting signed token.
 	ErrGetToken = errors.New("Get signed token failed")
 )
 
@@ -60,10 +59,10 @@ type Service interface {
 	// identified by the non-nil error values in the response.
 	Login(context.Context, User) (string, errors.Error)
 
-	// Get authenticated user info for the given token
+	// Get authenticated user info for the given token.
 	UserInfo(ctx context.Context, token string) (User, errors.Error)
 
-	// UpdateUser updates the user metadata
+	// UpdateUser updates the user metadata.
 	UpdateUser(ctx context.Context, token string, user User) errors.Error
 
 	// GenerateResetToken email where mail will be sent.
@@ -77,7 +76,7 @@ type Service interface {
 	// token can be authentication token or password reset token.
 	ResetPassword(_ context.Context, resetToken, password string) errors.Error
 
-	//SendPasswordReset sends reset password link to email
+	//SendPasswordReset sends reset password link to email.
 	SendPasswordReset(_ context.Context, host, email, token string) errors.Error
 }
 
@@ -163,7 +162,7 @@ func (svc usersService) GenerateResetToken(ctx context.Context, email, host stri
 
 	t, err := svc.issue(ctx, email, authn.RecoveryKey)
 	if err != nil {
-		return errors.Wrap(ErrGeneratingResetToken, err)
+		return errors.Wrap(ErrRecoveryToken, err)
 	}
 	return svc.SendPasswordReset(ctx, host, email, t)
 }