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

After OAUTH, SAML auth completion, Redirect to App custom url scheme with token data as query params. #16447

Merged
merged 30 commits into from
Jan 19, 2021
Merged

After OAUTH, SAML auth completion, Redirect to App custom url scheme with token data as query params. #16447

merged 30 commits into from
Jan 19, 2021

Conversation

anurag6713
Copy link
Contributor

@anurag6713 anurag6713 commented Dec 2, 2020 •
edited
Loading

Summary

MM-30480:
For mobile clients, After Auth process is completed, It redirects to the specified url with cookie data as query params.

This is used by mobile client for new OAuth implementation as we cannot read cookies from the mobile browser, we will pass them using query params

MM-29644:

  • Web: Ignores redirect URL passed from client
  • Mobile: Validates the Scheme from redirect URL and makes sure it doesn't include default schemes (http, https etc)

Added screens to show auth success or on error:
Simulator Screen Shot - iPhone 11 - 2020-12-14 at 14 46 57
Simulator Screen Shot - iPhone 11 - 2020-12-14 at 15 06 40

Ticket Link

https://mattermost.atlassian.net/browse/MM-30480
https://mattermost.atlassian.net/browse/MM-29644

Release Note

  1. Added ability to redirect to the mobile app after OAuth & SAML auth completion.
  2. Validating RedirectURL for web and mobile OAuth flows.

@mm-cloud-bot
Copy link

@anurag6713: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

I understand the commands that are listed here

@anurag6713 anurag6713 marked this pull request as ready for review December 6, 2020 15:09
@anurag6713 anurag6713 requested review from sbishel and mkraft December 6, 2020 15:10
@anurag6713 anurag6713 added 2: Dev Review Requires review by a developer and removed do-not-merge/release-note-label-needed labels Dec 6, 2020
@mm-cloud-bot
Copy link

@anurag6713: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

I understand the commands that are listed here

mkraft
mkraft suggested changes Dec 7, 2020
View reviewed changes
sbishel
sbishel requested changes Dec 7, 2020
View reviewed changes
@anurag6713
Copy link
Contributor Author

/update-branch

@mm-cloud-bot
Copy link

@anurag6713: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

I understand the commands that are listed here

@anurag6713 anurag6713 changed the title After OAUTH completion, Redirect to specified url with cookie data as query params. After OAUTH, SAML auth completion, Redirect to specified url with cookie data as query params. Dec 10, 2020
@anurag6713 anurag6713 requested review from sbishel and mkraft December 11, 2020 02:16
@furqanmlk
Copy link
Contributor

Looks good to me,
Verified
5.30.0 - OAUTH Gitlab, Google App, Office 365 ✅
5.32.0 - OAUTH Gitlab, Google App, Office 365 ✅
5.25.0 - OAUTH Gitlab, Google App, Office 365 ✅

https://mattermost.atlassian.net/browse/MM-29644
image

Login Complete
image

@anurag6713 anurag6713 added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester. May occur at the same time as Dev Review labels Jan 18, 2021
@mm-cloud-bot
Copy link

@anurag6713: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

I understand the commands that are listed here

@anurag6713 anurag6713 merged commit 3da6f27 into mattermost:master Jan 19, 2021
@amyblais amyblais added this to the v5.32.0 milestone Jan 19, 2021
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jan 20, 2021
@mm-cloud-bot
Copy link

@anurag6713: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

I understand the commands that are listed here

1 similar comment
@mm-cloud-bot
Copy link

@anurag6713: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

I understand the commands that are listed here

@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Needed Requires documentation and removed Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jan 20, 2021
@amyblais
Copy link
Member

@anurag6713 Should the new setting be added to Telemetry https://github.com/mattermost/mattermost-server/blob/master/services/telemetry/telemetry.go?

@anurag6713
Copy link
Contributor Author

@anurag6713 Should the new setting be added to Telemetry https://github.com/mattermost/mattermost-server/blob/master/services/telemetry/telemetry.go?

@amyblais Yes we need to add this setting to telemetry. As I have merged this PR, will add it in a new PR. Thanks!

cwarnermm added a commit to mattermost/docs that referenced this pull request Feb 11, 2021
@cwarnermm
Redirect to App custom url scheme with token data as query params
a312e02 
Documentation for: mattermost/mattermost#16447

Updated:
- Self-Managed Admin Guide > Configure Mattermost > Configuration Settings > Site Configuration
   - Added new config setting: App Custom URL Schemes
@cwarnermm cwarnermm mentioned this pull request Feb 11, 2021
Merged
cwarnermm added a commit to mattermost/docs that referenced this pull request Feb 12, 2021
@cwarnermm
Redirect to App custom url scheme with token data as query params
95a7204 
Documentation for: mattermost/mattermost#16447

Updated:
- Corrected config.json values for the config setting based on dev reviewer feedback
amyblais pushed a commit to mattermost/docs that referenced this pull request Feb 12, 2021
@cwarnermm @justinegeffen
Redirect to App custom url scheme with token data as query params (#4306
bc59854 
)

* Redirect to App custom url scheme with token data as query params

Documentation for: mattermost/mattermost#16447

Updated:
- Self-Managed Admin Guide > Configure Mattermost > Configuration Settings > Site Configuration
   - Added new config setting: App Custom URL Schemes

* Update source/administration/config-settings.rst

Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com>

* Redirect to App custom url scheme with token data as query params

Documentation for: mattermost/mattermost#16447

Updated:
- Corrected config.json values for the config setting based on dev reviewer feedback

Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com>
@amyblais amyblais added Docs/Done Required documentation has been written and removed Docs/Needed Requires documentation labels Feb 13, 2021
amyblais added a commit to mattermost/docs that referenced this pull request Feb 15, 2021
@amyblais @cwarnermm
@mattermod @justinegeffen
V5.32 documentation (#4307)
c396a20 
* Update conf.py

* MM-31716 Remove the word experimental from gossip setting (#4273)

* MM-31716 Remove the word experimental from gossip setting

Documentation for: https://mattermost.atlassian.net/browse/MM-31716

Updated:
- Self-Managed Admin Guide > Configure Mattermost > Configuration Settings > Use Experimental Gossip
   - Removed the word "Experimental" from the heading title, the config.json setting name
   - Documented this setting as true by default (instead of false)

* MM-31716 Remove the word experimental from gossip setting

Documentation for: https://mattermost.atlassian.net/browse/MM-31716

Updated:
- Self-Managed Admin Guide > Scaling Mattermost > High Availability Cluster (E20) > Configuration and Compatibility > Mattermost Server Configuration  > Configuration Settings
   - Under step #1, updated the code sample to remove the word "Experimental" from the "UseExperimentalGossip" setting, and set the setting to "true" (from false)

* MM-31716 Remove the word experimental from gossip setting

Documentation for: https://mattermost.atlassian.net/browse/MM-31716

Updated:
- Self-Managed Admin Guide > Scaling Mattermost > High Availability Cluster (E20) > Upgrade Guide > Upgrading to Version 4.0 and Later
   - Under step #3, updated the code sample to remove the word "Experimental" from the "UseExperimentalGossip" setting, and set the setting to "true" (from false)

* Documentation for: https://mattermost.atlassian.net/browse/MM-31716

Documentation for: https://mattermost.atlassian.net/browse/MM-31716

Updated:
- Telemetry documentation to remove the word "Experimental" from the "UseExperimentalGossip" configuration setting.

* Update open-source-components.rst (#4282)

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

* Update important-upgrade-notes.rst (#4272)

* Update important-upgrade-notes.rst

* Update important-upgrade-notes.rst

* Update important-upgrade-notes.rst

* Update important-upgrade-notes.rst

* Update important-upgrade-notes.rst

* MM-4644 Upgrade Rate Limiting help text (#4276)

* MM-4644 Upgrade Rate Limiting help text

Documentation for: https://mattermost.atlassian.net/browse/MM-4644

Updated: 
- Self-Managed Admin Guide > Configure Mattermost > Configuration Settings > Environment > Rate Limiting
   - Documented what rate limiting does and why it is useful

* Update source/administration/config-settings.rst

Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com>

Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com>

* Adding Swedish and Bulgarian translations #7289 (#4274)

* Adding Swedish and Bulgarian translations #7289

Documentation for: mattermost/mattermost-webapp#7289

Updated:
- User's Guide > Settings > Account Settings > Display
   - Added Bulgarian and Swedish as supported languages

* Adding Swedish and Bulgarian translations #7289

Documentation for: mattermost/mattermost-webapp#7289

Updated:
- Cloud Admin Guide > Configuration > Site Configuration > Localization
   - Updated the first line to document that a total of 18 languages are available by default with the addition of Bulgarian and Swedish

* Adding Swedish and Bulgarian translations #7289

Documentation for: mattermost/mattermost-webapp#7289

Updated:
- Product Overview > Mattermost Team Edition
   - Updated the last bullet point about multi-language support to include Bulgarian and Swedish

* Adding Swedish and Bulgarian translations #7289

Documentation for: mattermost/mattermost-webapp#7289

Updated:
- User's Guide > Getting Started > User Mattermost Desktop App > App Options > Check Spelling
   - Updated list of languages that support spell check to match the current product UI.

Note: This updated list does not currently include all 18 supported languages, nor does it include the two most recently added languages: Bulgarian and Swedish.

* Adding Swedish and Bulgarian translations #7289

Documentation for: mattermost/mattermost-webapp#7289

Updated:
- Self-Managed Admin Guide > Configure Mattermost > Configuration Settings > Site Configuration
   - Updated Default Server Language, Default Client Language, and Available Languages settings to include support for Bulgarian and Swedish

* Update source/help/apps/desktop-guide.rst

Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com>

* Update source/overview/product.rst

Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com>

* Update source/overview/product.rst

Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com>

* Adding Swedish and Bulgarian translations #7289

Documentation for: mattermost/mattermost-webapp#7289

Updated:
- User's Guide > Settings > Account Settings > Display
   - Reordered the list of supported languages to match the product UI

Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com>

* Redirect to App custom url scheme with token data as query params (#4306)

* Redirect to App custom url scheme with token data as query params

Documentation for: mattermost/mattermost#16447

Updated:
- Self-Managed Admin Guide > Configure Mattermost > Configuration Settings > Site Configuration
   - Added new config setting: App Custom URL Schemes

* Update source/administration/config-settings.rst

Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com>

* Redirect to App custom url scheme with token data as query params

Documentation for: mattermost/mattermost#16447

Updated:
- Corrected config.json values for the config setting based on dev reviewer feedback

Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com>

* v5.32 Changelog (#4256)

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

* Update changelog.md

Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com>
Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkraft mkraft mkraft approved these changes

@furqanmlk furqanmlk furqanmlk approved these changes

@sbishel sbishel sbishel approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Done Required changelog entry has been written do-not-merge/release-note-label-needed Docs/Done Required documentation has been written
Projects
None yet
Milestone
v5.32.0
Development

Successfully merging this pull request may close these issues.
7 participants
@anurag6713 @mm-cloud-bot @furqanmlk @amyblais @mkraft @sbishel @mattermod