Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MF-1006 - Password recovery endpoint is not working #1007

Merged
merged 1 commit into from
Jan 16, 2020
Merged

MF-1006 - Password recovery endpoint is not working #1007

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
72 changes: 50 additions & 22 deletions authn/api/grpc/endpoint_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,38 +46,66 @@ 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()})
assert.Nil(t, err, fmt.Sprintf("Issuing login key expected to succeed: %s", err))
userKey, err := svc.Issue(context.Background(), email, authn.Key{Type: authn.UserKey, IssuedAt: time.Now()})
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 {
token string
id string
kind uint32
err error
cases := []struct {
desc string
id string
kind uint32
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()})
assert.Nil(t, err, fmt.Sprintf("Issuing login key expected to succeed: %s", err))
userKey, err := svc.Issue(context.Background(), email, authn.Key{Type: authn.UserKey, IssuedAt: time.Now()})
assert.Nil(t, err, fmt.Sprintf("Issuing user 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))
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))

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))
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())
Expand All @@ -90,19 +118,19 @@ func TestIdentify(t *testing.T) {
err error
}{
{
desc: "identify user with reset token",
token: resetKey.Secret,
desc: "identify user with recovery token",
token: recoveryKey.Secret,
id: email,
err: nil,
},
{
desc: "identify user with user token",
token: userKey.Secret,
desc: "identify user with API token",
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"),
Expand Down
46 changes: 23 additions & 23 deletions authn/api/http/endpoint_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()})
assert.Nil(t, err, fmt.Sprintf("Issuing login key expected to succeed: %s", err))
userKey, err := svc.Issue(context.Background(), email, authn.Key{Type: authn.UserKey, IssuedAt: time.Now()})
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
Expand All @@ -99,48 +99,48 @@ func TestIssue(t *testing.T) {
status int
}{
{
desc: "issue login key",
req: toJSON(lk),
desc: "issue user key",
req: toJSON(uk),
ct: contentType,
token: "",
status: http.StatusCreated,
},
{
desc: "issue user key",
req: toJSON(uk),
desc: "issue API key",
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",
req: toJSON(lk),
ct: "", token: loginKey.Secret,
desc: "issue user key wrong content type",
req: toJSON(uk),
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: "",
Expand Down Expand Up @@ -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()})
assert.Nil(t, err, fmt.Sprintf("Issuing login key expected to succeed: %s", err))
userKey, err := svc.Issue(context.Background(), email, authn.Key{Type: authn.UserKey, IssuedAt: time.Now()})
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)
assert.Nil(t, err, fmt.Sprintf("Issuing login key expected to succeed: %s", err))
k, err := svc.Issue(context.Background(), userKey.Secret, key)
assert.Nil(t, err, fmt.Sprintf("Issuing user key expected to succeed: %s", err))

ts := newServer(svc)
defer ts.Close()
Expand All @@ -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,
},
{
Expand Down
26 changes: 6 additions & 20 deletions authn/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import (
)

const (
loginDuration = 10 * time.Hour
resetDuration = 5 * time.Minute
issuerName = "mainflux.authn"
loginDuration = 10 * time.Hour
recoveryDuration = 5 * time.Minute
issuerName = "mainflux.authn"
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use this as default values and provide env variables? IMHO 5min is to short for recovery token, I would set at least 15+ min


var (
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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)
Expand Down
30 changes: 15 additions & 15 deletions authn/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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(),
Expand All @@ -48,24 +48,24 @@ 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,
},
issuer: email,
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(),
Expand All @@ -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,
},
}
Expand Down Expand Up @@ -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)})
Expand All @@ -239,8 +239,8 @@ func TestIdentify(t *testing.T) {
err: nil,
},
{
desc: "identify reset key",
key: resetKey.Secret,
desc: "identify recovery key",
key: recoveryKey.Secret,
id: email,
err: nil,
},
Expand Down
2 changes: 2 additions & 0 deletions users/api/transport.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()})
Expand Down
Loading