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.

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

Deprecate standalone SQL store level #4

Merged
merged 8 commits into from
Jul 13, 2024
35 changes: 3 additions & 32 deletions server/api/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,15 @@ import (
"net/http"

"github.com/mattermost/mattermost-plugin-boards/server/model"
"github.com/mattermost/mattermost-plugin-boards/server/services/auth"
"github.com/mattermost/mattermost-plugin-boards/server/utils"

"github.com/mattermost/mattermost/server/public/shared/mlog"
)

func (a *API) sessionRequired(handler func(w http.ResponseWriter, r *http.Request)) func(w http.ResponseWriter, r *http.Request) {
return a.attachSession(handler, true)
return a.attachSession(handler)
}

func (a *API) attachSession(handler func(w http.ResponseWriter, r *http.Request), required bool) func(w http.ResponseWriter, r *http.Request) {
func (a *API) attachSession(handler func(w http.ResponseWriter, r *http.Request)) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
token, _ := auth.ParseAuthTokenFromRequest(r)

if a.MattermostAuth && r.Header.Get("Mattermost-User-Id") != "" {
Rajat-Dabade marked this conversation as resolved.
Show resolved Hide resolved
userID := r.Header.Get("Mattermost-User-Id")
now := utils.GetMillis()
Expand All @@ -37,30 +32,6 @@ func (a *API) attachSession(handler func(w http.ResponseWriter, r *http.Request)
return
}

session, err := a.app.GetSession(token)
if err != nil {
if required {
a.errorResponse(w, r, model.NewErrUnauthorized(err.Error()))
return
}

handler(w, r)
return
}

authService := session.AuthService
if authService != a.authService {
msg := `Session authService mismatch`
a.logger.Error(msg,
mlog.String("sessionID", session.ID),
mlog.String("want", a.authService),
mlog.String("got", authService),
)
a.errorResponse(w, r, model.NewErrUnauthorized(msg))
return
}

ctx := context.WithValue(r.Context(), sessionContextKey, session)
handler(w, r.WithContext(ctx))
a.errorResponse(w, r, model.NewErrUnauthorized("Unauthorized"))
}
}
2 changes: 1 addition & 1 deletion server/api/blocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (

func (a *API) registerBlocksRoutes(r *mux.Router) {
// Blocks APIs
r.HandleFunc("/boards/{boardID}/blocks", a.attachSession(a.handleGetBlocks, false)).Methods("GET")
r.HandleFunc("/boards/{boardID}/blocks", a.attachSession(a.handleGetBlocks)).Methods("GET")
r.HandleFunc("/boards/{boardID}/blocks", a.sessionRequired(a.handlePostBlocks)).Methods("POST")
r.HandleFunc("/boards/{boardID}/blocks", a.sessionRequired(a.handlePatchBlocks)).Methods("PATCH")
r.HandleFunc("/boards/{boardID}/blocks/{blockID}", a.sessionRequired(a.handleDeleteBlock)).Methods("DELETE")
Expand Down
2 changes: 1 addition & 1 deletion server/api/boards.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
func (a *API) registerBoardsRoutes(r *mux.Router) {
r.HandleFunc("/teams/{teamID}/boards", a.sessionRequired(a.handleGetBoards)).Methods("GET")
r.HandleFunc("/boards", a.sessionRequired(a.handleCreateBoard)).Methods("POST")
r.HandleFunc("/boards/{boardID}", a.attachSession(a.handleGetBoard, false)).Methods("GET")
r.HandleFunc("/boards/{boardID}", a.attachSession(a.handleGetBoard)).Methods("GET")
r.HandleFunc("/boards/{boardID}", a.sessionRequired(a.handlePatchBoard)).Methods("PATCH")
r.HandleFunc("/boards/{boardID}", a.sessionRequired(a.handleDeleteBoard)).Methods("DELETE")
r.HandleFunc("/boards/{boardID}/duplicate", a.sessionRequired(a.handleDuplicateBoard)).Methods("POST")
Expand Down
4 changes: 2 additions & 2 deletions server/api/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ func FileInfoResponseFromJSON(data io.Reader) (*mmModel.FileInfo, error) {

func (a *API) registerFilesRoutes(r *mux.Router) {
// Files API
r.HandleFunc("/files/teams/{teamID}/{boardID}/{filename}", a.attachSession(a.handleServeFile, false)).Methods("GET")
r.HandleFunc("/files/teams/{teamID}/{boardID}/{filename}/info", a.attachSession(a.getFileInfo, false)).Methods("GET")
r.HandleFunc("/files/teams/{teamID}/{boardID}/{filename}", a.attachSession(a.handleServeFile)).Methods("GET")
r.HandleFunc("/files/teams/{teamID}/{boardID}/{filename}/info", a.attachSession(a.getFileInfo)).Methods("GET")
r.HandleFunc("/teams/{teamID}/{boardID}/files", a.sessionRequired(a.handleUploadFile)).Methods("POST")
}

Expand Down
5 changes: 0 additions & 5 deletions server/app/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ const (
SecondsPerMinute = 60
)

// GetSession Get a user active session and refresh the session if is needed.
func (a *App) GetSession(token string) (*model.Session, error) {
return a.auth.GetSession(token)
}

// IsValidReadToken validates the read token for a block.
func (a *App) IsValidReadToken(boardID string, readToken string) (bool, error) {
return a.auth.IsValidReadToken(boardID, readToken)
Expand Down
3 changes: 1 addition & 2 deletions server/app/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ func SetupTestHelper(t *testing.T) (*TestHelper, func()) {
filesBackend := &mocks.FileBackend{}
auth := auth.New(&cfg, store, nil)
logger, _ := mlog.NewLogger()
sessionToken := "TESTTOKEN"
wsserver := ws.NewServer(auth, sessionToken, false, logger, store)
wsserver := ws.NewServer(auth, logger, store)
webhook := webhook.NewClient(&cfg, logger)
metricsService := metrics.NewMetrics(metrics.InstanceInfo{})

Expand Down
18 changes: 0 additions & 18 deletions server/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@ import (
"github.com/mattermost/mattermost-plugin-boards/server/services/config"
"github.com/mattermost/mattermost-plugin-boards/server/services/permissions"
"github.com/mattermost/mattermost-plugin-boards/server/services/store"
"github.com/mattermost/mattermost-plugin-boards/server/utils"
"github.com/pkg/errors"
)

type AuthInterface interface {
GetSession(token string) (*model.Session, error)
IsValidReadToken(boardID string, readToken string) (bool, error)
DoesUserHaveTeamAccess(userID string, teamID string) bool
}
Expand All @@ -28,22 +26,6 @@ func New(config *config.Configuration, store store.Store, permissions permission
return &Auth{config: config, store: store, permissions: permissions}
}

// GetSession Get a user active session and refresh the session if needed.
func (a *Auth) GetSession(token string) (*model.Session, error) {
if len(token) < 1 {
return nil, errors.New("no session token")
}

session, err := a.store.GetSession(token, a.config.SessionExpireTime)
if err != nil {
return nil, errors.Wrap(err, "unable to get the session for the token")
}
if session.UpdateAt < (utils.GetMillis() - utils.SecondsToMillis(a.config.SessionRefreshTime)) {
_ = a.store.RefreshSession(session)
}
return session, nil
}

// IsValidReadToken validates the read token for a board.
func (a *Auth) IsValidReadToken(boardID string, readToken string) (bool, error) {
sharing, err := a.store.GetSharing(boardID)
Expand Down
28 changes: 0 additions & 28 deletions server/auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,37 +54,9 @@ func setupTestHelper(t *testing.T) *TestHelper {

func TestGetSession(t *testing.T) {
Rajat-Dabade marked this conversation as resolved.
Show resolved Hide resolved
th := setupTestHelper(t)

testcases := []struct {
title string
token string
refreshTime int64
isError bool
}{
{"fail, no token", "", 0, true},
{"fail, invalid username", "badToken", 0, true},
{"success, good token", "goodToken", 1000, false},
}

th.Store.EXPECT().GetSession("badToken", gomock.Any()).Return(nil, errors.New("Invalid Token"))
th.Store.EXPECT().GetSession("goodToken", gomock.Any()).Return(mockSession, nil)
th.Store.EXPECT().RefreshSession(gomock.Any()).Return(nil)

for _, test := range testcases {
t.Run(test.title, func(t *testing.T) {
if test.refreshTime > 0 {
th.Auth.config.SessionRefreshTime = test.refreshTime
}

session, err := th.Auth.GetSession(test.token)
if test.isError {
require.Error(t, err)
} else {
require.NoError(t, err)
require.NotNil(t, session)
}
})
}
}

Rajat-Dabade marked this conversation as resolved.
Show resolved Hide resolved
func TestIsValidReadToken(t *testing.T) {
Expand Down
10 changes: 4 additions & 6 deletions server/boards/boardsapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,11 @@ func NewBoardsApp(api model.ServicesAPI) (*BoardsApp, error) {
if err != nil {
return nil, fmt.Errorf("error initializing the DB: %w", err)
}
if cfg.AuthMode == server.MattermostAuthMod {
layeredStore, err2 := mattermostauthlayer.New(cfg.DBType, sqlDB, db, logger, api, storeParams.TablePrefix)
if err2 != nil {
return nil, fmt.Errorf("error initializing the DB: %w", err2)
}
db = layeredStore
layeredStore, err2 := mattermostauthlayer.New(cfg.DBType, sqlDB, db, logger, api, storeParams.TablePrefix)
if err2 != nil {
return nil, fmt.Errorf("error initializing the DB: %w", err2)
}
db = layeredStore

permissionsService := mmpermissions.New(db, api, logger)

Expand Down
3 changes: 1 addition & 2 deletions server/server/initHandlers.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package server

func (s *Server) initHandlers() {
cfg := s.config
s.api.MattermostAuth = cfg.AuthMode == MattermostAuthMod
s.api.MattermostAuth = true
Rajat-Dabade marked this conversation as resolved.
Show resolved Hide resolved
}
19 changes: 1 addition & 18 deletions server/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ import (
const (
cleanupSessionTaskFrequency = 10 * time.Minute
updateMetricsTaskFrequency = 15 * time.Minute

minSessionExpiryTime = int64(60 * 60 * 24 * 31) // 31 days

MattermostAuthMod = "mattermost"
)

type Server struct {
Expand Down Expand Up @@ -78,7 +74,7 @@ func New(params Params) (*Server, error) {
// if no ws adapter is provided, we spin up a websocket server
wsAdapter := params.WSAdapter
if wsAdapter == nil {
wsAdapter = ws.NewServer(authenticator, params.SingleUserToken, params.Cfg.AuthMode == MattermostAuthMod, params.Logger, params.DBStore)
wsAdapter = ws.NewServer(authenticator, params.Logger, params.DBStore)
}

filesBackendSettings := filestore.FileBackendSettings{}
Expand Down Expand Up @@ -253,19 +249,6 @@ func (s *Server) Start() error {
}
}

if s.config.AuthMode != MattermostAuthMod {
s.cleanUpSessionsTask = scheduler.CreateRecurringTask("cleanUpSessions", func() {
secondsAgo := minSessionExpiryTime
if secondsAgo < s.config.SessionExpireTime {
secondsAgo = s.config.SessionExpireTime
}

if err := s.store.CleanUpSessions(secondsAgo); err != nil {
s.logger.Error("Unable to clean up the sessions", mlog.Err(err))
}
}, cleanupSessionTaskFrequency)
}

metricsUpdater := func() {
blockCounts, err := s.store.GetBlockCountsByType()
if err != nil {
Expand Down
40 changes: 0 additions & 40 deletions server/services/store/mattermostauthlayer/mattermostauthlayer.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,22 +115,6 @@ func (s *MattermostAuthLayer) GetUserByUsername(username string) (*model.User, e
return &user, nil
}

func (s *MattermostAuthLayer) CreateUser(user *model.User) (*model.User, error) {
return nil, store.NewNotSupportedError("no user creation allowed from focalboard, create it using mattermost")
}

func (s *MattermostAuthLayer) UpdateUser(user *model.User) (*model.User, error) {
return nil, store.NewNotSupportedError("no update allowed from focalboard, update it using mattermost")
}

func (s *MattermostAuthLayer) UpdateUserPassword(username, password string) error {
return store.NewNotSupportedError("no update allowed from focalboard, update it using mattermost")
}

func (s *MattermostAuthLayer) UpdateUserPasswordByID(userID, password string) error {
return store.NewNotSupportedError("no update allowed from focalboard, update it using mattermost")
}

func (s *MattermostAuthLayer) PatchUserPreferences(userID string, patch model.UserPreferencesPatch) (mmModel.Preferences, error) {
preferences, err := s.GetUserPreferences(userID)
if err != nil {
Expand Down Expand Up @@ -236,30 +220,6 @@ func (s *MattermostAuthLayer) GetActiveUserCount(updatedSecondsAgo int64) (int,
return count, nil
}

func (s *MattermostAuthLayer) GetSession(token string, expireTime int64) (*model.Session, error) {
return nil, store.NewNotSupportedError("sessions not used when using mattermost")
}

func (s *MattermostAuthLayer) CreateSession(session *model.Session) error {
return store.NewNotSupportedError("no update allowed from focalboard, update it using mattermost")
}

func (s *MattermostAuthLayer) RefreshSession(session *model.Session) error {
return store.NewNotSupportedError("no update allowed from focalboard, update it using mattermost")
}

func (s *MattermostAuthLayer) UpdateSession(session *model.Session) error {
return store.NewNotSupportedError("no update allowed from focalboard, update it using mattermost")
}

func (s *MattermostAuthLayer) DeleteSession(sessionID string) error {
return store.NewNotSupportedError("no update allowed from focalboard, update it using mattermost")
}

func (s *MattermostAuthLayer) CleanUpSessions(expireTime int64) error {
return store.NewNotSupportedError("no update allowed from focalboard, update it using mattermost")
}

func (s *MattermostAuthLayer) GetTeam(id string) (*model.Team, error) {
if id == "0" {
team := model.Team{
Expand Down
14 changes: 0 additions & 14 deletions server/services/store/mockstore/mockstore.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading