You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by ti...@apache.org on 2021/02/27 11:00:39 UTC

[servicecomb-service-center] branch master updated: Bugfix/expiration time validator (#871)

This is an automated email from the ASF dual-hosted git repository.

tianxiaoliang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-service-center.git


The following commit(s) were added to refs/heads/master by this push:
     new 9d813ed  Bugfix/expiration time validator (#871)
9d813ed is described below

commit 9d813ed691dc184f73ba17c5c38e9fd242c56d1f
Author: Dante Sun <da...@gmail.com>
AuthorDate: Sat Feb 27 19:00:33 2021 +0800

    Bugfix/expiration time validator (#871)
    
    * 'go mod tidy'
    
    * More ignore rules
    
    * validating for token exipration time
    
    * Fixing ut follow the new rule of expiration time
---
 .gitignore                                         | 10 +++-
 go.mod                                             |  8 +++-
 pkg/validate/token_expiration_time_checker.go      | 53 ++++++++++++++++++++++
 pkg/validate/token_expiration_time_checker_test.go | 39 ++++++++++++++++
 server/resource/v4/auth_resource.go                | 11 +++--
 server/resource/v4/rbac_resource_test.go           | 13 +++++-
 server/service/validate.go                         |  4 +-
 7 files changed, 126 insertions(+), 12 deletions(-)

diff --git a/.gitignore b/.gitignore
index 1c85ed4..52b54cc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -39,4 +39,12 @@ docs/_templates
 .DS_Store
 main
 # go mod
-go.sum
\ No newline at end of file
+go.sum
+
+# VS Code Project files
+.vscode
+
+# Delve 
+__debug_bin
+private.key
+rbac.pub
\ No newline at end of file
diff --git a/go.mod b/go.mod
index 1f1de6c..665e8c4 100644
--- a/go.mod
+++ b/go.mod
@@ -19,14 +19,16 @@ require (
 	github.com/ghodss/yaml v1.0.0
 	github.com/go-chassis/cari v0.0.2-0.20210208095358-3bccdf2ce456
 	github.com/go-chassis/foundation v0.2.2
-	github.com/go-chassis/go-archaius v1.3.6-0.20201210061741-7450779aaeb8
+	github.com/go-chassis/go-archaius v1.4.0
 	github.com/go-chassis/go-chassis/v2 v2.1.1-0.20210218100404-85e04ad6bd31
 	github.com/go-chassis/kie-client v0.0.0-20210122061843-eee856b0a9af
 	github.com/golang/protobuf v1.4.2
+	github.com/google/go-cmp v0.5.4 // indirect
 	github.com/gorilla/websocket v1.4.2
 	github.com/grpc-ecosystem/go-grpc-middleware v1.2.2 // indirect
 	github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 // indirect
 	github.com/grpc-ecosystem/grpc-gateway v1.16.0 // indirect
+	github.com/hashicorp/golang-lru v0.5.4 // indirect
 	github.com/hashicorp/serf v0.8.3
 	github.com/iancoleman/strcase v0.1.2
 	github.com/jonboulle/clockwork v0.2.2 // indirect
@@ -34,7 +36,6 @@ require (
 	github.com/karlseguin/expect v1.0.7 // indirect
 	github.com/labstack/echo/v4 v4.1.18-0.20201218141459-936c48a17e97
 	github.com/mattn/go-runewidth v0.0.9 // indirect
-	github.com/mitchellh/mapstructure v1.3.3 // indirect
 	github.com/natefinch/lumberjack v0.0.0-20170531160350-a96e63847dc3
 	github.com/olekukonko/tablewriter v0.0.0-20180506121414-d4647c9c7a84
 	github.com/onsi/ginkgo v1.14.0
@@ -49,6 +50,7 @@ require (
 	github.com/prometheus/procfs v0.0.0-20190117184657-bf6a532e95b1
 	github.com/rs/cors v0.0.0-20170608165155-8dd4211afb5d // v1.1
 	github.com/satori/go.uuid v1.1.0
+	github.com/sirupsen/logrus v1.6.0 // indirect
 	github.com/soheilhy/cmux v0.1.4 // indirect
 	github.com/spf13/cobra v0.0.3
 	github.com/spf13/pflag v1.0.5
@@ -62,6 +64,8 @@ require (
 	go.uber.org/multierr v1.6.0 // indirect
 	go.uber.org/zap v1.10.0
 	golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a
+	golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9 // indirect
+	golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 // indirect
 	golang.org/x/text v0.3.5 // indirect
 	golang.org/x/time v0.0.0-20190308202827-9d24e82272b4
 	google.golang.org/grpc v1.33.1
diff --git a/pkg/validate/token_expiration_time_checker.go b/pkg/validate/token_expiration_time_checker.go
new file mode 100644
index 0000000..7cc919f
--- /dev/null
+++ b/pkg/validate/token_expiration_time_checker.go
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package validate
+
+import (
+	"time"
+
+	"github.com/apache/servicecomb-service-center/pkg/log"
+)
+
+// TokenExpirationTimeChecker validates Account.TokenExpirationTime should >= 15m and <=24h
+type TokenExpirationTimeChecker struct {
+}
+
+const (
+	//MinTokenDuration the minimum time duration of a token
+	MinTokenDuration time.Duration = time.Minute * 15
+	//MaxTokenDuration the maximum time duration of a token
+	MaxTokenDuration time.Duration = time.Hour * 24
+)
+
+//MatchString ensures TokenExpirationTime is a valid time.Duration and in legal range.
+func (p *TokenExpirationTimeChecker) MatchString(s string) bool {
+	duration, err := time.ParseDuration(s)
+	if err != nil {
+		log.Errorf(err, "Invalid duration value '%s'", s)
+		return false
+	}
+	if duration > MaxTokenDuration || duration < MinTokenDuration {
+		log.Errorf(err, "TokenExpirationTime('%s') should >= 15m and <= 24h.", s)
+		return false
+	}
+	return true
+}
+
+func (p *TokenExpirationTimeChecker) String() string {
+	return "TokenExpirationTime"
+}
diff --git a/pkg/validate/token_expiration_time_checker_test.go b/pkg/validate/token_expiration_time_checker_test.go
new file mode 100644
index 0000000..6f93919
--- /dev/null
+++ b/pkg/validate/token_expiration_time_checker_test.go
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package validate
+
+import (
+	"testing"
+)
+
+func TestExpirationTime_MatchString(t *testing.T) {
+	checker := TokenExpirationTimeChecker{}
+	if !checker.MatchString("30m") {
+		t.Fail()
+	}
+	if checker.MatchString("00s") {
+		t.Fatalf("0s is not between 15m and 24h")
+	}
+	if checker.MatchString("14m59s") {
+		t.Fatalf("0s is not between 15m and 24h")
+	}
+	if checker.MatchString("24h1s") {
+		t.Fatalf("0s is not between 15m and 24h")
+	}
+
+}
diff --git a/server/resource/v4/auth_resource.go b/server/resource/v4/auth_resource.go
index c8712f4..ba8aba5 100644
--- a/server/resource/v4/auth_resource.go
+++ b/server/resource/v4/auth_resource.go
@@ -20,11 +20,12 @@ package v4
 import (
 	"context"
 	"encoding/json"
-	"github.com/apache/servicecomb-service-center/pkg/rbacframe"
-	rbacsvc "github.com/apache/servicecomb-service-center/server/service/rbac"
 	"io/ioutil"
 	"net/http"
 
+	"github.com/apache/servicecomb-service-center/pkg/rbacframe"
+	rbacsvc "github.com/apache/servicecomb-service-center/server/service/rbac"
+
 	"github.com/apache/servicecomb-service-center/datasource"
 	errorsEx "github.com/apache/servicecomb-service-center/pkg/errors"
 	"github.com/apache/servicecomb-service-center/pkg/log"
@@ -195,14 +196,14 @@ func (r *AuthResource) Login(w http.ResponseWriter, req *http.Request) {
 		controller.WriteError(w, discovery.ErrInvalidParams, err.Error())
 		return
 	}
+	if a.TokenExpirationTime == "" {
+		a.TokenExpirationTime = "30m"
+	}
 	err = service.ValidateAccountLogin(a)
 	if err != nil {
 		controller.WriteError(w, discovery.ErrInvalidParams, err.Error())
 		return
 	}
-	if a.TokenExpirationTime == "" {
-		a.TokenExpirationTime = "30m"
-	}
 	t, err := authr.Login(context.TODO(), a.Name, a.Password,
 		authr.ExpireAfter(a.TokenExpirationTime))
 	if err != nil {
diff --git a/server/resource/v4/rbac_resource_test.go b/server/resource/v4/rbac_resource_test.go
index e5ef35e..edf25fe 100644
--- a/server/resource/v4/rbac_resource_test.go
+++ b/server/resource/v4/rbac_resource_test.go
@@ -21,13 +21,14 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
-	rbacmodel "github.com/go-chassis/cari/rbac"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"testing"
 	"time"
 
+	rbacmodel "github.com/go-chassis/cari/rbac"
+
 	"github.com/apache/servicecomb-service-center/pkg/rest"
 	"github.com/apache/servicecomb-service-center/server/config"
 	v4 "github.com/apache/servicecomb-service-center/server/resource/v4"
@@ -224,6 +225,14 @@ func TestAuthResource_GetAccount(t *testing.T) {
 		r, _ := http.NewRequest(http.MethodPost, "/v4/token", bytes.NewBuffer(b))
 		w := httptest.NewRecorder()
 		rest.GetRouter().ServeHTTP(w, r)
+		assert.Equal(t, http.StatusBadRequest, w.Code)
+	})
+
+	t.Run("get a expiration token", func(t *testing.T) {
+		b, _ := json.Marshal(&rbacmodel.Account{Name: "root", Password: "Complicated_password1", TokenExpirationTime: "15m"})
+		r, _ := http.NewRequest(http.MethodPost, "/v4/token", bytes.NewBuffer(b))
+		w := httptest.NewRecorder()
+		rest.GetRouter().ServeHTTP(w, r)
 		assert.Equal(t, http.StatusOK, w.Code)
 		to := &rbacmodel.Token{}
 		json.Unmarshal(w.Body.Bytes(), to)
@@ -233,7 +242,7 @@ func TestAuthResource_GetAccount(t *testing.T) {
 		r3.Header.Set(restful.HeaderAuth, "Bearer "+to.TokenStr)
 		w3 := httptest.NewRecorder()
 		rest.GetRouter().ServeHTTP(w3, r3)
-		assert.Equal(t, http.StatusUnauthorized, w3.Code)
+		assert.Equal(t, http.StatusOK, w3.Code)
 	})
 }
 
diff --git a/server/service/validate.go b/server/service/validate.go
index 1e443a5..c91c9f8 100644
--- a/server/service/validate.go
+++ b/server/service/validate.go
@@ -35,7 +35,6 @@ var accountLoginValidator = &validate.Validator{}
 func init() {
 	var roleRegex, _ = regexp.Compile(`^$|^(admin|developer|[a-zA-Z]\w{2,15})$`)
 	var accountRegex, _ = regexp.Compile(`^[a-zA-Z]\w{3,15}$`)
-	var expirationRegex, _ = regexp.Compile(`^$|^(\d{1,2}d|\d{1,2}h|\d{1,3}m|\d{2,3}s)$`)
 	createAccountValidator.AddRule("Name", &validate.Rule{Regexp: accountRegex})
 	createAccountValidator.AddRule("Roles", &validate.Rule{Regexp: roleRegex})
 	createAccountValidator.AddRule("Password", &validate.Rule{Regexp: &validate.PasswordChecker{}})
@@ -43,8 +42,9 @@ func init() {
 	changePWDValidator.AddRule("Password", &validate.Rule{Regexp: &validate.PasswordChecker{}})
 	changePWDValidator.AddRule("Name", &validate.Rule{Regexp: accountRegex})
 
-	accountLoginValidator.AddRule("TokenExpirationTime", &validate.Rule{Regexp: expirationRegex})
+	accountLoginValidator.AddRule("TokenExpirationTime", &validate.Rule{Regexp: &validate.TokenExpirationTimeChecker{}})
 }
+
 func Validate(v interface{}) error {
 	err := baseCheck(v)
 	if err != nil {