Skip to content

MM-67945 Adding flag to activate forced prompt=consent on google oauth#132

Open
avasconcelos114 wants to merge 1 commit intomasterfrom
MM-67945
Open

MM-67945 Adding flag to activate forced prompt=consent on google oauth#132
avasconcelos114 wants to merge 1 commit intomasterfrom
MM-67945

Conversation

@avasconcelos114
Copy link
Contributor

@avasconcelos114 avasconcelos114 commented Mar 17, 2026

Summary

This PR adds the provider flag to ensure that users are always prompted to give consent in Google Calendar.
The relevant changes that this PR relies on can be found in mattermost/mattermost-plugin-mscalendar#482

⚠️ Note: This PR can only be merged once a new release of the MS Calendar plugin is out since this flag will only become valid in a new versionm, so the CI failures here are expected for now

Ticket Link

Fixes https://mattermost.atlassian.net/browse/MM-67945

QA Notes

Just some regression testing around multiple subsequent account connections (and confirming that refresh tokens will keep the session alive) will confirm that the current behavior is being preserved as intended

Change Impact: 🟡 Medium

Reasoning: This PR adds a single configuration flag (ForceOAuth2Consent: true) to the Google Calendar provider's OAuth2 feature set. While the change itself is isolated and straightforward, it directly impacts the critical OAuth2 authentication flow and introduces a dependency on an external plugin update (mscalendar) that must be released first before this can be safely merged.

Regression Risk: Medium. The change touches the OAuth2 authentication flow, which is a critical path in the plugin. Although the flag addition is isolated to provider configuration initialization with no logic changes, the feature only becomes valid when the mscalendar dependency is updated to a compatible version. Merging before the dependency update could introduce unexpected behavior. There are no unit tests covering the GetGcalProviderConfig() function or the new ForceOAuth2Consent flag.

QA Recommendation: Manual QA is recommended to validate the forced consent behavior. Test scenarios should include: (1) initial Google Calendar account connection with forced consent prompt, (2) multiple sequential account reconnections to verify consent is requested each time, (3) refresh token persistence and session continuity after the forced consent flow, and (4) OAuth token refresh behavior. QA should not be skipped given the authentication-critical nature of this change and the external dependency coordination required. Confirm mscalendar plugin v1.6.1 or later (the version supporting ForceOAuth2Consent) is deployed before testing.

@avasconcelos114 avasconcelos114 self-assigned this Mar 17, 2026
@avasconcelos114 avasconcelos114 requested a review from a team as a code owner March 17, 2026 20:40
@avasconcelos114 avasconcelos114 added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester Do Not Merge Should not be merged until this label is removed labels Mar 17, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 17, 2026

📝 Walkthrough

Walkthrough

Added ForceOAuth2Consent configuration flag to the Google Calendar provider settings in GetGcalProviderConfig function, requiring explicit OAuth2 consent for the provider. Single-line addition with no other modifications.

Changes

Cohort / File(s) Summary
OAuth2 Configuration
gcal/gcal.go
Added ForceOAuth2Consent: true to Google Calendar provider features to enforce OAuth2 consent requirement during provider setup.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested reviewers

  • nevyangelova
  • marianunez
  • ogi-m

Poem

🐰 A whisker twitch, a config true,
OAuth2 consent, now shiny and new!
One line added, security blessed,
Google Calendar provider—fully dressed!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main change: adding a flag to force OAuth2 consent prompt for Google Calendar authentication.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch MM-67945
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@gcal/gcal.go`:
- Line 31: The code is setting a non-existent field ForceOAuth2Consent on
config.ProviderFeatures (causing compile failure); remove the ForceOAuth2Consent
assignment and instead implement forced OAuth consent via the oauth2 flow (use
oauth2.ApprovalForce when building the auth URL with oauth2.Config.AuthCodeURL
or equivalent). Locate where ProviderFeatures is constructed (the struct using
ProviderFeatures and the call site that generates the OAuth auth URL) and either
delete the ForceOAuth2Consent line or conditionally set the auth URL option
oauth2.ApprovalForce so the consent prompt is forced without relying on a
nonexistent struct field; alternatively revert to waiting for the dependency to
add ForceOAuth2Consent in a future release.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 4dd2d2db-fdb3-4198-9d50-f51ae6b2c619

📥 Commits

Reviewing files that changed from the base of the PR and between 0dd764f and 47101b9.

📒 Files selected for processing (1)
  • gcal/gcal.go

Features: config.ProviderFeatures{
EncryptedStore: true,
EventNotifications: false,
ForceOAuth2Consent: true,
Copy link

@coderabbitai coderabbitai bot Mar 17, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Verify mscalendar dependency in go.mod =="
rg -n 'mattermost-plugin-mscalendar' go.mod

echo
echo "== If vendored, verify ProviderFeatures struct supports ForceOAuth2Consent =="
if [ -d vendor ]; then
  rg -n -C3 'type\s+ProviderFeatures\s+struct|ForceOAuth2Consent' vendor
else
  echo "No vendor directory found; verify the consumed module tag/commit includes ForceOAuth2Consent."
fi

Repository: mattermost/mattermost-plugin-google-calendar

Length of output: 366


🏁 Script executed:

cat -n gcal/gcal.go | sed -n '25,40p'

Repository: mattermost/mattermost-plugin-google-calendar

Length of output: 368


🌐 Web query:

mattermost-plugin-mscalendar v1.6.0 ForceOAuth2Consent ProviderFeatures github

💡 Result:

In mattermost-plugin-mscalendar v1.6.0 (released Feb 17, 2026, supports Mattermost 10.7.0+), the plugin was changed to force an OAuth consent prompt on every account connection to help ensure a refresh token is returned. This is the “ForceOAuth2Consent” behavior you’re referring to. [1]

Where it’s implemented (GitHub):

  • In calendar/engine/oauth2.go, AuthCodeURL now includes oauth2.ApprovalForce:
    • conf.AuthCodeURL(state, oauth2.AccessTypeOffline, oauth2.ApprovalForce) [2]
  • The related test expectation was updated to include prompt=consent in the authorize URL. [2]

About ProviderFeatures:

  • There is no ProviderFeatures identifier/config in this repository (a repo-wide GitHub code search returns 0 matches), so ForceOAuth2Consent here is not implemented via a ProviderFeatures flag, but directly via the OAuth URL options shown above. [3]

References: [1] [2] [3]


🌐 Web query:

mattermost-plugin-mscalendar github releases latest versions after v1.6.0

💡 Result:

There are no GitHub releases (or tags) newer than v1.6.0 for mattermost/mattermost-plugin-mscalendar as of March 17, 2026v1.6.0 (released Feb 17, 2026) is still the latest release/tag. [1], [2]

Versions visible around that point:

  • v1.6.0Feb 17, 2026 (latest)
  • v1.6.0-rc1Feb 11, 2026 (pre-release) [1], [2]

Sources: [1], [2]


ForceOAuth2Consent field does not exist in config.ProviderFeatures of the currently resolved mscalendar v1.6.0 dependency.

Line 31 attempts to set a field that is not defined in the dependency's struct. Either:

  1. Wait for mattermost-plugin-mscalendar to add ForceOAuth2Consent to ProviderFeatures and release a new version, or
  2. Implement OAuth consent forcing using the current v1.6.0 mechanism (oauth2.ApprovalForce in the auth URL options instead of a struct field).

This blocks compilation and merge.

🧰 Tools
🪛 GitHub Actions: ci

[error] 31-31: unknown field ForceOAuth2Consent in struct literal of type "github.com/mattermost/mattermost-plugin-mscalendar/calendar/config".ProviderFeatures

🪛 GitHub Check: plugin-ci / lint

[failure] 31-31:
unknown field ForceOAuth2Consent in struct literal of type "github.com/mattermost/mattermost-plugin-mscalendar/calendar/config".ProviderFeatures

🪛 GitHub Check: plugin-ci / test

[failure] 31-31:
unknown field ForceOAuth2Consent in struct literal of type "github.com/mattermost/mattermost-plugin-mscalendar/calendar/config".ProviderFeatures

🪛 golangci-lint (2.11.3)

[error] 31-31: : # github.com/mattermost/mattermost-plugin-google-calendar/gcal [github.com/mattermost/mattermost-plugin-google-calendar/gcal.test]
gcal/gcal.go:31:4: unknown field ForceOAuth2Consent in struct literal of type "github.com/mattermost/mattermost-plugin-mscalendar/calendar/config".ProviderFeatures

(typecheck)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gcal/gcal.go` at line 31, The code is setting a non-existent field
ForceOAuth2Consent on config.ProviderFeatures (causing compile failure); remove
the ForceOAuth2Consent assignment and instead implement forced OAuth consent via
the oauth2 flow (use oauth2.ApprovalForce when building the auth URL with
oauth2.Config.AuthCodeURL or equivalent). Locate where ProviderFeatures is
constructed (the struct using ProviderFeatures and the call site that generates
the OAuth auth URL) and either delete the ForceOAuth2Consent line or
conditionally set the auth URL option oauth2.ApprovalForce so the consent prompt
is forced without relying on a nonexistent struct field; alternatively revert to
waiting for the dependency to add ForceOAuth2Consent in a future release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are doing number 1 as the PR in mscalendar already exists and will have a version released with the ForceOAuth2Consent value

Copy link

Choose a reason for hiding this comment

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

@avasconcelos114, understood! That makes sense — the "Do Not Merge" label on this PR aligns well with the approach. Once the mscalendar PR (#482) is merged and a new version is released with ForceOAuth2Consent in config.ProviderFeatures, the only remaining step will be bumping the mscalendar dependency here to pick up that version, after which the compilation errors should resolve automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester Do Not Merge Should not be merged until this label is removed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant