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

[MI-3082]: Fixed the issue 198 'Added the logic to refresh and store the token' #7

Merged
merged 8 commits into from
May 30, 2023
Merged

[MI-3082]: Fixed the issue 198 'Added the logic to refresh and store the token' #7

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e58d5ab
[MI-3082]:Added refresh token logic and refactored the code
Kshitij-Katiyar May 15, 2023
c1dfd90
[MI-3082]:Fixed lint errors
Kshitij-Katiyar May 15, 2023
f78628c
[MI-3082]: Fixed review comments
Kshitij-Katiyar May 16, 2023
848fefe
[MI-3082]: Fixed review comment
Kshitij-Katiyar May 16, 2023
059b440
[MI-3082]:Fixed the review comments
Kshitij-Katiyar May 19, 2023
4937b66
[MI-3082]:Fixed review comments
Kshitij-Katiyar May 19, 2023
d4922cb
[MI-3082]:Fixed review comments
Kshitij-Katiyar May 22, 2023
a649090
[MI-3082]:Fixed review comments
Kshitij-Katiyar May 29, 2023
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
10 changes: 8 additions & 2 deletions server/mscalendar/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"context"

"github.com/mattermost/mattermost-plugin-mscalendar/server/remote"
"github.com/mattermost/mattermost-plugin-mscalendar/server/store"
"github.com/mattermost/mattermost-plugin-mscalendar/server/serializer"
)

type Client interface {
Expand All @@ -21,7 +21,13 @@ func (m *mscalendar) MakeClient() (remote.Client, error) {
return nil, err
}

return m.Remote.MakeClient(context.Background(), m.actingUser.OAuth2Token, m.Store, m.actingUser.MattermostUserID, store.GetCheckUserStatus(m.Store, m.Logger, m.actingUser.MattermostUserID), store.GetChangeUserStatus(m.Store, m.Logger, m.actingUser.MattermostUserID, m.Poster)), nil
tokenHelpers := &serializer.UserTokenHelpers{
CheckUserStatus: m.Store.MakeCheckUserStatus,
ChangeUserStatus: m.Store.MakeChangeUserStatus,
RefreshAndStoreToken: m.Store.RefreshAndStoreToken,
}

return m.Remote.MakeUserClient(context.Background(), m.actingUser.OAuth2Token, m.actingUser.MattermostUserID, m.Poster, tokenHelpers), nil
}

func (m *mscalendar) MakeSuperuserClient() (remote.Client, error) {
Expand Down
7 changes: 6 additions & 1 deletion server/mscalendar/notification.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,12 @@ func (processor *notificationProcessor) processNotification(n *remote.Notificati
n.Subscription = sub.Remote
n.SubscriptionCreator = creator.Remote

client := processor.Remote.MakeClient(context.Background(), creator.OAuth2Token, processor.Store, sub.MattermostCreatorID, store.GetCheckUserStatus(processor.Store, processor.Logger, sub.MattermostCreatorID), store.GetChangeUserStatus(processor.Store, processor.Logger, sub.MattermostCreatorID, processor.Poster))
tokenHelpers := &serializer.UserTokenHelpers{
CheckUserStatus: processor.Store.MakeCheckUserStatus,
ChangeUserStatus: processor.Store.MakeChangeUserStatus,
RefreshAndStoreToken: processor.Store.RefreshAndStoreToken,
}
client := processor.Remote.MakeUserClient(context.Background(), creator.OAuth2Token, sub.MattermostCreatorID, processor.Poster, tokenHelpers)

if n.RecommendRenew {
var renewed *serializer.Subscription
Expand Down
6 changes: 3 additions & 3 deletions server/mscalendar/notification_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,11 @@ func TestProcessNotification(t *testing.T) {
mockStore.EXPECT().LoadUser("creator_mm_id").Return(user, nil).Times(1)

if tc.notification.ClientState == subscription.Remote.ClientState {
mockRemote.EXPECT().MakeClient(context.Background(), &oauth2.Token{
mockRemote.EXPECT().MakeUserClient(context.Background(), &oauth2.Token{
AccessToken: "creator_oauth_token",
}, mockStore, "creator_mm_id", gomock.Any(), gomock.Any()).Return(mockClient).Times(1)
mockClient.EXPECT().GetMailboxSettings(user.Remote.ID).Return(&remote.MailboxSettings{TimeZone: "Eastern Standard Time"}, nil)
}, "creator_mm_id", mockPoster, gomock.Any()).Return(mockClient)

mockClient.EXPECT().GetMailboxSettings(user.Remote.ID).Return(&remote.MailboxSettings{TimeZone: "Eastern Standard Time"}, nil)
if tc.notification.RecommendRenew {
mockClient.EXPECT().RenewSubscription("remote_subscription_id").Return(&serializer.Subscription{}, nil).Times(1)
mockStore.EXPECT().StoreUserSubscription(user, &store.Subscription{
Expand Down
9 changes: 8 additions & 1 deletion server/mscalendar/oauth2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"golang.org/x/oauth2"

"github.com/mattermost/mattermost-plugin-mscalendar/server/config"
"github.com/mattermost/mattermost-plugin-mscalendar/server/serializer"
"github.com/mattermost/mattermost-plugin-mscalendar/server/store"
"github.com/mattermost/mattermost-plugin-mscalendar/server/utils/oauth2connect"
)
Expand Down Expand Up @@ -71,7 +72,13 @@ func (app *oauth2App) CompleteOAuth2(authedUserID, code, state string) error {
return err
}

client := app.Remote.MakeClient(ctx, tok, app.Store, mattermostUserID, store.GetCheckUserStatus(app.Store, app.Logger, mattermostUserID), store.GetChangeUserStatus(app.Store, app.Logger, mattermostUserID, app.Poster))
tokenHelpers := &serializer.UserTokenHelpers{
CheckUserStatus: app.Store.MakeCheckUserStatus,
ChangeUserStatus: app.Store.MakeChangeUserStatus,
RefreshAndStoreToken: app.Store.RefreshAndStoreToken,
}

client := app.Remote.MakeClient(ctx, tok, mattermostUserID, app.Poster, tokenHelpers)
Copy link

Choose a reason for hiding this comment

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

Should this be MakeUserClient instead?

Copy link
Collaborator Author

@Kshitij-Katiyar Kshitij-Katiyar May 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@mickmister I guess this gets run during the Oauth, so I thought this won't need the refresh token part here.

Copy link

@mickmister mickmister May 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

so I thought this won't need the refresh token part here.

I would say that we should still call MakeUserClient, to differentiate that this is a client for a user, and not a superuser client

me, err := client.GetMe()
if err != nil {
return err
Expand Down
54 changes: 34 additions & 20 deletions server/remote/mock_remote/mock_remote.go
View file
Open in desktop

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

10 changes: 6 additions & 4 deletions server/remote/msgraph/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
msgraph "github.com/yaegashi/msgraph.go/v1.0"

"github.com/mattermost/mattermost-plugin-mscalendar/server/config"
"github.com/mattermost/mattermost-plugin-mscalendar/server/serializer"
"github.com/mattermost/mattermost-plugin-mscalendar/server/utils/bot"
)

Expand All @@ -21,9 +22,10 @@ type client struct {
httpClient *http.Client
rbuilder *msgraph.GraphServiceRequestBuilder

conf *config.Config
bot.Logger
mattermostUserID string
conf *config.Config
tokenHelpers *serializer.UserTokenHelpers
Copy link

Choose a reason for hiding this comment

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

I'm not sure if serializier is the right package for UserTokenHelpers. It's only used in remote, and it's not ever serialized.

Copy link
Collaborator Author

@Kshitij-Katiyar Kshitij-Katiyar May 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@mickmister I have moved this to the remote package


checkUserStatus func() bool
changeUserStatus func(error)
bot.Logger
bot.Poster
}
6 changes: 3 additions & 3 deletions server/remote/msgraph/create_calendar.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ import (
// CreateCalendar creates a calendar
func (c *client) CreateCalendar(remoteUserID string, calIn *remote.Calendar) (*remote.Calendar, error) {
var calOut = remote.Calendar{}
if !c.checkUserStatus() {
c.Logger.Warnf(LogUserInactive)
if c.tokenHelpers != nil && !c.tokenHelpers.CheckUserStatus(c.Logger, c.mattermostUserID) {
c.Logger.Warnf(LogUserInactive, c.mattermostUserID)
return nil, errors.New(ErrorUserInactive)
}

err := c.rbuilder.Users().ID(remoteUserID).Calendars().Request().JSONRequest(c.ctx, http.MethodPost, "", &calIn, &calOut)
if err != nil {
c.changeUserStatus(err)
c.tokenHelpers.ChangeUserStatus(err, c.Logger, c.mattermostUserID, c.Poster)
Kshitij-Katiyar marked this conversation as resolved.
Show resolved Hide resolved
return nil, errors.Wrap(err, "msgraph CreateCalendar")
}
c.Logger.With(bot.LogContext{
Expand Down
6 changes: 3 additions & 3 deletions server/remote/msgraph/create_event.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ import (
// CreateEvent creates a calendar event
func (c *client) CreateEvent(remoteUserID string, in *serializer.Event) (*serializer.Event, error) {
var out = serializer.Event{}
if !c.checkUserStatus() {
c.Logger.Warnf(LogUserInactive)
if c.tokenHelpers != nil && !c.tokenHelpers.CheckUserStatus(c.Logger, c.mattermostUserID) {
c.Logger.Warnf(LogUserInactive, c.mattermostUserID)
return nil, errors.New(ErrorUserInactive)
}

err := c.rbuilder.Users().ID(remoteUserID).Events().Request().JSONRequest(c.ctx, http.MethodPost, "", &in, &out)
if err != nil {
c.changeUserStatus(err)
c.tokenHelpers.ChangeUserStatus(err, c.Logger, c.mattermostUserID, c.Poster)
return nil, errors.Wrap(err, "msgraph CreateEvent")
}
return &out, nil
Expand Down
6 changes: 3 additions & 3 deletions server/remote/msgraph/delete_calendar.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ import (
)

func (c *client) DeleteCalendar(remoteUserID string, calID string) error {
if !c.checkUserStatus() {
c.Logger.Warnf(LogUserInactive)
if c.tokenHelpers != nil && !c.tokenHelpers.CheckUserStatus(c.Logger, c.mattermostUserID) {
c.Logger.Warnf(LogUserInactive, c.mattermostUserID)
return errors.New(ErrorUserInactive)
}
err := c.rbuilder.Users().ID(remoteUserID).Calendars().ID(calID).Request().Delete(c.ctx)
if err != nil {
c.changeUserStatus(err)
c.tokenHelpers.ChangeUserStatus(err, c.Logger, c.mattermostUserID, c.Poster)
return errors.Wrap(err, "msgraph DeleteCalendar")
}
c.Logger.With(bot.LogContext{}).Infof("msgraph: DeleteCalendar deleted calendar `%v`.", calID)
Expand Down
24 changes: 12 additions & 12 deletions server/remote/msgraph/event.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,30 +14,30 @@ import (

func (c *client) GetEvent(remoteUserID, eventID string) (*serializer.Event, error) {
e := &serializer.Event{}
if !c.checkUserStatus() {
c.Logger.Warnf(LogUserInactive)
if !c.tokenHelpers.CheckUserStatus(c.Logger, c.mattermostUserID) {
c.Logger.Warnf(LogUserInactive, c.mattermostUserID)
return nil, errors.New(ErrorUserInactive)
}

err := c.rbuilder.Users().ID(remoteUserID).Events().ID(eventID).Request().JSONRequest(
c.ctx, http.MethodGet, "", nil, &e)
if err != nil {
c.changeUserStatus(err)
c.tokenHelpers.ChangeUserStatus(err, c.Logger, c.mattermostUserID, c.Poster)
return nil, errors.Wrap(err, "msgraph GetEvent")
}
return e, nil
}

func (c *client) AcceptEvent(remoteUserID, eventID string) error {
dummy := &msgraph.EventAcceptRequestParameter{}
if !c.checkUserStatus() {
c.Logger.Warnf(LogUserInactive)
if !c.tokenHelpers.CheckUserStatus(c.Logger, c.mattermostUserID) {
c.Logger.Warnf(LogUserInactive, c.mattermostUserID)
return errors.New(ErrorUserInactive)
}

err := c.rbuilder.Users().ID(remoteUserID).Events().ID(eventID).Accept(dummy).Request().Post(c.ctx)
if err != nil {
c.changeUserStatus(err)
c.tokenHelpers.ChangeUserStatus(err, c.Logger, c.mattermostUserID, c.Poster)
return errors.Wrap(err, "msgraph Accept Event")
}

Expand All @@ -46,29 +46,29 @@ func (c *client) AcceptEvent(remoteUserID, eventID string) error {

func (c *client) DeclineEvent(remoteUserID, eventID string) error {
dummy := &msgraph.EventDeclineRequestParameter{}
if !c.checkUserStatus() {
c.Logger.Warnf(LogUserInactive)
if !c.tokenHelpers.CheckUserStatus(c.Logger, c.mattermostUserID) {
c.Logger.Warnf(LogUserInactive, c.mattermostUserID)
return errors.New(ErrorUserInactive)
}

err := c.rbuilder.Users().ID(remoteUserID).Events().ID(eventID).Decline(dummy).Request().Post(c.ctx)
if err != nil {
c.changeUserStatus(err)
c.tokenHelpers.ChangeUserStatus(err, c.Logger, c.mattermostUserID, c.Poster)
return errors.Wrap(err, "msgraph DeclineEvent")
}
return nil
}

func (c *client) TentativelyAcceptEvent(remoteUserID, eventID string) error {
dummy := &msgraph.EventTentativelyAcceptRequestParameter{}
if !c.checkUserStatus() {
c.Logger.Warnf(LogUserInactive)
if c.tokenHelpers != nil && !c.tokenHelpers.CheckUserStatus(c.Logger, c.mattermostUserID) {
c.Logger.Warnf(LogUserInactive, c.mattermostUserID)
return errors.New(ErrorUserInactive)
}

err := c.rbuilder.Users().ID(remoteUserID).Events().ID(eventID).TentativelyAccept(dummy).Request().Post(c.ctx)
if err != nil {
c.changeUserStatus(err)
c.tokenHelpers.ChangeUserStatus(err, c.Logger, c.mattermostUserID, c.Poster)
return errors.Wrap(err, "msgraph TentativelyAcceptEvent")
}
return nil
Expand Down
6 changes: 3 additions & 3 deletions server/remote/msgraph/find_meeting_times.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ import (
// FindMeetingTimes finds meeting time suggestions for a calendar event
func (c *client) FindMeetingTimes(remoteUserID string, params *remote.FindMeetingTimesParameters) (*remote.MeetingTimeSuggestionResults, error) {
meetingsOut := &remote.MeetingTimeSuggestionResults{}
if !c.checkUserStatus() {
c.Logger.Warnf(LogUserInactive)
if c.tokenHelpers != nil && !c.tokenHelpers.CheckUserStatus(c.Logger, c.mattermostUserID) {
c.Logger.Warnf(LogUserInactive, c.mattermostUserID)
return nil, errors.New(ErrorUserInactive)
}

req := c.rbuilder.Users().ID(remoteUserID).FindMeetingTimes(nil).Request()
err := req.JSONRequest(c.ctx, http.MethodPost, "", &params, &meetingsOut)
if err != nil {
c.changeUserStatus(err)
c.tokenHelpers.ChangeUserStatus(err, c.Logger, c.mattermostUserID, c.Poster)
return nil, errors.Wrap(err, "msgraph FindMeetingTimes")
}

Expand Down
6 changes: 3 additions & 3 deletions server/remote/msgraph/get_calendars.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,16 @@ func (c *client) GetCalendars(remoteUserID string) ([]*remote.Calendar, error) {
var v struct {
Value []*remote.Calendar `json:"value"`
}
if !c.checkUserStatus() {
c.Logger.Warnf(LogUserInactive)
if c.tokenHelpers != nil && !c.tokenHelpers.CheckUserStatus(c.Logger, c.mattermostUserID) {
c.Logger.Warnf(LogUserInactive, c.mattermostUserID)
return nil, errors.New(ErrorUserInactive)
}

req := c.rbuilder.Users().ID(remoteUserID).Calendars().Request()
req.Expand("children")
err := req.JSONRequest(c.ctx, http.MethodGet, "", nil, &v)
if err != nil {
c.changeUserStatus(err)
c.tokenHelpers.ChangeUserStatus(err, c.Logger, c.mattermostUserID, c.Poster)
return nil, errors.Wrap(err, "msgraph GetCalendars")
}
c.Logger.With(bot.LogContext{
Expand Down
Loading