oauth-manager: fix evaluation of should refresh function (#280)

* oauth-manager: fix evaluation of should refresh function

Author: jkralik

coap-gateway/schema/device/status/status.go

@@ -36,5 +36,5 @@ func (s Status) IsOnline() bool {
 	if s.ValidUntil <= 0 {
 		return s.State == State_Online
 	}
-	return time.Now().Before(time.Unix(s.ValidUntil, 0))
+	return time.Now().UnixNano() < time.Unix(s.ValidUntil, 0).UnixNano()
 }

coap-gateway/service/auth.go

@@ -11,7 +11,7 @@ import (
 )
 
 func isExpired(e time.Time) bool {
-	return !e.IsZero() && time.Now().After(e)
+	return !e.IsZero() && e.UnixNano() < time.Now().UnixNano()
 }
 
 func NewAuthInterceptor() kitNetCoap.Interceptor {

coap-gateway/service/client.go

@@ -52,6 +52,13 @@ func (a *authCtx) GetDeviceId() string {
 	return ""
 }
 
+func (a *authCtx) GetAccessToken() string {
+	if a != nil {
+		return a.AccessToken
+	}
+	return ""
+}
+
 const pendingDeviceSubscriptionToken = "pending"
 
 //Client a setup of connection
@@ -115,7 +122,7 @@ func (client *Client) cancelResourceSubscription(token string, wantWait bool) (b
 }
 
 func (client *Client) observeResource(ctx context.Context, deviceID, href string, observable, allowDuplicit bool) (err error) {
-	log.Debugf("coap-gw: client.observeResource /%v%v ins %v: observe resource", deviceID, href)
+	log.Debugf("coap-gw: observing resource /%v%v", deviceID, href)
 	instanceID := getInstanceID(href)
 	client.observedResourcesLock.Lock()
 	defer client.observedResourcesLock.Unlock()

coap-gateway/service/clientDeleteHandler_test.go

@@ -79,8 +79,9 @@ func Test_clientDeleteHandler(t *testing.T) {
 			ctx, cancel := context.WithTimeout(context.Background(), TestExchangeTimeout)
 			defer cancel()
 			req, err := tcp.NewDeleteRequest(ctx, tt.args.path)
+			require.NoError(t, err)
 			resp, err := co.Do(req)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.Equal(t, tt.wantsCode.String(), resp.Code().String())
 			if tt.wantsContent != nil {
 				require.NotEmpty(t, resp.Body())

coap-gateway/service/devicesStatusUpdater.go

@@ -66,7 +66,7 @@ func (u *devicesStatusUpdater) updateOnlineStatus(client *Client, validUntil tim
 		return time.Time{}, fmt.Errorf("cannot get service token: %w", err)
 	}
 	ctx := kitNetGrpc.CtxWithUserID(kitNetGrpc.CtxWithToken(client.Context(), serviceToken.AccessToken), authCtx.GetUserID())
-	if authCtx.Expire.Before(validUntil) {
+	if authCtx.Expire.UnixNano() < validUntil.UnixNano() {
 		validUntil = authCtx.Expire
 	}
 
@@ -85,7 +85,7 @@ func (u *devicesStatusUpdater) getDevicesToUpdate(now time.Time) []*deviceExpire
 		case <-d.client.Context().Done():
 			delete(u.devices, key)
 		default:
-			if now.Add(u.deviceStatusValidity / 2).After(d.expires) {
+			if d.expires.UnixNano() < now.Add(u.deviceStatusValidity/2).UnixNano() {
 				res = append(res, d)
 			}
 		}

coap-gateway/service/signUp.go

@@ -153,6 +153,9 @@ func signOffHandler(s mux.ResponseWriter, req *mux.Message, client *Client) {
 	if deviceID == "" {
 		deviceID = authCurrentCtx.GetDeviceId()
 	}
+	if accessToken == "" {
+		accessToken = authCurrentCtx.GetAccessToken()
+	}
 
 	err := validateSignOff(deviceID, accessToken)
 	if err != nil {

coap-gateway/service/signUp_test.go

@@ -74,3 +74,32 @@ func TestSignOffHandler(t *testing.T) {
 		t.Run(test.name, tf)
 	}
 }
+
+func TestSignOffWithSignInHandler(t *testing.T) {
+	shutdown := setUp(t)
+	defer shutdown()
+
+	co := testCoapDial(t, testCfg.GW_HOST)
+	if co == nil {
+		return
+	}
+	defer co.Close()
+
+	signUpEl := testEl{"signUp", input{coapCodes.POST, `{"di": "` + CertIdentity + `", "accesstoken":"` + oauthTest.DeviceAccessToken + `", "authprovider": "` + oauthTest.NewTestProvider().GetProviderName() + `"}`, nil}, output{coapCodes.Changed, TestCoapSignUpResponse{RefreshToken: "refresh-token", UserID: AuthorizationUserId}, nil}}
+	testPostHandler(t, uri.SignUp, signUpEl, co)
+
+	signInEl := testEl{"signIn", input{coapCodes.POST, `{"di": "` + CertIdentity + `", "uid":"` + AuthorizationUserId + `", "accesstoken":"` + oauthTest.DeviceAccessToken + `", "login": true }`, nil}, output{coapCodes.Changed, TestCoapSignInResponse{}, nil}}
+	testPostHandler(t, uri.SignIn, signInEl, co)
+
+	tbl := []testEl{
+		{"Deleted", input{coapCodes.DELETE, `{}`, nil}, output{coapCodes.Deleted, nil, nil}},
+	}
+
+	for _, test := range tbl {
+		tf := func(t *testing.T) {
+			// delete record for signUp
+			testPostHandler(t, uri.SignUp, test, co)
+		}
+		t.Run(test.name, tf)
+	}
+}

pkg/security/oauth/manager/manager.go

@@ -15,16 +15,16 @@ import (
 
 // Manager holds certificates from filesystem watched for changes
 type Manager struct {
-	mutex             sync.Mutex
-	config            clientcredentials.Config
-	requestTimeout    time.Duration
-	tickFrequency     time.Duration
-	startRefreshToken time.Time
-	token             *oauth2.Token
-	httpClient        *http.Client
-	tokenErr          error
-	doneWg            sync.WaitGroup
-	done              chan struct{}
+	mutex                sync.Mutex
+	config               clientcredentials.Config
+	requestTimeout       time.Duration
+	tickFrequency        time.Duration
+	nextTokenRenewalTime time.Time
+	token                *oauth2.Token
+	httpClient           *http.Client
+	tokenErr             error
+	doneWg               sync.WaitGroup
+	done                 chan struct{}
 }
 
 // NewManagerFromConfiguration creates a new oauth manager which refreshing token.
@@ -40,18 +40,19 @@ func NewManagerFromConfiguration(config Config, tlsCfg *tls.Config) (*Manager, e
 		Transport: t,
 		Timeout:   config.RequestTimeout,
 	}
-	token, startRefreshToken, err := getToken(cfg, httpClient, config.RequestTimeout)
+	token, nextTokenRenewalTime, err := getToken(cfg, httpClient, config.RequestTimeout)
 	if err != nil {
 		return nil, err
 	}
+	log.Infof("client credential token is refreshed, the next refresh token occurs after %v", nextTokenRenewalTime)
 
 	mgr := &Manager{
-		config:            cfg,
-		token:             token,
-		startRefreshToken: startRefreshToken,
-		requestTimeout:    config.RequestTimeout,
-		httpClient:        httpClient,
-		tickFrequency:     config.TickFrequency,
+		config:               cfg,
+		token:                token,
+		nextTokenRenewalTime: nextTokenRenewalTime,
+		requestTimeout:       config.RequestTimeout,
+		httpClient:           httpClient,
+		tickFrequency:        config.TickFrequency,
 
 		done: make(chan struct{}),
 	}
@@ -78,7 +79,23 @@ func (a *Manager) Close() {
 }
 
 func (a *Manager) shouldRefresh() bool {
-	return time.Now().After(a.startRefreshToken)
+	/*
+		We cannot use time.Now().After(a.nextTokenRenewalTime ) because
+		golang using monotonic clock for comparision.
+
+		So if we have 2 times:
+		// update time on PC to future eg: `date MMDDHHMM`
+		t1 := time.Now() eg (2021-06-15T12:00:00)
+		// return back time on PC: `date MMDDHHMM`
+		t2 := time.Now() eg (2021-06-01T12:00:00)
+		and then you call t2.After(t1) - it's return true :)
+
+		more info: https://github.com/golang/go/blob/master/src/time/time.go
+
+		the issue can occurs when pc hibernates.
+	*/
+
+	return time.Now().UnixNano() > a.nextTokenRenewalTime.UnixNano()
 }
 
 func getToken(cfg clientcredentials.Config, httpClient *http.Client, requestTimeout time.Duration) (*oauth2.Token, time.Time, error) {
@@ -88,24 +105,26 @@ func getToken(cfg clientcredentials.Config, httpClient *http.Client, requestTime
 	ctx = context.WithValue(ctx, oauth2.HTTPClient, httpClient)
 
 	token, err := cfg.Token(ctx)
-	var startRefreshToken time.Time
+	var nextTokenRenewalTime time.Time
 	if err == nil {
 		now := time.Now()
-		startRefreshToken = now.Add(token.Expiry.Sub(now) * 2 / 3)
+		nextTokenRenewalTime = now.Add(token.Expiry.Sub(now) * 2 / 3)
 	}
-	return token, startRefreshToken, err
+	return token, nextTokenRenewalTime, err
 }
 
 func (a *Manager) refreshToken() {
-	token, startRefreshToken, err := getToken(a.config, a.httpClient, a.requestTimeout)
+	token, nextTokenRenewalTime, err := getToken(a.config, a.httpClient, a.requestTimeout)
 	if err != nil {
 		log.Errorf("cannot refresh token: %v", err)
+	} else {
+		log.Infof("client credential token is refreshed, the next refresh token occurs after %v", nextTokenRenewalTime)
 	}
 	a.mutex.Lock()
 	defer a.mutex.Unlock()
 	a.token = token
 	a.tokenErr = err
-	a.startRefreshToken = startRefreshToken
+	a.nextTokenRenewalTime = nextTokenRenewalTime
 }
 
 func (a *Manager) watchToken() {