From ed9c3e2aa3db0f31ac7f25fa8b1c7835f463ceb5 Mon Sep 17 00:00:00 2001 From: Caleb Doxsey Date: Tue, 31 Jan 2023 09:41:09 -0700 Subject: [PATCH] identity: fix nil reference error when there is no authenticator (#3930) --- internal/identity/manager/manager.go | 21 +++++++++++++++--- internal/identity/manager/manager_test.go | 27 +++++++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/internal/identity/manager/manager.go b/internal/identity/manager/manager.go index 520810bea0a..8ec1c7c34fa 100644 --- a/internal/identity/manager/manager.go +++ b/internal/identity/manager/manager.go @@ -187,6 +187,16 @@ func (mgr *Manager) refreshSession(ctx context.Context, userID, sessionID string Str("session_id", sessionID). Msg("refreshing session") + authenticator := mgr.cfg.Load().authenticator + if authenticator == nil { + log.Info(ctx). + Str("user_id", userID). + Str("session_id", sessionID). + Msg("no authenticator defined, deleting session") + mgr.deleteSession(ctx, userID, sessionID) + return + } + s, ok := mgr.sessions.Get(userID, sessionID) if !ok { log.Warn(ctx). @@ -214,7 +224,7 @@ func (mgr *Manager) refreshSession(ctx context.Context, userID, sessionID string return } - newToken, err := mgr.cfg.Load().authenticator.Refresh(ctx, FromOAuthToken(s.OauthToken), &s) + newToken, err := authenticator.Refresh(ctx, FromOAuthToken(s.OauthToken), &s) metrics.RecordIdentityManagerSessionRefresh(ctx, err) mgr.recordLastError(metrics_ids.IdentityManagerLastSessionRefreshError, err) if isTemporaryError(err) { @@ -233,7 +243,7 @@ func (mgr *Manager) refreshSession(ctx context.Context, userID, sessionID string } s.OauthToken = ToOAuthToken(newToken) - err = mgr.cfg.Load().authenticator.UpdateUserInfo(ctx, FromOAuthToken(s.OauthToken), &s) + err = authenticator.UpdateUserInfo(ctx, FromOAuthToken(s.OauthToken), &s) metrics.RecordIdentityManagerUserRefresh(ctx, err) mgr.recordLastError(metrics_ids.IdentityManagerLastUserRefreshError, err) if isTemporaryError(err) { @@ -268,6 +278,11 @@ func (mgr *Manager) refreshUser(ctx context.Context, userID string) { Str("user_id", userID). Msg("refreshing user") + authenticator := mgr.cfg.Load().authenticator + if authenticator == nil { + return + } + u, ok := mgr.users.Get(userID) if !ok { log.Warn(ctx). @@ -286,7 +301,7 @@ func (mgr *Manager) refreshUser(ctx context.Context, userID string) { continue } - err := mgr.cfg.Load().authenticator.UpdateUserInfo(ctx, FromOAuthToken(s.OauthToken), &u) + err := authenticator.UpdateUserInfo(ctx, FromOAuthToken(s.OauthToken), &u) metrics.RecordIdentityManagerUserRefresh(ctx, err) mgr.recordLastError(metrics_ids.IdentityManagerLastUserRefreshError, err) if isTemporaryError(err) { diff --git a/internal/identity/manager/manager_test.go b/internal/identity/manager/manager_test.go index 4eee0c69499..ff73c6d350f 100644 --- a/internal/identity/manager/manager_test.go +++ b/internal/identity/manager/manager_test.go @@ -9,6 +9,8 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "golang.org/x/oauth2" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/timestamppb" @@ -36,6 +38,31 @@ func (mock mockAuthenticator) UpdateUserInfo(_ context.Context, _ *oauth2.Token, return errors.New("update user info") } +func TestManager_refresh(t *testing.T) { + ctrl := gomock.NewController(t) + ctx, clearTimeout := context.WithTimeout(context.Background(), time.Second*10) + t.Cleanup(clearTimeout) + + client := mock_databroker.NewMockDataBrokerServiceClient(ctrl) + mgr := New(WithDataBrokerClient(client)) + mgr.onUpdateRecords(ctx, updateRecordsMessage{ + records: []*databroker.Record{ + databroker.NewRecord(&session.Session{ + Id: "s1", + UserId: "u1", + OauthToken: &session.OAuthToken{}, + ExpiresAt: timestamppb.New(time.Now().Add(time.Second * 10)), + }), + databroker.NewRecord(&user.User{ + Id: "u1", + }), + }, + }) + client.EXPECT().Get(gomock.Any(), gomock.Any()).Return(nil, status.Error(codes.NotFound, "not found")) + mgr.refreshSession(ctx, "u1", "s1") + mgr.refreshUser(ctx, "u1") +} + func TestManager_onUpdateRecords(t *testing.T) { ctrl := gomock.NewController(t)